Merge branch 'hash-always-has-type' of github.com:obsidiansystems/nix into better-ca-parse-errors

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

mk/tests.mk

@@ -1,45 +1,15 @@
 # Run program $1 as part of ‘make installcheck’.
+
+test-deps =
+
 define run-install-test
 
-  installcheck: $1
+  installcheck: $1.test
 
-  _installcheck-list += $1
+  .PHONY: $1.test
+  $1.test: $1 $(test-deps)
+	@env TEST_NAME=$(notdir $(basename $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

src/libfetchers/cache.hh

@@ -6,6 +6,8 @@ namespace nix::fetchers {
 
 struct Cache
 {
+    virtual ~Cache() { }
+
     virtual void add(
         ref<Store> store,
         const Attrs & inAttrs,

src/libstore/build.cc

@@ -2753,8 +2753,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<StorePath> queryPathFromHashPart(const std::string & hashPart) override
     { throw Error("queryPathFromHashPart"); }

src/libstore/daemon.cc

@@ -347,6 +347,15 @@ static void performOp(TunnelLogger * logger, ref<Store> 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();

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";

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<StorePathSet>([&]() {
+    return retrySQLite<OutputPathMap>([&]() {
         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;
     });

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<StorePath> queryPathFromHashPart(const std::string & hashPart) override;
 

src/libstore/path.hh

@@ -62,6 +62,7 @@ public:
 
 typedef std::set<StorePath> StorePathSet;
 typedef std::vector<StorePath> StorePaths;
+typedef std::map<string, StorePath> OutputPathMap;
 
 /* Extension of derivations in the Nix store. */
 const std::string drvExtension = ".drv";

src/libstore/remote-store.cc

@@ -38,6 +38,29 @@ void writeStorePaths(const Store & store, Sink & out, const StorePathSet & paths
         out << store.printStorePath(i);
 }
 
+std::map<string, StorePath> readOutputPathMap(const Store & store, Source & from)
+{
+    std::map<string, StorePath> pathMap;
+    auto rawInput = readStrings<Strings>(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()) {
+        auto thisKey = *curInput++;
+        auto thisValue = *curInput++;
+        pathMap.emplace(thisKey, store.parseStorePath(thisValue));
+    }
+    return pathMap;
+}
+
+void writeOutputPathMap(const Store & store, Sink & out, const std::map<string, StorePath> & 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 +435,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<StorePathSet>(*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<StorePath> RemoteStore::queryPathFromHashPart(const std::string & hashPart)
 {
     auto conn(getConnection());

src/libstore/remote-store.hh

@@ -51,6 +51,7 @@ public:
 
     StorePathSet queryDerivationOutputs(const StorePath & path) override;
 
+    OutputPathMap queryDerivationOutputMap(const StorePath & path) override;
     std::optional<StorePath> queryPathFromHashPart(const std::string & hashPart) override;
 
     StorePathSet querySubstitutablePaths(const StorePathSet & paths) override;

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());

src/libstore/store-api.hh

@@ -419,8 +419,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. */

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<class T> 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);
 
 }

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);

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;
     }
 

src/libutil/hash.cc

@@ -406,7 +406,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);
     }
 }
 

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;
+
+    }
+
 }

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");
@@ -135,13 +135,7 @@ StorePath getDerivationEnvironment(ref<Store> 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 { htSHA256 },
-        },
-    });
+    drv.outputs.insert_or_assign("out", DerivationOutput { .path = shellOutPath });
     drv.env["out"] = store->printStorePath(shellOutPath);
     auto shellDrvPath2 = writeDerivation(store, drv, drvName);
 

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.
@@ -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=$!

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 <<EOF
 with import ./config.nix; mkDerivation {
   name = "gc-A";
   buildCommand = ''
     set -x
     [[ \$(ls \$NIX_STORE/*-garbage? | wc -l) = 3 ]]
+
     mkdir \$out
     echo foo > \$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) ]]

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...

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 {

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*

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

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
 

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)

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 bash -c '[[ -n $stdenv ]]'
+
+# Test 'nix print-dev-env'.
+source <(nix print-dev-env -f shell.nix shellDrv)
+[[ -n $stdenv ]]