Merge pull request #8214 from raphaelr/installable-derived-path-warn

Display valid installable in `InstallableDerivedPath::parse` warning

4 files changed, 67 insertions, 9 deletions

diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc
index e5f0f1b33..9a2ffda39 100644
--- a/src/libstore/derived-path.cc
+++ b/src/libstore/derived-path.cc
@@ -62,15 +62,31 @@ std::string DerivedPath::Opaque::to_string(const Store & store) const
 std::string DerivedPath::Built::to_string(const Store & store) const
 {
     return store.printStorePath(drvPath)
-        + "!"
+        + '^'
+        + outputs.to_string();
+}
+
+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(
-        [&](const auto & req) { return req.to_string(store); },
-        this->raw());
+    return std::visit(overloaded {
+        [&](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); },
+    }, this->raw());
 }
 
 
@@ -87,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
         ? (DerivedPath) DerivedPath::Opaque::parse(store, s)
         : (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 res;
diff --git a/src/libstore/derived-path.hh b/src/libstore/derived-path.hh
index 2155776b1..5f7acbebc 100644
--- a/src/libstore/derived-path.hh
+++ b/src/libstore/derived-path.hh
@@ -48,8 +48,18 @@ struct DerivedPathBuilt {
     StorePath drvPath;
     OutputsSpec outputs;
 
+    /**
+     * Uses `^` as the separator
+     */
     std::string to_string(const Store & store) const;
-    static DerivedPathBuilt parse(const Store & store, std::string_view, std::string_view);
+    /**
+     * 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;
 
     GENERATE_CMP(DerivedPathBuilt, me->drvPath, me->outputs);
@@ -81,8 +91,22 @@ struct DerivedPath : _DerivedPathRaw {
         return static_cast<const Raw &>(*this);
     }
 
+    /**
+     * 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);
+    /**
+     * Uses `!` as the separator
+     */
+    static DerivedPath parseLegacy(const Store & store, std::string_view);
 };
 
 /**
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index e128c3a29..b862902d1 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -90,12 +90,12 @@ void write(const Store & store, Sink & out, const ContentAddress & ca)
 DerivedPath read(const Store & store, Source & from, Phantom<DerivedPath> _)
 {
     auto s = readString(from);
-    return DerivedPath::parse(store, s);
+    return DerivedPath::parseLegacy(store, s);
 }
 
 void write(const Store & store, Sink & out, const DerivedPath & req)
 {
-    out << req.to_string(store);
+    out << req.to_string_legacy(store);
 }
 
 
diff --git a/src/libstore/tests/derived-path.cc b/src/libstore/tests/derived-path.cc
index d1ac2c5e7..e6d32dbd0 100644
--- a/src/libstore/tests/derived-path.cc
+++ b/src/libstore/tests/derived-path.cc
@@ -53,6 +53,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(
+    DerivedPathTest,
     prop_round_rip,
     (const DerivedPath & o))
 {