From 114a6e2b09eda7f23e7776e1cdf77715044e073e Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Wed, 11 Jan 2023 11:54:43 -0500
Subject: [PATCH] Make it hard to construct an empty `OutputsSpec::Names`

This should be a non-empty set, and so we don't want people doing this
by accident. We remove the zero-0 constructor with a little inheritance
trickery.
---
 src/libcmd/installables.cc            |  2 +-
 src/libstore/build/derivation-goal.cc |  4 ++--
 src/libstore/misc.cc                  |  2 +-
 src/libstore/outputs-spec.cc          |  8 ++++----
 src/libstore/outputs-spec.hh          | 17 ++++++++++++++++-
 src/libstore/path-with-outputs.cc     |  2 +-
 src/nix-build/nix-build.cc            |  2 +-
 7 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc
index c791eef39..3457f2e47 100644
--- a/src/libcmd/installables.cc
+++ b/src/libcmd/installables.cc
@@ -479,7 +479,7 @@ struct InstallableAttrPath : InstallableValue
             auto derivedPath = byDrvPath.emplace(*drvPath, DerivedPath::Built {
                 .drvPath = *drvPath,
                 // Not normally legal, but we will merge right below
-                .outputs = OutputsSpec::Names { },
+                .outputs = OutputsSpec::Names { StringSet { } },
             }).first;
 
             derivedPath->second.outputs.merge(std::visit(overloaded {
diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index 98c1ddaae..61169635a 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -990,7 +990,7 @@ void DerivationGoal::resolvedFinished()
                 return resolvedDrv.outputNames();
             },
             [&](const OutputsSpec::Names & names) {
-                return names;
+                return static_cast<std::set<std::string>>(names);
             },
         }, wantedOutputs.raw());
 
@@ -1325,7 +1325,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity()
             return StringSet {};
         },
         [&](const OutputsSpec::Names & names) {
-            return names;
+            return static_cast<StringSet>(names);
         },
     }, wantedOutputs.raw());
     DrvOutputs validOutputs;
diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc
index 1ff855cd0..5758c3d93 100644
--- a/src/libstore/misc.cc
+++ b/src/libstore/misc.cc
@@ -317,7 +317,7 @@ OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd,
             return names;
         },
         [&](const OutputsSpec::Names & names) {
-            return names;
+            return static_cast<std::set<std::string>>(names);
         },
     }, bfd.outputs);
     for (auto & output : outputNames) {
diff --git a/src/libstore/outputs-spec.cc b/src/libstore/outputs-spec.cc
index e7bd8ebd8..891252990 100644
--- a/src/libstore/outputs-spec.cc
+++ b/src/libstore/outputs-spec.cc
@@ -32,7 +32,7 @@ std::optional<OutputsSpec> OutputsSpec::parseOpt(std::string_view s)
         return { OutputsSpec::All {} };
 
     if (match[2].matched)
-        return { tokenizeString<OutputsSpec::Names>(match[2].str(), ",") };
+        return OutputsSpec::Names { tokenizeString<StringSet>(match[2].str(), ",") };
 
     assert(false);
 }
@@ -139,11 +139,11 @@ void to_json(nlohmann::json & json, const ExtendedOutputsSpec & extendedOutputsS
 
 void from_json(const nlohmann::json & json, OutputsSpec & outputsSpec)
 {
-    auto names = json.get<OutputNames>();
-    if (names == OutputNames({"*"}))
+    auto names = json.get<StringSet>();
+    if (names == StringSet({"*"}))
         outputsSpec = OutputsSpec::All {};
     else
-        outputsSpec = names;
+        outputsSpec = OutputsSpec::Names { std::move(names) };
 }
 
 
diff --git a/src/libstore/outputs-spec.hh b/src/libstore/outputs-spec.hh
index e81695da9..9c477ee2b 100644
--- a/src/libstore/outputs-spec.hh
+++ b/src/libstore/outputs-spec.hh
@@ -8,7 +8,22 @@
 
 namespace nix {
 
-typedef std::set<std::string> OutputNames;
+struct OutputNames : std::set<std::string> {
+    using std::set<std::string>::set;
+
+    // These need to be "inherited manually"
+    OutputNames(const std::set<std::string> & s)
+        : std::set<std::string>(s)
+    { }
+    OutputNames(std::set<std::string> && s)
+        : std::set<std::string>(s)
+    { }
+
+    /* This set should always be non-empty, so we delete this
+       constructor in order make creating empty ones by mistake harder.
+       */
+    OutputNames() = delete;
+};
 
 struct AllOutputs {
     bool operator < (const AllOutputs & _) const { return false; }
diff --git a/src/libstore/path-with-outputs.cc b/src/libstore/path-with-outputs.cc
index 10e0cc63e..6c0704ed9 100644
--- a/src/libstore/path-with-outputs.cc
+++ b/src/libstore/path-with-outputs.cc
@@ -53,7 +53,7 @@ std::variant<StorePathWithOutputs, StorePath> StorePathWithOutputs::tryFromDeriv
                         return {};
                     },
                     [&](const OutputsSpec::Names & outputs) {
-                        return outputs;
+                        return static_cast<StringSet>(outputs);
                     },
                 }, bfd.outputs.raw()),
             };
diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc
index 0a7a21de2..049838bb1 100644
--- a/src/nix-build/nix-build.cc
+++ b/src/nix-build/nix-build.cc
@@ -421,7 +421,7 @@ static void main_nix_build(int argc, char * * argv)
             {
                 pathsToBuild.push_back(DerivedPath::Built {
                     .drvPath = inputDrv,
-                    .outputs = inputOutputs
+                    .outputs = OutputsSpec::Names { inputOutputs },
                 });
                 pathsToCopy.insert(inputDrv);
             }