From 35e4f5f455bab2ad244f49a17295fe25433e0ac3 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 18 Jan 2025 09:44:46 +0100 Subject: [PATCH 1/2] nix-util: Add concatMapStrings (cherry picked from commit 583a852c8a6b7461dad3635337a7d2fe6f205fd3) Change-Id: I38e9174d800339cae26bcdbebb0e27008a96839e --- lix/libutil/strings.hh | 18 ++++++++++++++++- tests/unit/libutil/tests.cc | 39 +++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/lix/libutil/strings.hh b/lix/libutil/strings.hh index 2f865c379..67238824a 100644 --- a/lix/libutil/strings.hh +++ b/lix/libutil/strings.hh @@ -5,6 +5,7 @@ #include "lix/libutil/types.hh" #include +#include namespace nix { @@ -34,7 +35,6 @@ MakeError(FormatError, Error); */ template C tokenizeString(std::string_view s, std::string_view separators = " \t\n\r"); - /** * Concatenate the given strings with a separator between the * elements. @@ -63,6 +63,22 @@ auto concatStrings(Parts && ... parts) } +/** + * Apply a function to the `iterable`'s items and concat them with `separator`. + */ +template +std::string concatMapStringsSep(std::string_view separator, const C & iterable, F fn) +{ + boost::container::small_vector strings; + strings.reserve(iterable.size()); + for (const auto & elem : iterable) { + strings.push_back(fn(elem)); + } + return concatStringsSep(separator, strings); +} + + + /** * Add quotes around a collection of strings. */ diff --git a/tests/unit/libutil/tests.cc b/tests/unit/libutil/tests.cc index 1349be71f..00fcf8269 100644 --- a/tests/unit/libutil/tests.cc +++ b/tests/unit/libutil/tests.cc @@ -667,4 +667,43 @@ namespace nix { ASSERT_EQ(filterANSIEscapes("f๐ˆ๐ˆbรคr", true, 4), "f๐ˆ๐ˆb"); } + /* ---------------------------------------------------------------------------- + * concatMapStringsSep + * --------------------------------------------------------------------------*/ + TEST(concatMapStringsSep, empty) + { + Strings strings; + + ASSERT_EQ(concatMapStringsSep(",", strings, [](const std::string & s) { return s; }), ""); + } + + TEST(concatMapStringsSep, justOne) + { + Strings strings; + strings.push_back("this"); + + ASSERT_EQ(concatMapStringsSep(",", strings, [](const std::string & s) { return s; }), "this"); + } + + TEST(concatMapStringsSep, two) + { + Strings strings; + strings.push_back("this"); + strings.push_back("that"); + + ASSERT_EQ(concatMapStringsSep(",", strings, [](const std::string & s) { return s; }), "this,that"); + } + + TEST(concatMapStringsSep, map) + { + std::map strings; + strings["this"] = "that"; + strings["1"] = "one"; + + ASSERT_EQ( + concatMapStringsSep( + ", ", strings, [](const std::pair & s) { return s.first + " -> " + s.second; }), + "1 -> one, this -> that"); + } + } From 108c051fd826dabf9b011c9db01b75f7a7724eec Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Wed, 25 Dec 2024 21:09:58 +0000 Subject: [PATCH 2/2] local-derivation-goal: improve "illegal reference" error Before the change "illegal reference" was hard to interpret as it did not mention what derivation actually hits it. Today's `nixpkgs` example: Before the change: $ nix build --no-link -f. postgresql_14 ... error: derivation contains an illegal reference specifier 'man' After the change: $ nix build --no-link -f. postgresql_14 ... error: derivation '/nix/store/bxp6g57limvwiga61vdlyvhy7i8rp6wd-postgresql-14.15.drv' output check for 'lib' contains an illegal reference specifier 'man', expected store path or output name (one of [debug, dev, doc, lib, out]) Co-authored-by: Robert Hensing Co-authored-by: Maximilian Bosch (cherry picked from commit ec46a7e4dea8c568677d3d98588810bcd178f048) Change-Id: I36e3e951c282123e780a920d5bef59de74de9fe0 --- lix/libstore/build/local-derivation-goal.cc | 20 ++++++++++++++++++-- tests/functional/check-refs.nix | 6 ++++++ tests/functional/check-refs.sh | 4 ++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index 625f33cba..e332104fd 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -2516,8 +2516,24 @@ void LocalDerivationGoal::checkOutputs(const std::mappath); else if (auto storePath = get(alreadyRegisteredOutputs, i)) spec.insert(*storePath); - else - throw BuildError("derivation contains an illegal reference specifier '%s'", i); + else { + std::string outputsListing = concatMapStringsSep( + ", ", + newlyBuiltOutputs, + [](auto & o) { return o.first; } + ); + if (!alreadyRegisteredOutputs.empty()) { + outputsListing.append(outputsListing.empty() ? "" : ", "); + outputsListing.append(concatMapStringsSep( + ", ", + alreadyRegisteredOutputs, + [](auto & o) { return o.first; }) + ); + } + throw BuildError("derivation '%s' output check for '%s' contains an illegal reference specifier '%s'," + " expected store path or output name (one of [%s])", + worker.store.printStorePath(drvPath), outputName, i, outputsListing); + } } auto used = recursive diff --git a/tests/functional/check-refs.nix b/tests/functional/check-refs.nix index 89690e456..1a900830e 100644 --- a/tests/functional/check-refs.nix +++ b/tests/functional/check-refs.nix @@ -74,4 +74,10 @@ rec { buildCommand = ''echo ${dep} > "''${outputs[out]}"''; }; + test12 = makeTest 12 { + builder = builtins.toFile "builder.sh" "mkdir $out $lib"; + outputs = ["out" "lib"]; + disallowedReferences = ["dev"]; + }; + } diff --git a/tests/functional/check-refs.sh b/tests/functional/check-refs.sh index 3b587d1e5..d4a5aa29f 100644 --- a/tests/functional/check-refs.sh +++ b/tests/functional/check-refs.sh @@ -51,3 +51,7 @@ if isDaemonNewer 2.12pre20230103; then test11=$(nix-build -o $RESULT check-refs.nix -A test11) [[ -z $(nix-store -q --references "$test11") ]] fi + +# test12 should fail (syntactically invalid). +expectStderr 1 nix-build -vvv -o "$RESULT" check-refs.nix -A test12 >"$TEST_ROOT/test12.stderr" +grepQuiet -F "output check for 'lib' contains an illegal reference specifier 'dev', expected store path or output name (one of [lib, out])" < "$TEST_ROOT/test12.stderr"