Fix nix print-dev-env with drv paths

Fixes #8309 This regression was because both `CmdDevelop` and `CmdPrintDevEnv` were switched to be `InstallableValueCommand` subclasses, but actually neither should have been. The `nixpkgsFlakeRef` method should indeed not be on the base installable class, because "flake refs" and "nixpkgs" are not installable-wide notions, but that doesn't mean these commands should only accept installable values.
NixOS · May 10, 2023 · 82cf9de · 82cf9de
1 parent 53a1354
commit 82cf9de
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 13 deletions.
diff --git a/src/libcmd/installable-flake.cc b/src/libcmd/installable-flake.cc
@@ -234,7 +234,7 @@ FlakeRef InstallableFlake::nixpkgsFlakeRef() const
         }
     }
 
-    return InstallableValue::nixpkgsFlakeRef();
+    return defaultNixpkgsFlakeRef();
 }
 
 }
diff --git a/src/libcmd/installable-flake.hh b/src/libcmd/installable-flake.hh
@@ -67,9 +67,22 @@ struct InstallableFlake : InstallableValue
 
     std::shared_ptr<flake::LockedFlake> getLockedFlake() const;
 
-    FlakeRef nixpkgsFlakeRef() const override;
+    FlakeRef nixpkgsFlakeRef() const;
 };
 
+/**
+ * Default flake ref for referring to Nixpkgs. For flakes that don't
+ * have their own Nixpkgs input, or other installables.
+ *
+ * It is a layer violation for Nix to know about Nixpkgs; currently just
+ * `nix develop` does. Be wary of using this /
+ * `InstallableFlake::nixpkgsFlakeRef` more places.
+ */
+static inline FlakeRef defaultNixpkgsFlakeRef()
+{
+    return FlakeRef::fromAttrs({{"type","indirect"}, {"id", "nixpkgs"}});
+}
+
 ref<eval_cache::EvalCache> openEvalCache(
     EvalState & state,
     std::shared_ptr<flake::LockedFlake> lockedFlake);

diff --git a/src/libcmd/installable-value.hh b/src/libcmd/installable-value.hh
@@ -96,11 +96,6 @@ struct InstallableValue : Installable
 
     UnresolvedApp toApp(EvalState & state);
 
-    virtual FlakeRef nixpkgsFlakeRef() const
-    {
-        return FlakeRef::fromAttrs({{"type","indirect"}, {"id", "nixpkgs"}});
-    }
-
     static InstallableValue & require(Installable & installable);
     static ref<InstallableValue> require(ref<Installable> installable);
 };

diff --git a/src/nix/develop.cc b/src/nix/develop.cc
@@ -252,7 +252,7 @@ static StorePath getDerivationEnvironment(ref<Store> store, ref<Store> evalStore
     throw Error("get-env.sh failed to produce an environment");
 }
 
-struct Common : InstallableValueCommand, MixProfile
+struct Common : InstallableCommand, MixProfile
 {
     std::set<std::string> ignoreVars{
         "BASHOPTS",
@@ -374,7 +374,7 @@ struct Common : InstallableValueCommand, MixProfile
         return res;
     }
 
-    StorePath getShellOutPath(ref<Store> store, ref<InstallableValue> installable)
+    StorePath getShellOutPath(ref<Store> store, ref<Installable> installable)
     {
         auto path = installable->getStorePath();
         if (path && hasSuffix(path->to_string(), "-env"))
@@ -393,7 +393,7 @@ struct Common : InstallableValueCommand, MixProfile
     }
 
     std::pair<BuildEnvironment, std::string>
-    getBuildEnvironment(ref<Store> store, ref<InstallableValue> installable)
+    getBuildEnvironment(ref<Store> store, ref<Installable> installable)
     {
         auto shellOutPath = getShellOutPath(store, installable);
 
@@ -481,7 +481,7 @@ struct CmdDevelop : Common, MixEnvironment
           ;
     }
 
-    void run(ref<Store> store, ref<InstallableValue> installable) override
+    void run(ref<Store> store, ref<Installable> installable) override
     {
         auto [buildEnvironment, gcroot] = getBuildEnvironment(store, installable);
 
@@ -538,10 +538,14 @@ struct CmdDevelop : Common, MixEnvironment
             nixpkgsLockFlags.inputOverrides = {};
             nixpkgsLockFlags.inputUpdates = {};
 
+            auto nixpkgs = defaultNixpkgsFlakeRef();
+            if (auto * i = dynamic_cast<const InstallableFlake *>(&*installable))
+                nixpkgs = i->nixpkgsFlakeRef();
+
             auto bashInstallable = make_ref<InstallableFlake>(
                 this,
                 state,
-                installable->nixpkgsFlakeRef(),
+                std::move(nixpkgs),
                 "bashInteractive",
                 DefaultOutputs(),
                 Strings{},
@@ -605,7 +609,7 @@ struct CmdPrintDevEnv : Common, MixJSON
 
     Category category() override { return catUtility; }
 
-    void run(ref<Store> store, ref<InstallableValue> installable) override
+    void run(ref<Store> store, ref<Installable> installable) override
     {
         auto buildEnvironment = getBuildEnvironment(store, installable).first;
 

diff --git a/tests/nix-shell.sh b/tests/nix-shell.sh
@@ -98,6 +98,18 @@ nix develop -f "$shellDotNix" shellDrv -c echo foo |& grepQuiet foo
 nix print-dev-env -f "$shellDotNix" shellDrv > $TEST_ROOT/dev-env.sh
 nix print-dev-env -f "$shellDotNix" shellDrv --json > $TEST_ROOT/dev-env.json
 
+# Test with raw drv
+
+shellDrv=$(nix-instantiate "$shellDotNix" -A shellDrv.out)
+
+nix develop $shellDrv -c bash -c '[[ -n $stdenv ]]'
+
+nix print-dev-env $shellDrv > $TEST_ROOT/dev-env2.sh
+nix print-dev-env $shellDrv --json > $TEST_ROOT/dev-env2.json
+
+diff $TEST_ROOT/dev-env{,2}.sh
+diff $TEST_ROOT/dev-env{,2}.json
+
 # Ensure `nix print-dev-env --json` contains variable assignments.
 [[ $(jq -r .variables.arr1.value[2] $TEST_ROOT/dev-env.json) = '3 4' ]]