Merge pull request #7631 from edolstra/output-names

OutputSpec: Allow all valid output names

5 files changed, 35 insertions, 13 deletions

diff --git a/src/libstore/outputs-spec.cc b/src/libstore/outputs-spec.cc
index a9e4320d5..096443cb2 100644
--- a/src/libstore/outputs-spec.cc
+++ b/src/libstore/outputs-spec.cc
@@ -21,7 +21,8 @@ bool OutputsSpec::contains(const std::string & outputName) const
 
 std::optional<OutputsSpec> OutputsSpec::parseOpt(std::string_view s)
 {
-    static std::regex regex(R"((\*)|([a-z]+(,[a-z]+)*))");
+    // See checkName() for valid output name characters.
+    static std::regex regex(R"((\*)|([a-zA-Z\+\-\._\?=]+(,[a-zA-Z\+\-\._\?=]+)*))");
 
     std::smatch match;
     std::string s2 { s }; // until some improves std::regex
@@ -42,7 +43,7 @@ OutputsSpec OutputsSpec::parse(std::string_view s)
 {
     std::optional spec = parseOpt(s);
     if (!spec)
-        throw Error("Invalid outputs specifier: '%s'", s);
+        throw Error("invalid outputs specifier '%s'", s);
     return *spec;
 }
 
@@ -65,7 +66,7 @@ std::pair<std::string_view, ExtendedOutputsSpec> ExtendedOutputsSpec::parse(std:
 {
     std::optional spec = parseOpt(s);
     if (!spec)
-        throw Error("Invalid extended outputs specifier: '%s'", s);
+        throw Error("invalid extended outputs specifier '%s'", s);
     return *spec;
 }
 
diff --git a/src/libstore/tests/outputs-spec.cc b/src/libstore/tests/outputs-spec.cc
index c9c2cafd0..836ba7e82 100644
--- a/src/libstore/tests/outputs-spec.cc
+++ b/src/libstore/tests/outputs-spec.cc
@@ -40,6 +40,13 @@ TEST(OutputsSpec, names_out) {
     ASSERT_EQ(expected.to_string(), str);
 }
 
+TEST(OutputsSpec, names_underscore) {
+    std::string_view str = "a_b";
+    OutputsSpec expected = OutputsSpec::Names { "a_b" };
+    ASSERT_EQ(OutputsSpec::parse(str), expected);
+    ASSERT_EQ(expected.to_string(), str);
+}
+
 TEST(OutputsSpec, names_out_bin) {
     OutputsSpec expected = OutputsSpec::Names { "out", "bin" };
     ASSERT_EQ(OutputsSpec::parse("out,bin"), expected);
diff --git a/tests/build.sh b/tests/build.sh
index 036fb037e..a00fb5232 100644
--- a/tests/build.sh
+++ b/tests/build.sh
@@ -42,20 +42,21 @@ nix build -f multiple-outputs.nix --json 'a^*' --no-link | jq --exit-status '
 nix build -f multiple-outputs.nix --json e --no-link | jq --exit-status '
   (.[0] |
     (.drvPath | match(".*multiple-outputs-e.drv")) and
-    (.outputs | keys == ["a", "b"]))
+    (.outputs | keys == ["a_a", "b"]))
 '
 
 # But not when it's overriden.
-nix build -f multiple-outputs.nix --json e^a --no-link | jq --exit-status '
+nix build -f multiple-outputs.nix --json e^a_a --no-link
+nix build -f multiple-outputs.nix --json e^a_a --no-link | jq --exit-status '
   (.[0] |
     (.drvPath | match(".*multiple-outputs-e.drv")) and
-    (.outputs | keys == ["a"]))
+    (.outputs | keys == ["a_a"]))
 '
 
 nix build -f multiple-outputs.nix --json 'e^*' --no-link | jq --exit-status '
   (.[0] |
     (.drvPath | match(".*multiple-outputs-e.drv")) and
-    (.outputs | keys == ["a", "b", "c"]))
+    (.outputs | keys == ["a_a", "b", "c"]))
 '
 
 # Test building from raw store path to drv not expression.
@@ -88,7 +89,7 @@ nix build "$drv^first,second" --no-link --json | jq --exit-status '
     (.outputs |
       (keys | length == 2) and
       (.first | match(".*multiple-outputs-a-first")) and
-	  (.second | match(".*multiple-outputs-a-second"))))
+      (.second | match(".*multiple-outputs-a-second"))))
 '
 
 nix build "$drv^*" --no-link --json | jq --exit-status '
@@ -97,14 +98,14 @@ nix build "$drv^*" --no-link --json | jq --exit-status '
     (.outputs |
       (keys | length == 2) and
       (.first | match(".*multiple-outputs-a-first")) and
-	  (.second | match(".*multiple-outputs-a-second"))))
+      (.second | match(".*multiple-outputs-a-second"))))
 '
 
 # Make sure that `--impure` works (regression test for https://github.com/NixOS/nix/issues/6488)
 nix build --impure -f multiple-outputs.nix --json e --no-link | jq --exit-status '
   (.[0] |
     (.drvPath | match(".*multiple-outputs-e.drv")) and
-    (.outputs | keys == ["a", "b"]))
+    (.outputs | keys == ["a_a", "b"]))
 '
 
 testNormalization () {
diff --git a/tests/multiple-outputs.nix b/tests/multiple-outputs.nix
index 1429bc648..413d392e4 100644
--- a/tests/multiple-outputs.nix
+++ b/tests/multiple-outputs.nix
@@ -91,9 +91,9 @@ rec {
 
   e = mkDerivation {
     name = "multiple-outputs-e";
-    outputs = [ "a" "b" "c" ];
-    meta.outputsToInstall = [ "a" "b" ];
-    buildCommand = "mkdir $a $b $c";
+    outputs = [ "a_a" "b" "c" ];
+    meta.outputsToInstall = [ "a_a" "b" ];
+    buildCommand = "mkdir $a_a $b $c";
   };
 
   independent = mkDerivation {
@@ -117,4 +117,14 @@ rec {
       '';
   };
 
+  invalid-output-name-1 = mkDerivation {
+    name = "invalid-output-name-1";
+    outputs = [ "out/"];
+  };
+
+  invalid-output-name-2 = mkDerivation {
+    name = "invalid-output-name-2";
+    outputs = [ "x" "foo$"];
+  };
+
 }
diff --git a/tests/multiple-outputs.sh b/tests/multiple-outputs.sh
index 0d45ad35b..66be6fa64 100644
--- a/tests/multiple-outputs.sh
+++ b/tests/multiple-outputs.sh
@@ -83,3 +83,6 @@ nix-store --gc --keep-derivations --keep-outputs
 nix-store --gc --print-roots
 rm -rf $NIX_STORE_DIR/.links
 rmdir $NIX_STORE_DIR
+
+nix build -f multiple-outputs.nix invalid-output-name-1 2>&1 | grep 'contains illegal character'
+nix build -f multiple-outputs.nix invalid-output-name-2 2>&1 | grep 'contains illegal character'