Get rid of the --derivation flag

src/libcmd/command.cc

@@ -130,16 +130,6 @@ ref<EvalState> EvalCommand::getEvalState()
     return ref<EvalState>(evalState);
 }
 
-MixOperateOnOptions::MixOperateOnOptions()
-{
-    addFlag({
-        .longName = "derivation",
-        .description = "Operate on the [store derivation](../../glossary.md#gloss-store-derivation) rather than its outputs.",
-        .category = installablesCategory,
-        .handler = {&operateOn, OperateOn::Derivation},
-    });
-}
-
 BuiltPathsCommand::BuiltPathsCommand(bool recursive)
     : recursive(recursive)
 {
@@ -177,7 +167,7 @@ void BuiltPathsCommand::run(ref<Store> store, Installables && installables)
         for (auto & p : store->queryAllValidPaths())
             paths.push_back(BuiltPath::Opaque{p});
     } else {
-        paths = Installable::toBuiltPaths(getEvalStore(), store, realiseMode, operateOn, installables);
+        paths = Installable::toBuiltPaths(getEvalStore(), store, realiseMode, installables);
         if (recursive) {
             // XXX: This only computes the store path closure, ignoring
             // intermediate realisations

src/libcmd/command.hh

@@ -206,20 +206,13 @@ private:
     std::string _installable{"."};
 };
 
-struct MixOperateOnOptions : virtual Args
-{
-    OperateOn operateOn = OperateOn::Output;
-
-    MixOperateOnOptions();
-};
-
 /**
  * A command that operates on zero or more extant store paths.
  *
  * If the argument the user passes is a some sort of recipe for a path
  * not yet built, it must be built first.
  */
-struct BuiltPathsCommand : InstallablesCommand, virtual MixOperateOnOptions
+struct BuiltPathsCommand : InstallablesCommand
 {
 private:
 

src/libcmd/installables.cc

@@ -669,33 +669,22 @@ BuiltPaths Installable::toBuiltPaths(
     ref<Store> evalStore,
     ref<Store> store,
     Realise mode,
-    OperateOn operateOn,
     const Installables & installables)
 {
-    if (operateOn == OperateOn::Output) {
-        BuiltPaths res;
-        for (auto & p : Installable::build(evalStore, store, mode, installables))
-            res.push_back(p.path);
-        return res;
-    } else {
-        if (mode == Realise::Nothing)
-            settings.readOnlyMode = true;
-
-        BuiltPaths res;
-        for (auto & drvPath : Installable::toDerivations(store, installables, true))
-            res.push_back(BuiltPath::Opaque{drvPath});
-        return res;
-    }
+    BuiltPaths res;
+    for (auto & p : Installable::build(evalStore, store, mode, installables))
+        res.push_back(p.path);
+    return res;
 }
 
 StorePathSet Installable::toStorePaths(
     ref<Store> evalStore,
     ref<Store> store,
-    Realise mode, OperateOn operateOn,
+    Realise mode,
     const Installables & installables)
 {
     StorePathSet outPaths;
-    for (auto & path : toBuiltPaths(evalStore, store, mode, operateOn, installables)) {
+    for (auto & path : toBuiltPaths(evalStore, store, mode, installables)) {
         auto thisOutPaths = path.outPaths();
         outPaths.insert(thisOutPaths.begin(), thisOutPaths.end());
     }
@@ -705,10 +694,10 @@ StorePathSet Installable::toStorePaths(
 StorePath Installable::toStorePath(
     ref<Store> evalStore,
     ref<Store> store,
-    Realise mode, OperateOn operateOn,
+    Realise mode,
     ref<Installable> installable)
 {
-    auto paths = toStorePaths(evalStore, store, mode, operateOn, {installable});
+    auto paths = toStorePaths(evalStore, store, mode, {installable});
 
     if (paths.size() != 1)
         throw Error("argument '%s' should evaluate to one store path", installable->what());

src/libcmd/installables.hh

@@ -39,20 +39,6 @@ enum class Realise {
     Nothing
 };
 
-/**
- * How to handle derivations in commands that operate on store paths.
- */
-enum class OperateOn {
-    /**
-     * Operate on the output path.
-     */
-    Output,
-    /**
-     * Operate on the .drv path.
-     */
-    Derivation
-};
-
 /**
  * Extra info about a DerivedPath
  *
@@ -170,14 +156,12 @@ struct Installable
         ref<Store> evalStore,
         ref<Store> store,
         Realise mode,
-        OperateOn operateOn,
         const Installables & installables);
 
     static StorePath toStorePath(
         ref<Store> evalStore,
         ref<Store> store,
         Realise mode,
-        OperateOn operateOn,
         ref<Installable> installable);
 
     static std::set<StorePath> toDerivations(
@@ -189,7 +173,6 @@ struct Installable
         ref<Store> evalStore,
         ref<Store> store,
         Realise mode,
-        OperateOn operateOn,
         const Installables & installables);
 };
 

src/nix/develop.cc

@@ -374,7 +374,7 @@ struct Common : InstallableCommand, MixProfile
             auto dir = absPath(dir_);
             auto installable = parseInstallable(store, installable_);
             auto builtPaths = Installable::toStorePaths(
-                getEvalStore(), store, Realise::Nothing, OperateOn::Output, {installable});
+                getEvalStore(), store, Realise::Nothing, {installable});
             for (auto & path: builtPaths) {
                 auto from = store->printStorePath(path);
                 if (script.find(from) == std::string::npos)
@@ -628,7 +628,7 @@ struct CmdDevelop : Common, MixEnvironment
 
             bool found = false;
 
-            for (auto & path : Installable::toStorePaths(getEvalStore(), store, Realise::Outputs, OperateOn::Output, {bashInstallable})) {
+            for (auto & path : Installable::toStorePaths(getEvalStore(), store, Realise::Outputs, {bashInstallable})) {
                 auto s = store->printStorePath(path) + "/bin/bash";
                 if (pathExists(s)) {
                     shell = s;

src/nix/diff-closures.cc

@@ -106,7 +106,7 @@ void printClosureDiff(
 
 using namespace nix;
 
-struct CmdDiffClosures : SourceExprCommand, MixOperateOnOptions
+struct CmdDiffClosures : SourceExprCommand
 {
     std::string _before, _after;
 
@@ -131,9 +131,9 @@ struct CmdDiffClosures : SourceExprCommand, MixOperateOnOptions
     void run(ref<Store> store) override
     {
         auto before = parseInstallable(store, _before);
-        auto beforePath = Installable::toStorePath(getEvalStore(), store, Realise::Outputs, operateOn, before);
+        auto beforePath = Installable::toStorePath(getEvalStore(), store, Realise::Outputs, before);
         auto after = parseInstallable(store, _after);
-        auto afterPath = Installable::toStorePath(getEvalStore(), store, Realise::Outputs, operateOn, after);
+        auto afterPath = Installable::toStorePath(getEvalStore(), store, Realise::Outputs, after);
         printClosureDiff(store, beforePath, afterPath, "");
     }
 };

src/nix/run.cc

@@ -99,7 +99,7 @@ struct CmdShell : InstallablesCommand, MixEnvironment
 
     void run(ref<Store> store, Installables && installables) override
     {
-        auto outPaths = Installable::toStorePaths(getEvalStore(), store, Realise::Outputs, OperateOn::Output, installables);
+        auto outPaths = Installable::toStorePaths(getEvalStore(), store, Realise::Outputs, installables);
 
         auto accessor = store->getFSAccessor();
 

src/nix/why-depends.cc

@@ -27,7 +27,7 @@ static std::string filterPrintable(const std::string & s)
     return res;
 }
 
-struct CmdWhyDepends : SourceExprCommand, MixOperateOnOptions
+struct CmdWhyDepends : SourceExprCommand
 {
     std::string _package, _dependency;
     bool all = false;
@@ -78,7 +78,7 @@ struct CmdWhyDepends : SourceExprCommand, MixOperateOnOptions
     void run(ref<Store> store) override
     {
         auto package = parseInstallable(store, _package);
-        auto packagePath = Installable::toStorePath(getEvalStore(), store, Realise::Outputs, operateOn, package);
+        auto packagePath = Installable::toStorePath(getEvalStore(), store, Realise::Outputs, package);
 
         /* We don't need to build `dependency`. We try to get the store
          * path if it's already known, and if not, then it's not a dependency.
@@ -93,7 +93,7 @@ struct CmdWhyDepends : SourceExprCommand, MixOperateOnOptions
         auto dependency = parseInstallable(store, _dependency);
         auto optDependencyPath = [&]() -> std::optional<StorePath> {
             try {
-                return {Installable::toStorePath(getEvalStore(), store, Realise::Derivation, operateOn, dependency)};
+                return {Installable::toStorePath(getEvalStore(), store, Realise::Derivation, dependency)};
             } catch (MissingRealisation &) {
                 return std::nullopt;
             }

tests/functional/build.sh

@@ -66,7 +66,7 @@ nix build -f multiple-outputs.nix --json 'e.a_a.outPath' --no-link | jq --exit-s
 '
 
 # Illegal type of string context
-expectStderr 1 nix build -f multiple-outputs.nix 'e.a_a.drvPath' \
+expectStderr 1 nix build --impure --expr 'builtins.addDrvOutputDependencies (import ./multiple-outputs.nix).e.a_a.drvPath' \
   | grepQuiet "has a context which refers to a complete source and binary closure."
 
 # No string context
@@ -77,7 +77,7 @@ expectStderr 1 nix build --expr '""' --no-link \
 expectStderr 1 nix build --impure --expr 'with (import ./multiple-outputs.nix).e.a_a; "${drvPath}${outPath}"' --no-link \
   | grepQuiet "has 2 entries in its context. It should only have exactly one entry"
 
-nix build --impure --json --expr 'builtins.unsafeDiscardOutputDependency (import ./multiple-outputs.nix).e.a_a.drvPath' --no-link | jq --exit-status '
+nix build --json -f multiple-outputs.nix e.a_a.drvPath --no-link | jq --exit-status '
   (.[0] | match(".*multiple-outputs-e.drv"))
 '
 

tests/functional/config.nix.in

@@ -16,15 +16,32 @@ rec {
 
   shared = builtins.getEnv "_NIX_TEST_SHARED";
 
-  mkDerivation = args:
-    derivation ({
+  mkDerivation = args: let
+
+    drv = derivation ({
       inherit system;
       builder = shell;
       args = ["-e" args.builder or (builtins.toFile "builder-${args.name}.sh" ''
         if [ -e "$NIX_ATTRS_SH_FILE" ]; then source $NIX_ATTRS_SH_FILE; fi;
         eval "$buildCommand"
       '')];
       PATH = path;
-    } // caArgs // removeAttrs args ["builder" "meta"])
-    // { meta = args.meta or {}; };
+    } // caArgs // removeAttrs args ["builder" "meta"]);
+
+    fixUp = value: value // outputsMap // {
+      # Not unsafe, and we will add back if we need it.
+      # Nixpkgs should do this too, when `addDrvOutputDependencies` makes it into the NixOS stable release.
+      drvPath = (builtins.unsafeDiscardOutputDependency value.drvPath);
+    };
+
+    outputsMap = builtins.listToAttrs (map
+      (name: {
+        inherit name;
+        value = fixUp drv.${name};
+      })
+      (drv.outputs or []));
+
+  in fixUp drv // {
+    meta = args.meta or {};
+  };
 }

tests/functional/dyn-drv/eval-outputOf.sh

@@ -20,15 +20,15 @@ expectStderr 1 nix --experimental-features 'nix-command dynamic-derivations' eva
 #
 # Like the above, this is a restriction we could relax later.
 expectStderr 1 nix --experimental-features 'nix-command dynamic-derivations' eval --impure --expr \
-    'builtins.outputOf (import ../dependencies.nix {}).drvPath "out"' \
+    'builtins.outputOf (builtins.addDrvOutputDependencies (import ../dependencies.nix {}).drvPath) "out"' \
     | grepQuiet "has a context which refers to a complete source and binary closure. This is not supported at this time"
 
 # Test using `builtins.outputOf` with static derivations
 testStaticHello () {
     nix eval --impure --expr \
         'with (import ./text-hashed-output.nix); let
            a = hello.outPath;
-           b = builtins.outputOf (builtins.unsafeDiscardOutputDependency hello.drvPath) "out";
+           b = builtins.outputOf hello.drvPath "out";
          in builtins.trace a
            (builtins.trace b
              (assert a == b; null))'
@@ -53,7 +53,7 @@ NIX_TESTS_CA_BY_DEFAULT=1 testStaticHello
 nix eval --impure --expr \
     'with (import ./text-hashed-output.nix); let
        a = producingDrv.outPath;
-       b = builtins.outputOf (builtins.builtins.unsafeDiscardOutputDependency producingDrv.drvPath) "out";
+       b = builtins.outputOf producingDrv.drvPath "out";
      in builtins.trace a
        (builtins.trace b
          (assert a == b; null))'
@@ -67,7 +67,7 @@ testDynamicHello () {
     nix eval --impure --expr \
         'with (import ./text-hashed-output.nix); let
            a = builtins.outputOf producingDrv.outPath "out";
-           b = builtins.outputOf (builtins.outputOf (builtins.unsafeDiscardOutputDependency producingDrv.drvPath) "out") "out";
+           b = builtins.outputOf (builtins.outputOf producingDrv.drvPath "out") "out";
          in builtins.trace a
            (builtins.trace b
              (assert a == b; null))'

tests/functional/dyn-drv/recursive-mod-json.nix

@@ -8,7 +8,7 @@ mkDerivation rec {
 
   requiredSystemFeatures = [ "recursive-nix" ];
 
-  drv = builtins.unsafeDiscardOutputDependency (import ./text-hashed-output.nix).hello.drvPath;
+  drv = (import ./text-hashed-output.nix).hello.drvPath;
 
   buildCommand = ''
     export NIX_CONFIG='experimental-features = nix-command ca-derivations'

tests/functional/dyn-drv/text-hashed-output.nix

@@ -17,7 +17,7 @@ rec {
     name = "hello.drv";
     buildCommand = ''
       echo "Copying the derivation"
-      cp ${builtins.unsafeDiscardOutputDependency hello.drvPath} $out
+      cp ${hello.drvPath} $out
     '';
     __contentAddressed = true;
     outputHashMode = "text";

tests/functional/dyn-drv/text-hashed-output.sh

@@ -20,7 +20,7 @@ nix show-derivation "$drvProducingDrv"
 
 out1=$(nix-build ./text-hashed-output.nix -A producingDrv --no-out-link)
 
-nix path-info $drv --derivation --json | jq
-nix path-info $out1 --derivation --json | jq
+nix path-info $drv --json | jq
+nix path-info $drvProducingDrv --json | jq
 
 test $out1 == $drv

tests/functional/export-graph.nix

@@ -23,7 +23,7 @@ rec {
   foo."bar.buildGraph" = mkDerivation {
     name = "dependencies";
     builder = builtins.toFile "build-graph-builder" "${printRefs}";
-    exportReferencesGraph = ["refs" (import ./dependencies.nix {}).drvPath];
+    exportReferencesGraph = ["refs" (builtins.addDrvOutputDependencies (import ./dependencies.nix {}).drvPath) ];
   };
 
 }

tests/functional/flakes/build-paths.sh

@@ -49,9 +49,9 @@ cat > $flake1Dir/flake.nix <<EOF
       goodPath = path;
     };
 
-    a10 = builtins.unsafeDiscardOutputDependency self.drvCall.drvPath;
+    a10 = self.drvCall.drvPath;
 
-    a11 = self.drvCall.drvPath;
+    a11 = builtins.addDrvOutputDependencies self.drvCall.drvPath;
 
     a12 = self.drvCall.outPath;
 

tests/functional/multiple-outputs.nix

@@ -59,7 +59,7 @@ rec {
 
   c = mkDerivation {
     name = "multiple-outputs-c";
-    drv = b.drvPath;
+    drv = builtins.addDrvOutputDependencies b.drvPath;
     builder = builtins.toFile "builder.sh"
       ''
         mkdir $out
@@ -69,7 +69,7 @@ rec {
 
   d = mkDerivation {
     name = "multiple-outputs-d";
-    drv = builtins.unsafeDiscardOutputDependency b.drvPath;
+    drv = b.drvPath;
     builder = builtins.toFile "builder.sh"
       ''
         mkdir $out

tests/functional/why-depends.sh

@@ -6,7 +6,7 @@ cp ./dependencies.nix ./dependencies.builder0.sh ./config.nix $TEST_HOME
 
 cd $TEST_HOME
 
-nix why-depends --derivation --file ./dependencies.nix input2_drv input1_drv
+nix why-depends --file ./dependencies.nix input2_drv.drvPath input1_drv.drvPath
 nix why-depends --file ./dependencies.nix input2_drv input1_drv
 
 nix-build ./dependencies.nix -A input0_drv -o dep