From d38f860c3ef001a456d4d447f89219de5e3c830c Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 10 Jun 2020 11:20:52 +0200 Subject: [PATCH 01/16] Add a way to get all the outputs of a derivation with their label Generalize `queryDerivationOutputNames` and `queryDerivationOutputs` by adding a `queryDerivationOutputMap` that returns the map `outputName=>outputPath` (not that this is not equivalent to merging the results of `queryDerivationOutputs` and `queryDerivationOutputNames` as sets don't preserve the order, so we would end up with an incorrect mapping). squash! Add a way to get all the outputs of a derivation with their label Rename StorePathMap to OutputPathMap --- src/libstore/build.cc | 4 ++-- src/libstore/daemon.cc | 9 ++++++++ src/libstore/local-store.cc | 11 ++++++---- src/libstore/local-store.hh | 2 +- src/libstore/path.hh | 1 + src/libstore/remote-store.cc | 38 +++++++++++++++++++++++++++++++++ src/libstore/remote-store.hh | 1 + src/libstore/store-api.cc | 10 +++++++++ src/libstore/store-api.hh | 7 ++++-- src/libstore/worker-protocol.hh | 6 ++++-- 10 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 0c25897f8..bb668def5 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2750,8 +2750,8 @@ struct RestrictedStore : public LocalFSStore void queryReferrers(const StorePath & path, StorePathSet & referrers) override { } - StorePathSet queryDerivationOutputs(const StorePath & path) override - { throw Error("queryDerivationOutputs"); } + OutputPathMap queryDerivationOutputMap(const StorePath & path) override + { throw Error("queryDerivationOutputMap"); } std::optional queryPathFromHashPart(const std::string & hashPart) override { throw Error("queryPathFromHashPart"); } diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 842aef20c..6042bd610 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -347,6 +347,15 @@ static void performOp(TunnelLogger * logger, ref store, break; } + case wopQueryDerivationOutputMap: { + auto path = store->parseStorePath(readString(from)); + logger->startWork(); + OutputPathMap outputs = store->queryDerivationOutputMap(path); + logger->stopWork(); + writeOutputPathMap(*store, to, outputs); + break; + } + case wopQueryDeriver: { auto path = store->parseStorePath(readString(from)); logger->startWork(); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 0dfbed9fc..eed225349 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -774,17 +774,20 @@ StorePathSet LocalStore::queryValidDerivers(const StorePath & path) } -StorePathSet LocalStore::queryDerivationOutputs(const StorePath & path) +OutputPathMap LocalStore::queryDerivationOutputMap(const StorePath & path) { - return retrySQLite([&]() { + return retrySQLite([&]() { auto state(_state.lock()); auto useQueryDerivationOutputs(state->stmtQueryDerivationOutputs.use() (queryValidPathId(*state, path))); - StorePathSet outputs; + OutputPathMap outputs; while (useQueryDerivationOutputs.next()) - outputs.insert(parseStorePath(useQueryDerivationOutputs.getStr(1))); + outputs.emplace( + useQueryDerivationOutputs.getStr(0), + parseStorePath(useQueryDerivationOutputs.getStr(1)) + ); return outputs; }); diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index e17cc45ae..ff36cb00e 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -133,7 +133,7 @@ public: StorePathSet queryValidDerivers(const StorePath & path) override; - StorePathSet queryDerivationOutputs(const StorePath & path) override; + OutputPathMap queryDerivationOutputMap(const StorePath & path) override; std::optional queryPathFromHashPart(const std::string & hashPart) override; diff --git a/src/libstore/path.hh b/src/libstore/path.hh index 4f79843fe..e43a8b50c 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -62,6 +62,7 @@ public: typedef std::set StorePathSet; typedef std::vector StorePaths; +typedef std::map OutputPathMap; /* Extension of derivations in the Nix store. */ const std::string drvExtension = ".drv"; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index b7cc7a5fc..dc82dbdd9 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -38,6 +38,32 @@ void writeStorePaths(const Store & store, Sink & out, const StorePathSet & paths out << store.printStorePath(i); } +std::map readOutputPathMap(const Store & store, Source & from) +{ + std::map pathMap; + auto rawInput = readStrings(from); + auto curInput = rawInput.begin(); + while (curInput != rawInput.end()) { + string thisKey = *curInput; + curInput = std::next(curInput); + if (curInput == rawInput.end()) { + throw Error("Got an odd number of elements from the daemon when trying to read a map… that's odd"); + } + string thisValue = *curInput; + curInput = std::next(curInput); + pathMap.emplace(thisKey, store.parseStorePath(thisValue)); + } + return pathMap; +} + +void writeOutputPathMap(const Store & store, Sink & out, const std::map & pathMap) +{ + out << 2*pathMap.size(); + for (auto & i : pathMap) { + out << i.first; + out << store.printStorePath(i.second); + } +} /* TODO: Separate these store impls into different files, give them better names */ RemoteStore::RemoteStore(const Params & params) @@ -412,12 +438,24 @@ StorePathSet RemoteStore::queryValidDerivers(const StorePath & path) StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path) { auto conn(getConnection()); + if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 0x16) { + return Store::queryDerivationOutputs(path); + } conn->to << wopQueryDerivationOutputs << printStorePath(path); conn.processStderr(); return readStorePaths(*this, conn->from); } +OutputPathMap RemoteStore::queryDerivationOutputMap(const StorePath & path) +{ + auto conn(getConnection()); + conn->to << wopQueryDerivationOutputMap << printStorePath(path); + conn.processStderr(); + return readOutputPathMap(*this, conn->from); + +} + std::optional RemoteStore::queryPathFromHashPart(const std::string & hashPart) { auto conn(getConnection()); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 80c8e9f11..fb2052752 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -51,6 +51,7 @@ public: StorePathSet queryDerivationOutputs(const StorePath & path) override; + OutputPathMap queryDerivationOutputMap(const StorePath & path) override; std::optional queryPathFromHashPart(const std::string & hashPart) override; StorePathSet querySubstitutablePaths(const StorePathSet & paths) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index e4a4ae11e..c0a8bc9f6 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -242,6 +242,16 @@ bool Store::PathInfoCacheValue::isKnownNow() return std::chrono::steady_clock::now() < time_point + ttl; } +StorePathSet Store::queryDerivationOutputs(const StorePath & path) +{ + auto outputMap = this->queryDerivationOutputMap(path); + StorePathSet outputPaths; + for (auto & i: outputMap) { + outputPaths.emplace(std::move(i.second)); + } + return outputPaths; +} + bool Store::isValidPath(const StorePath & storePath) { std::string hashPart(storePath.hashPart()); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 25d78c297..b122e05d6 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -418,8 +418,11 @@ public: virtual StorePathSet queryValidDerivers(const StorePath & path) { return {}; }; /* Query the outputs of the derivation denoted by `path'. */ - virtual StorePathSet queryDerivationOutputs(const StorePath & path) - { unsupported("queryDerivationOutputs"); } + virtual StorePathSet queryDerivationOutputs(const StorePath & path); + + /* Query the mapping outputName=>outputPath for the given derivation */ + virtual OutputPathMap queryDerivationOutputMap(const StorePath & path) + { unsupported("queryDerivationOutputMap"); } /* Query the full store path given the hash part of a valid store path, or empty if the path doesn't exist. */ diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index ac42457fc..8b538f6da 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -6,7 +6,7 @@ namespace nix { #define WORKER_MAGIC_1 0x6e697863 #define WORKER_MAGIC_2 0x6478696f -#define PROTOCOL_VERSION 0x115 +#define PROTOCOL_VERSION 0x116 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) @@ -30,7 +30,7 @@ typedef enum { wopSetOptions = 19, wopCollectGarbage = 20, wopQuerySubstitutablePathInfo = 21, - wopQueryDerivationOutputs = 22, + wopQueryDerivationOutputs = 22, // obsolete wopQueryAllValidPaths = 23, wopQueryFailedPaths = 24, wopClearFailedPaths = 25, @@ -49,6 +49,7 @@ typedef enum { wopNarFromPath = 38, wopAddToStoreNar = 39, wopQueryMissing = 40, + wopQueryDerivationOutputMap = 41, } WorkerOp; @@ -69,5 +70,6 @@ template T readStorePaths(const Store & store, Source & from); void writeStorePaths(const Store & store, Sink & out, const StorePathSet & paths); +void writeOutputPathMap(const Store & store, Sink & out, const OutputPathMap & paths); } From 2b834d48aae35c5050895fac540ca55387925d0f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 29 Jun 2020 22:45:41 +0200 Subject: [PATCH 02/16] NAR parser: Fix missing name field check Discovered by @Kloenk. --- src/libutil/archive.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index 6a8484705..51c88537e 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -262,7 +262,7 @@ static void parse(ParseSink & sink, Source & source, const Path & path) names[name] = 0; } } else if (s == "node") { - if (s.empty()) throw badArchive("entry name missing"); + if (name.empty()) throw badArchive("entry name missing"); parse(sink, source, path + "/" + name); } else throw badArchive("unknown field " + s); From e72a16a339092359e58ac7626ea89d4182b26642 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Tue, 30 Jun 2020 11:00:51 -0600 Subject: [PATCH 03/16] check for a null symbol --- src/libutil/error.hh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 1e6102ce1..6982e30aa 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -67,7 +67,11 @@ struct ErrPos { { line = pos.line; column = pos.column; - file = pos.file; + // is file symbol null? + if (pos.file.set()) + file = pos.file; + else + file = ""; return *this; } From a0705e0dd152f2a19d096d02024723a5614c7726 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Tue, 30 Jun 2020 11:01:46 -0600 Subject: [PATCH 04/16] invalid pos check --- src/libutil/tests/logging.cc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index 6a6fb4ac3..6a58b9425 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -289,4 +289,22 @@ namespace nix { "what about this " ANSI_YELLOW "%3%" ANSI_NORMAL " " ANSI_YELLOW "one" ANSI_NORMAL); } + + /* ---------------------------------------------------------------------------- + * ErrPos + * --------------------------------------------------------------------------*/ + + TEST(errpos, invalidPos) { + + // contains an invalid symbol, which we should not dereference! + Pos invalid; + + // constructing without access violation. + ErrPos ep(invalid); + + // assignment without access violation. + ep = invalid; + + } + } From 38ccf2e24168ab5c0d2a7cf6b5bdec1b0784ebcd Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 1 Jul 2020 15:31:34 +0200 Subject: [PATCH 05/16] Cleanup --- src/libstore/remote-store.cc | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index dc82dbdd9..8c648d671 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -42,15 +42,12 @@ std::map readOutputPathMap(const Store & store, Source & from { std::map pathMap; auto rawInput = readStrings(from); + if (rawInput.size() % 2) + throw Error("got an odd number of elements from the daemon when trying to read a output path map"); auto curInput = rawInput.begin(); while (curInput != rawInput.end()) { - string thisKey = *curInput; - curInput = std::next(curInput); - if (curInput == rawInput.end()) { - throw Error("Got an odd number of elements from the daemon when trying to read a map… that's odd"); - } - string thisValue = *curInput; - curInput = std::next(curInput); + auto thisKey = *curInput++; + auto thisValue = *curInput++; pathMap.emplace(thisKey, store.parseStorePath(thisValue)); } return pathMap; From 1b5aa60767f4a918250b157de054e1862240ca10 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 2 Jul 2020 11:34:15 +0200 Subject: [PATCH 06/16] Run the tests in parallel Cause the time needed to run the testsuite to drop from ~4mins to ~40s --- mk/run_test.sh | 28 ++++++++++++++++++++++++++++ mk/tests.mk | 41 ++++------------------------------------- tests/common.sh.in | 2 +- 3 files changed, 33 insertions(+), 38 deletions(-) create mode 100755 mk/run_test.sh diff --git a/mk/run_test.sh b/mk/run_test.sh new file mode 100755 index 000000000..6af5b070a --- /dev/null +++ b/mk/run_test.sh @@ -0,0 +1,28 @@ +#!/bin/sh + +set -u + +red="" +green="" +yellow="" +normal="" + +post_run_msg="ran test $1..." +if [ -t 1 ]; then + red="" + green="" + yellow="" + normal="" +fi +(cd $(dirname $1) && env ${TESTS_ENVIRONMENT} init.sh 2>/dev/null > /dev/null) +log="$(cd $(dirname $1) && env ${TESTS_ENVIRONMENT} $(basename $1) 2>&1)" +status=$? +if [ $status -eq 0 ]; then + echo "$post_run_msg [${green}PASS$normal]" +elif [ $status -eq 99 ]; then + echo "$post_run_msg [${yellow}SKIP$normal]" +else + echo "$post_run_msg [${red}FAIL$normal]" + echo "$log" | sed 's/^/ /' + exit "$status" +fi diff --git a/mk/tests.mk b/mk/tests.mk index 70c30661b..e2258ede6 100644 --- a/mk/tests.mk +++ b/mk/tests.mk @@ -1,45 +1,12 @@ # Run program $1 as part of ‘make installcheck’. define run-install-test - installcheck: $1 + installcheck: $1.test - _installcheck-list += $1 + .PHONY: $1.test + $1.test: $1 tests/common.sh tests/init.sh + @env TEST_NAME=$1 TESTS_ENVIRONMENT="$(tests-environment)" mk/run_test.sh $1 endef -# Color code from https://unix.stackexchange.com/a/10065 -installcheck: - @total=0; failed=0; \ - red=""; \ - green=""; \ - yellow=""; \ - normal=""; \ - if [ -t 1 ]; then \ - red=""; \ - green=""; \ - yellow=""; \ - normal=""; \ - fi; \ - for i in $(_installcheck-list); do \ - total=$$((total + 1)); \ - printf "running test $$i..."; \ - log="$$(cd $$(dirname $$i) && $(tests-environment) $$(basename $$i) 2>&1)"; \ - status=$$?; \ - if [ $$status -eq 0 ]; then \ - echo " [$${green}PASS$$normal]"; \ - elif [ $$status -eq 99 ]; then \ - echo " [$${yellow}SKIP$$normal]"; \ - else \ - echo " [$${red}FAIL$$normal]"; \ - echo "$$log" | sed 's/^/ /'; \ - failed=$$((failed + 1)); \ - fi; \ - done; \ - if [ "$$failed" != 0 ]; then \ - echo "$${red}$$failed out of $$total tests failed $$normal"; \ - exit 1; \ - else \ - echo "$${green}All tests succeeded$$normal"; \ - fi - .PHONY: check installcheck diff --git a/tests/common.sh.in b/tests/common.sh.in index 73fe77345..c00ee58a1 100644 --- a/tests/common.sh.in +++ b/tests/common.sh.in @@ -1,6 +1,6 @@ set -e -export TEST_ROOT=$(realpath ${TMPDIR:-/tmp}/nix-test) +export TEST_ROOT=$(realpath ${TMPDIR:-/tmp}/nix-test)/${TEST_NAME:-default} export NIX_STORE_DIR if ! NIX_STORE_DIR=$(readlink -f $TEST_ROOT/store 2> /dev/null); then # Maybe the build directory is symlinked. From c762385457ce9e34536ba5de0b144d16d25ffbf6 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 2 Jul 2020 15:43:35 +0200 Subject: [PATCH 07/16] Make the gc-concurrent test more reliable Use a fifo pipe to handle the synchronisation between the different threads rather than relying on delays --- tests/gc-concurrent.builder.sh | 5 ++++- tests/gc-concurrent.nix | 3 +++ tests/gc-concurrent.sh | 21 +++++++++++---------- tests/gc-concurrent2.builder.sh | 2 -- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/tests/gc-concurrent.builder.sh b/tests/gc-concurrent.builder.sh index 0cd67df3a..bb6dcd4cf 100644 --- a/tests/gc-concurrent.builder.sh +++ b/tests/gc-concurrent.builder.sh @@ -1,7 +1,10 @@ +echo "Build started" > "$lockFifo" + mkdir $out echo $(cat $input1/foo)$(cat $input2/bar) > $out/foobar -sleep 10 +# Wait for someone to write on the fifo +cat "$lockFifo" # $out should not have been GC'ed while we were sleeping, but just in # case... diff --git a/tests/gc-concurrent.nix b/tests/gc-concurrent.nix index 21671ea2c..0aba1f983 100644 --- a/tests/gc-concurrent.nix +++ b/tests/gc-concurrent.nix @@ -1,5 +1,7 @@ with import ./config.nix; +{ lockFifo ? null }: + rec { input1 = mkDerivation { @@ -16,6 +18,7 @@ rec { name = "gc-concurrent"; builder = ./gc-concurrent.builder.sh; inherit input1 input2; + inherit lockFifo; }; test2 = mkDerivation { diff --git a/tests/gc-concurrent.sh b/tests/gc-concurrent.sh index d395930ca..2c6622c62 100644 --- a/tests/gc-concurrent.sh +++ b/tests/gc-concurrent.sh @@ -2,7 +2,10 @@ source common.sh clearStore -drvPath1=$(nix-instantiate gc-concurrent.nix -A test1) +lockFifo1=$TEST_ROOT/test1.fifo +mkfifo "$lockFifo1" + +drvPath1=$(nix-instantiate gc-concurrent.nix -A test1 --argstr lockFifo "$lockFifo1") outPath1=$(nix-store -q $drvPath1) drvPath2=$(nix-instantiate gc-concurrent.nix -A test2) @@ -22,19 +25,16 @@ ln -s $outPath3 "$NIX_STATE_DIR"/gcroots/foo2 nix-store -rvv "$drvPath1" & pid1=$! -# Start build #2 in the background after 10 seconds. -(sleep 10 && nix-store -rvv "$drvPath2") & -pid2=$! +# Wait for the build of $drvPath1 to start +cat $lockFifo1 # Run the garbage collector while the build is running. -sleep 6 nix-collect-garbage -# Wait for build #1/#2 to finish. +# Unlock the build of $drvPath1 +echo "" > $lockFifo1 echo waiting for pid $pid1 to finish... wait $pid1 -echo waiting for pid $pid2 to finish... -wait $pid2 # Check that the root of build #1 and its dependencies haven't been # deleted. The should not be deleted by the GC because they were @@ -42,8 +42,9 @@ wait $pid2 cat $outPath1/foobar cat $outPath1/input-2/bar -# Check that build #2 has succeeded. It should succeed because the -# derivation is a GC root. +# Check that the build build $drvPath2 succeeds. +# It should succeed because the derivation is a GC root. +nix-store -rvv "$drvPath2" cat $outPath2/foobar rm -f "$NIX_STATE_DIR"/gcroots/foo* diff --git a/tests/gc-concurrent2.builder.sh b/tests/gc-concurrent2.builder.sh index 4bfb33103..4f6c58b96 100644 --- a/tests/gc-concurrent2.builder.sh +++ b/tests/gc-concurrent2.builder.sh @@ -3,5 +3,3 @@ echo $(cat $input1/foo)$(cat $input2/bar)xyzzy > $out/foobar # Check that the GC hasn't deleted the lock on our output. test -e "$out.lock" - -sleep 6 From 11ba4ec795cf52769e1c6403437cc51d6408b6e4 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 2 Jul 2020 15:43:35 +0200 Subject: [PATCH 08/16] Make the gc-auto test more reliable Use a fifo pipe to handle the synchronisation between the different threads rather than relying on delays --- tests/gc-auto.sh | 51 ++++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/tests/gc-auto.sh b/tests/gc-auto.sh index de1e2cfe4..b282644ca 100644 --- a/tests/gc-auto.sh +++ b/tests/gc-auto.sh @@ -13,24 +13,32 @@ fake_free=$TEST_ROOT/fake-free export _NIX_TEST_FREE_SPACE_FILE=$fake_free echo 1100 > $fake_free +fifoLock=$TEST_ROOT/fifoLock +mkfifo "$fifoLock" + expr=$(cat < \$out/bar - echo 1... - sleep 2 - echo 200 > ${fake_free}.tmp1 + + # Pretend that we run out of space + echo 100 > ${fake_free}.tmp1 mv ${fake_free}.tmp1 $fake_free - echo 2... - sleep 2 - echo 3... - sleep 2 - echo 4... - [[ \$(ls \$NIX_STORE/*-garbage? | wc -l) = 1 ]] + + # Wait for the GC to run + for i in {1..20}; do + echo ''\${i}... + if [[ \$(ls \$NIX_STORE/*-garbage? | wc -l) = 1 ]]; then + exit 0 + fi + sleep 1 + done + exit 1 ''; } EOF @@ -43,15 +51,9 @@ with import ./config.nix; mkDerivation { set -x mkdir \$out echo foo > \$out/bar - echo 1... - sleep 2 - echo 200 > ${fake_free}.tmp2 - mv ${fake_free}.tmp2 $fake_free - echo 2... - sleep 2 - echo 3... - sleep 2 - echo 4... + + # Wait for the first build to finish + cat "$fifoLock" ''; } EOF @@ -59,12 +61,19 @@ EOF nix build -v -o $TEST_ROOT/result-A -L "($expr)" \ --min-free 1000 --max-free 2000 --min-free-check-interval 1 & -pid=$! +pid1=$! nix build -v -o $TEST_ROOT/result-B -L "($expr2)" \ - --min-free 1000 --max-free 2000 --min-free-check-interval 1 + --min-free 1000 --max-free 2000 --min-free-check-interval 1 & +pid2=$! -wait "$pid" +# Once the first build is done, unblock the second one. +# If the first build fails, we need to postpone the failure to still allow +# the second one to finish +wait "$pid1" || FIRSTBUILDSTATUS=$? +echo "unlock" > $fifoLock +( exit ${FIRSTBUILDSTATUS:-0} ) +wait "$pid2" [[ foo = $(cat $TEST_ROOT/result-A/bar) ]] [[ foo = $(cat $TEST_ROOT/result-B/bar) ]] From a5b6e870fe9fb5ebf462c19963588d62f56d0f21 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 2 Jul 2020 16:37:30 +0200 Subject: [PATCH 09/16] Set gc-reserved-space to 0 in tests This reduces the amount of disk space needed to run the tests from half a gigabyte to 10 megabytes. --- tests/init.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/init.sh b/tests/init.sh index c62c4856a..0c2c0e170 100644 --- a/tests/init.sh +++ b/tests/init.sh @@ -18,6 +18,7 @@ build-users-group = keep-derivations = false sandbox = false experimental-features = nix-command flakes +gc-reserved-space = 0 include nix.conf.extra EOF From b5e42536977d84359f5fea3f42cfefdd46799eb1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 2 Jul 2020 18:24:11 +0200 Subject: [PATCH 10/16] Fix abort in 'nix develop' --- src/libutil/hash.cc | 2 +- src/nix/develop.cc | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 01fae3044..1a3e7c5d8 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -370,7 +370,7 @@ string printHashType(HashType ht) default: // illegal hash type enum value internally, as opposed to external input // which should be validated with nice error message. - abort(); + assert(false); } } diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 037987313..289e3bed3 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -135,13 +135,7 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) drv.inputSrcs.insert(std::move(getEnvShPath)); Hash h = hashDerivationModulo(*store, drv, true); auto shellOutPath = store->makeOutputPath("out", h, drvName); - drv.outputs.insert_or_assign("out", DerivationOutput { - .path = shellOutPath, - .hash = FixedOutputHash { - .method = FileIngestionMethod::Flat, - .hash = Hash { }, - }, - }); + drv.outputs.insert_or_assign("out", DerivationOutput { .path = shellOutPath }); drv.env["out"] = store->printStorePath(shellOutPath); auto shellDrvPath2 = writeDerivation(store, drv, drvName); From 5596f879b43c20e5115d7bf9fddb70cdeec78988 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 2 Jul 2020 18:32:45 +0200 Subject: [PATCH 11/16] Add test for nix develop --- tests/nix-shell.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/nix-shell.sh b/tests/nix-shell.sh index 235e2a5ff..2ed40068a 100644 --- a/tests/nix-shell.sh +++ b/tests/nix-shell.sh @@ -55,3 +55,10 @@ chmod a+rx $TEST_ROOT/shell.shebang.rb output=$($TEST_ROOT/shell.shebang.rb abc ruby) [ "$output" = '-e load("'"$TEST_ROOT"'/shell.shebang.rb") -- abc ruby' ] + +# Test 'nix develop'. +nix develop -f shell.nix shellDrv -c sh -c '[[ -n $stdenv ]]' + +# Test 'nix print-dev-env'. +source <(nix print-dev-env -f shell.nix shellDrv) +[[ -n $stdenv ]] From 5101ed18bca509a8cf43668b0701afad90c5c9c4 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 2 Jul 2020 17:15:02 +0200 Subject: [PATCH 12/16] Fix the test dependencies Reuse the pre-existing list rather than the one written as part of #3777 --- mk/tests.mk | 5 ++++- tests/local.mk | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/mk/tests.mk b/mk/tests.mk index e2258ede6..e6bd1ad79 100644 --- a/mk/tests.mk +++ b/mk/tests.mk @@ -1,10 +1,13 @@ # Run program $1 as part of ‘make installcheck’. + +test-deps = + define run-install-test installcheck: $1.test .PHONY: $1.test - $1.test: $1 tests/common.sh tests/init.sh + $1.test: $1 $(test-deps) @env TEST_NAME=$1 TESTS_ENVIRONMENT="$(tests-environment)" mk/run_test.sh $1 endef diff --git a/tests/local.mk b/tests/local.mk index 536661af8..f3ac330d8 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -40,4 +40,4 @@ tests-environment = NIX_REMOTE= $(bash) -e clean-files += $(d)/common.sh -installcheck: $(d)/common.sh $(d)/config.nix $(d)/plugins/libplugintest.$(SO_EXT) +test-deps += tests/common.sh tests/config.nix tests/plugins/libplugintest.$(SO_EXT) From 223fbe644a4cde45269c01e971331d9414971d46 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 2 Jul 2020 17:18:03 +0200 Subject: [PATCH 13/16] Shorten the path to the test root Fix a socket length failure on the OSX builders --- mk/tests.mk | 2 +- src/libstore/globals.cc | 2 +- tests/common.sh.in | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/mk/tests.mk b/mk/tests.mk index e6bd1ad79..2e39bb694 100644 --- a/mk/tests.mk +++ b/mk/tests.mk @@ -8,7 +8,7 @@ define run-install-test .PHONY: $1.test $1.test: $1 $(test-deps) - @env TEST_NAME=$1 TESTS_ENVIRONMENT="$(tests-environment)" mk/run_test.sh $1 + @env TEST_NAME=$(notdir $(basename $1)) TESTS_ENVIRONMENT="$(tests-environment)" mk/run_test.sh $1 endef diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index bee94cbd8..683fa5196 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -35,7 +35,7 @@ Settings::Settings() , nixLibexecDir(canonPath(getEnv("NIX_LIBEXEC_DIR").value_or(NIX_LIBEXEC_DIR))) , nixBinDir(canonPath(getEnv("NIX_BIN_DIR").value_or(NIX_BIN_DIR))) , nixManDir(canonPath(NIX_MAN_DIR)) - , nixDaemonSocketFile(canonPath(nixStateDir + DEFAULT_SOCKET_PATH)) + , nixDaemonSocketFile(canonPath(getEnv("NIX_DAEMON_SOCKET_PATH").value_or(nixStateDir + DEFAULT_SOCKET_PATH))) { buildUsersGroup = getuid() == 0 ? "nixbld" : ""; lockCPU = getEnv("NIX_AFFINITY_HACK") == "1"; diff --git a/tests/common.sh.in b/tests/common.sh.in index c00ee58a1..308126094 100644 --- a/tests/common.sh.in +++ b/tests/common.sh.in @@ -11,6 +11,7 @@ export NIX_LOCALSTATE_DIR=$TEST_ROOT/var export NIX_LOG_DIR=$TEST_ROOT/var/log/nix export NIX_STATE_DIR=$TEST_ROOT/var/nix export NIX_CONF_DIR=$TEST_ROOT/etc +export NIX_DAEMON_SOCKET_PATH=$TEST_ROOT/daemon-socket unset NIX_USER_CONF_FILES export _NIX_TEST_SHARED=$TEST_ROOT/shared if [[ -n $NIX_STORE ]]; then @@ -76,7 +77,7 @@ startDaemon() { rm -f $NIX_STATE_DIR/daemon-socket/socket nix-daemon & for ((i = 0; i < 30; i++)); do - if [ -e $NIX_STATE_DIR/daemon-socket/socket ]; then break; fi + if [ -e $NIX_DAEMON_SOCKET_PATH ]; then break; fi sleep 1 done pidDaemon=$! From 017efae01f81fabf23eba24407280171f8004448 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 3 Jul 2020 13:16:22 +0200 Subject: [PATCH 14/16] Hopefully fix macOS test failure --- tests/nix-shell.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/nix-shell.sh b/tests/nix-shell.sh index 2ed40068a..650904057 100644 --- a/tests/nix-shell.sh +++ b/tests/nix-shell.sh @@ -57,7 +57,7 @@ output=$($TEST_ROOT/shell.shebang.rb abc ruby) [ "$output" = '-e load("'"$TEST_ROOT"'/shell.shebang.rb") -- abc ruby' ] # Test 'nix develop'. -nix develop -f shell.nix shellDrv -c sh -c '[[ -n $stdenv ]]' +nix develop -f shell.nix shellDrv -c bash -c '[[ -n $stdenv ]]' # Test 'nix print-dev-env'. source <(nix print-dev-env -f shell.nix shellDrv) From 6f8fd3a3f22de24223bba05635aaa62107d92767 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 3 Jul 2020 14:50:07 +0200 Subject: [PATCH 15/16] Shut up a clang warning --- src/libfetchers/cache.hh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libfetchers/cache.hh b/src/libfetchers/cache.hh index d76ab1233..3db4f081c 100644 --- a/src/libfetchers/cache.hh +++ b/src/libfetchers/cache.hh @@ -6,6 +6,8 @@ namespace nix::fetchers { struct Cache { + virtual ~Cache() { } + virtual void add( ref store, const Attrs & inAttrs, From c3c7aedbb5ac869b7c454e90683f77b9c527a75a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 3 Jul 2020 14:58:58 +0200 Subject: [PATCH 16/16] nix develop: Fix bad regex This was accepted by libstdc++ but not libc++. https://hydra.nixos.org/build/123569154 --- src/nix/develop.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 289e3bed3..eb93f56fc 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -50,7 +50,7 @@ BuildEnvironment readEnvironment(const Path & path) R"re((?:\$?'(?:[^'\\]|\\[abeEfnrtv\\'"?])*'))re"; static std::string indexedArrayRegex = - R"re((?:\(( *\[[0-9]+]="(?:[^"\\]|\\.)*")**\)))re"; + R"re((?:\(( *\[[0-9]+\]="(?:[^"\\]|\\.)*")*\)))re"; static std::regex varRegex( "^(" + varNameRegex + ")=(" + simpleStringRegex + "|" + quotedStringRegex + "|" + indexedArrayRegex + ")\n");