Legacy vs non-legacy to_string/parse for DerivedPath

As requested by @roberth, it is good to call out the specific instances
we care about, which is `!` for the RPC protocols, and `^` for humans.

This doesn't take advantage of parametricity as much, but since the
human and computer interfaces are good to decouple anyways (we don't
care if they drift further apart over time in the slightest) some
separation and slight duplication is fine.

Also, unit test both round trips.
This commit is contained in:
John Ericson 2023-04-14 20:45:11 -04:00
parent 9e8f209036
commit ee420ac64e
5 changed files with 69 additions and 12 deletions

View file

@ -47,7 +47,7 @@ InstallableDerivedPath InstallableDerivedPath::parse(
}; };
warn( warn(
"The interpretation of store paths arguments ending in `.drv` recently changed. If this command is now failing try again with '%s'", "The interpretation of store paths arguments ending in `.drv` recently changed. If this command is now failing try again with '%s'",
oldDerivedPath.to_string(*store, '^')); oldDerivedPath.to_string(*store));
}; };
return DerivedPath::Opaque { return DerivedPath::Opaque {
.path = std::move(storePath), .path = std::move(storePath),

View file

@ -59,17 +59,32 @@ std::string DerivedPath::Opaque::to_string(const Store & store) const
return store.printStorePath(path); return store.printStorePath(path);
} }
std::string DerivedPath::Built::to_string(const Store & store, char separator) const std::string DerivedPath::Built::to_string(const Store & store) const
{ {
return store.printStorePath(drvPath) return store.printStorePath(drvPath)
+ separator + '^'
+ outputs.to_string(); + outputs.to_string();
} }
std::string DerivedPath::to_string(const Store & store, char separator) const std::string DerivedPath::Built::to_string_legacy(const Store & store) const
{
return store.printStorePath(drvPath)
+ '!'
+ outputs.to_string();
}
std::string DerivedPath::to_string(const Store & store) const
{ {
return std::visit(overloaded { return std::visit(overloaded {
[&](const DerivedPath::Built & req) { return req.to_string(store, separator); }, [&](const DerivedPath::Built & req) { return req.to_string(store); },
[&](const DerivedPath::Opaque & req) { return req.to_string(store); },
}, this->raw());
}
std::string DerivedPath::to_string_legacy(const Store & store) const
{
return std::visit(overloaded {
[&](const DerivedPath::Built & req) { return req.to_string_legacy(store); },
[&](const DerivedPath::Opaque & req) { return req.to_string(store); }, [&](const DerivedPath::Opaque & req) { return req.to_string(store); },
}, this->raw()); }, this->raw());
} }
@ -88,14 +103,24 @@ DerivedPath::Built DerivedPath::Built::parse(const Store & store, std::string_vi
}; };
} }
DerivedPath DerivedPath::parse(const Store & store, std::string_view s) static inline DerivedPath parseWith(const Store & store, std::string_view s, std::string_view separator)
{ {
size_t n = s.find("!"); size_t n = s.find(separator);
return n == s.npos return n == s.npos
? (DerivedPath) DerivedPath::Opaque::parse(store, s) ? (DerivedPath) DerivedPath::Opaque::parse(store, s)
: (DerivedPath) DerivedPath::Built::parse(store, s.substr(0, n), s.substr(n + 1)); : (DerivedPath) DerivedPath::Built::parse(store, s.substr(0, n), s.substr(n + 1));
} }
DerivedPath DerivedPath::parse(const Store & store, std::string_view s)
{
return parseWith(store, s, "^");
}
DerivedPath DerivedPath::parseLegacy(const Store & store, std::string_view s)
{
return parseWith(store, s, "!");
}
RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const
{ {
RealisedPath::Set res; RealisedPath::Set res;

View file

@ -48,8 +48,18 @@ struct DerivedPathBuilt {
StorePath drvPath; StorePath drvPath;
OutputsSpec outputs; OutputsSpec outputs;
std::string to_string(const Store & store, char separator = '!') const; /**
static DerivedPathBuilt parse(const Store & store, std::string_view, std::string_view); * Uses `^` as the separator
*/
std::string to_string(const Store & store) const;
/**
* Uses `!` as the separator
*/
std::string to_string_legacy(const Store & store) const;
/**
* The caller splits on the separator, so it works for both variants.
*/
static DerivedPathBuilt parse(const Store & store, std::string_view drvPath, std::string_view outputs);
nlohmann::json toJSON(ref<Store> store) const; nlohmann::json toJSON(ref<Store> store) const;
GENERATE_CMP(DerivedPathBuilt, me->drvPath, me->outputs); GENERATE_CMP(DerivedPathBuilt, me->drvPath, me->outputs);
@ -81,8 +91,22 @@ struct DerivedPath : _DerivedPathRaw {
return static_cast<const Raw &>(*this); return static_cast<const Raw &>(*this);
} }
std::string to_string(const Store & store, char separator = '!') const; /**
* Uses `^` as the separator
*/
std::string to_string(const Store & store) const;
/**
* Uses `!` as the separator
*/
std::string to_string_legacy(const Store & store) const;
/**
* Uses `^` as the separator
*/
static DerivedPath parse(const Store & store, std::string_view); static DerivedPath parse(const Store & store, std::string_view);
/**
* Uses `!` as the separator
*/
static DerivedPath parseLegacy(const Store & store, std::string_view);
}; };
/** /**

View file

@ -90,12 +90,12 @@ void write(const Store & store, Sink & out, const ContentAddress & ca)
DerivedPath read(const Store & store, Source & from, Phantom<DerivedPath> _) DerivedPath read(const Store & store, Source & from, Phantom<DerivedPath> _)
{ {
auto s = readString(from); auto s = readString(from);
return DerivedPath::parse(store, s); return DerivedPath::parseLegacy(store, s);
} }
void write(const Store & store, Sink & out, const DerivedPath & req) void write(const Store & store, Sink & out, const DerivedPath & req)
{ {
out << req.to_string(store); out << req.to_string_legacy(store);
} }

View file

@ -51,6 +51,14 @@ TEST_F(DerivedPathTest, force_init)
{ {
} }
RC_GTEST_FIXTURE_PROP(
DerivedPathTest,
prop_legacy_round_rip,
(const DerivedPath & o))
{
RC_ASSERT(o == DerivedPath::parseLegacy(*store, o.to_string_legacy(*store)));
}
RC_GTEST_FIXTURE_PROP( RC_GTEST_FIXTURE_PROP(
DerivedPathTest, DerivedPathTest,
prop_round_rip, prop_round_rip,