Split `OutputsSpec` and `ExtendedOutputsSpec`, use the former more

`DerivedPath::Built` and `DerivationGoal` were previously using a
regular set with the convention that the empty set means all outputs.
But it is easy to forget about this rule when processing those sets.
Using `OutputSpec` forces us to get it right.

1 files changed, 19 insertions, 4 deletions

diff --git a/src/libstore/path-with-outputs.cc b/src/libstore/path-with-outputs.cc
index 17230ffc4..10e0cc63e 100644
--- a/src/libstore/path-with-outputs.cc
+++ b/src/libstore/path-with-outputs.cc
@@ -15,10 +15,14 @@ std::string StorePathWithOutputs::to_string(const Store & store) const
 
 DerivedPath StorePathWithOutputs::toDerivedPath() const
 {
-    if (!outputs.empty() || path.isDerivation())
-        return DerivedPath::Built { path, outputs };
-    else
+    if (!outputs.empty()) {
+        return DerivedPath::Built { path, OutputsSpec::Names { outputs } };
+    } else if (path.isDerivation()) {
+        assert(outputs.empty());
+        return DerivedPath::Built { path, OutputsSpec::All { } };
+    } else {
         return DerivedPath::Opaque { path };
+    }
 }
 
 
@@ -41,7 +45,18 @@ std::variant<StorePathWithOutputs, StorePath> StorePathWithOutputs::tryFromDeriv
             return StorePathWithOutputs { bo.path };
         },
         [&](const DerivedPath::Built & bfd) -> std::variant<StorePathWithOutputs, StorePath> {
-            return StorePathWithOutputs { bfd.drvPath, bfd.outputs };
+            return StorePathWithOutputs {
+                .path = bfd.drvPath,
+                // Use legacy encoding of wildcard as empty set
+                .outputs = std::visit(overloaded {
+                    [&](const OutputsSpec::All &) -> StringSet {
+                        return {};
+                    },
+                    [&](const OutputsSpec::Names & outputs) {
+                        return outputs;
+                    },
+                }, bfd.outputs.raw()),
+            };
         },
     }, p.raw());
 }