Merge pull request #7600 from obsidiansystems/explicit-drv-ness

doc/manual/src/release-notes/rl-next.md

@@ -1,2 +1,18 @@
 # Release X.Y (202?-??-??)
 
+* The special handling of an [installable](../command-ref/new-cli/nix.md#installables) with `.drv` suffix being interpreted as all of the given [store derivation](../glossary.md#gloss-store-derivation)'s output paths is removed, and instead taken as the literal store path that it represents.
+
+  The new `^` syntax for store paths introduced in Nix 2.13 allows explicitly referencing output paths of a derivation.
+  Using this is better and more clear than relying on the now-removed `.drv` special handling.
+
+  For example,
+  ```shell-session
+  $ nix path-info /nix/store/gzaflydcr6sb3567hap9q6srzx8ggdgg-glibc-2.33-78.drv
+  ```
+
+  now gives info about the derivation itself, while
+
+  ```shell-session
+  $ nix path-info /nix/store/gzaflydcr6sb3567hap9q6srzx8ggdgg-glibc-2.33-78.drv^*
+  ```
+  provides information about each of its outputs.

src/libcmd/installable-derived-path.cc

@@ -31,27 +31,24 @@ InstallableDerivedPath InstallableDerivedPath::parse(
     ExtendedOutputsSpec extendedOutputsSpec)
 {
     auto derivedPath = std::visit(overloaded {
-        // If the user did not use ^, we treat the output more liberally.
+        // If the user did not use ^, we treat the output more
+        // liberally: we accept a symlink chain or an actual
+        // store path.
         [&](const ExtendedOutputsSpec::Default &) -> DerivedPath {
-            // First, we accept a symlink chain or an actual store path.
             auto storePath = store->followLinksToStorePath(prefix);
-            // Second, we see if the store path ends in `.drv` to decide what sort
-            // of derived path they want.
-            //
-            // This handling predates the `^` syntax. The `^*` in
-            // `/nix/store/hash-foo.drv^*` unambiguously means "do the
-            // `DerivedPath::Built` case", so plain `/nix/store/hash-foo.drv` could
-            // also unambiguously mean "do the DerivedPath::Opaque` case".
-            //
-            // Issue #7261 tracks reconsidering this `.drv` dispatching.
-            return storePath.isDerivation()
-                ? (DerivedPath) DerivedPath::Built {
-                    .drvPath = std::move(storePath),
-                    .outputs = OutputsSpec::All {},
-                }
-                : (DerivedPath) DerivedPath::Opaque {
-                    .path = std::move(storePath),
+            // Remove this prior to stabilizing the new CLI.
+            if (storePath.isDerivation()) {
+                auto oldDerivedPath = DerivedPath::Built {
+                    .drvPath = storePath,
+                    .outputs = OutputsSpec::All { },
                 };
+                warn(
+                    "The interpretation of store paths arguments ending in `.drv` recently changed. If this command is now failing try again with '%s'",
+                    oldDerivedPath.to_string(*store));
+            };
+            return DerivedPath::Opaque {
+                .path = std::move(storePath),
+            };
         },
         // If the user did use ^, we just do exactly what is written.
         [&](const ExtendedOutputsSpec::Explicit & outputSpec) -> DerivedPath {

src/libcmd/installables.cc

@@ -677,9 +677,12 @@ StorePathSet Installable::toDerivations(
         for (const auto & b : i->toDerivedPaths())
             std::visit(overloaded {
                 [&](const DerivedPath::Opaque & bo) {
-                    if (!useDeriver)
-                        throw Error("argument '%s' did not evaluate to a derivation", i->what());
-                    drvPaths.insert(getDeriver(store, *i, bo.path));
+                    drvPaths.insert(
+                        bo.path.isDerivation()
+                            ? bo.path
+                        : useDeriver
+                            ? getDeriver(store, *i, bo.path)
+                        : throw Error("argument '%s' did not evaluate to a derivation", i->what()));
                 },
                 [&](const DerivedPath::Built & bfd) {
                     drvPaths.insert(bfd.drvPath);

tests/binary-cache.sh

@@ -23,7 +23,7 @@ nix log $outPath 2>&1 | grep 'is not available'
 nix log --substituters file://$cacheDir $outPath | grep FOO
 
 # Test copying build logs from the binary cache.
-nix store copy-log --from file://$cacheDir $(nix-store -qd $outPath)
+nix store copy-log --from file://$cacheDir $(nix-store -qd $outPath)^'*'
 nix log $outPath | grep FOO
 
 basicDownloadTests() {

tests/ca/substitute.sh

@@ -28,6 +28,12 @@ nix realisation info --file ./content-addressed.nix transitivelyDependentCA
 nix realisation info --file ./content-addressed.nix dependentCA
 # nix realisation info --file ./content-addressed.nix rootCA --outputs out
 
+if isDaemonNewer "2.13"; then
+    pushToStore="../push-to-store.sh"
+else
+    pushToStore="../push-to-store-old.sh"
+fi
+
 # Same thing, but
 # 1. With non-ca derivations
 # 2. Erasing the realisations on the remote store
@@ -37,7 +43,7 @@ nix realisation info --file ./content-addressed.nix dependentCA
 #
 # Regression test for #4725
 clearStore
-nix build --file ../simple.nix -L --no-link --post-build-hook ../push-to-store.sh
+nix build --file ../simple.nix -L --no-link --post-build-hook "$pushToStore"
 clearStore
 rm -r "$REMOTE_STORE_DIR/realisations"
 nix build --file ../simple.nix -L --no-link --substitute --substituters "$REMOTE_STORE" --no-require-sigs -j0
@@ -52,7 +58,7 @@ if [[ -z "$(ls "$REMOTE_STORE_DIR/realisations")" ]]; then
 fi
 
 # Test the local realisation disk cache
-buildDrvs --post-build-hook ../push-to-store.sh
+buildDrvs --post-build-hook "$pushToStore"
 clearStore
 # Add the realisations of rootCA to the cachecache
 clearCacheCache

tests/post-hook.sh

@@ -9,8 +9,14 @@ echo 'require-sigs = false' >> $NIX_CONF_DIR/nix.conf
 
 restartDaemon
 
+if isDaemonNewer "2.13"; then
+    pushToStore="$PWD/push-to-store.sh"
+else
+    pushToStore="$PWD/push-to-store-old.sh"
+fi
+
 # Build the dependencies and push them to the remote store.
-nix-build -o $TEST_ROOT/result dependencies.nix --post-build-hook $PWD/push-to-store.sh
+nix-build -o $TEST_ROOT/result dependencies.nix --post-build-hook "$pushToStore"
 
 clearStore
 

tests/push-to-store-old.sh

@@ -0,0 +1,10 @@
+#!/bin/sh
+
+set -x
+set -e
+
+[ -n "$OUT_PATHS" ]
+[ -n "$DRV_PATH" ]
+
+echo Pushing "$OUT_PATHS" to "$REMOTE_STORE"
+printf "%s" "$DRV_PATH" | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs

tests/push-to-store.sh

@@ -7,4 +7,4 @@ set -e
 [ -n "$DRV_PATH" ]
 
 echo Pushing "$OUT_PATHS" to "$REMOTE_STORE"
-printf "%s" "$DRV_PATH" | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs
+printf "%s" "$DRV_PATH"^'*' | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs