Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

broken nix print-dev-env on drvs #8309

Closed
manveru opened this issue May 9, 2023 · 2 comments · Fixed by #8310
Closed

broken nix print-dev-env on drvs #8309

manveru opened this issue May 9, 2023 · 2 comments · Fixed by #8310
Labels

Comments

@manveru
Copy link
Contributor

manveru commented May 9, 2023

Describe the bug

nix print-dev-env doesn't work on derivations and store paths anymore. It seems to require a flake reference.

Steps To Reproduce

  1. nix run nix/2.15.1#nix -- print-dev-env "$(nix eval --raw nixpkgs#hello.drvPath)"
  2. See error
❮ nix run nix/2.15-maintenance#nix -- print-dev-env "$(nix eval --raw nixpkgs#hello.drvPath)"
warning: The interpretation of store paths arguments ending in `.drv` recently changed. If this command is now failing try again with '/nix/store/m7clchh41sryqpiz6x4mbrwhv4n08k3g-hello-2.12.1.drv^*'
error: installable '/nix/store/m7clchh41sryqpiz6x4mbrwhv4n08k3g-hello-2.12.1.drv' does not correspond to a Nix language value
Try '/nix/store/gh0h2cqcp3fnh3s1kcr0kicc7n2sidq1-nix-2.15.1/bin/nix --help' for more information.

Expected behavior

This should print the development environment for the given derivation.

nix-env --version output

nix (Nix) 2.15.0

Additional context

The commit that caused this is 256f3e3

The old behavior is used by https://github.com/divnix/std/blob/63b3a0e83f6a7c9d326f18198b2e6e4110b44816/src/blocktypes/devshells.nix#L55

I made a haphazard patch to fix the issue, but I'm sure there's a better way than duplicating all that code. After trying for a few hours I couldn't make any sense of the C++ errors and gave up on that.

diff --git a/src/nix/develop.cc b/src/nix/develop.cc
index 9e2dcff61..28efb9e9a 100644
--- a/src/nix/develop.cc
+++ b/src/nix/develop.cc
@@ -589,7 +589,7 @@ struct CmdDevelop : Common, MixEnvironment
     }
 };

-struct CmdPrintDevEnv : Common, MixJSON
+struct CmdPrintDevEnv : InstallableCommand, MixProfile, MixJSON
 {
     std::string description() override
     {
@@ -603,9 +603,31 @@ struct CmdPrintDevEnv : Common, MixJSON
           ;
     }

+    std::set<std::string> ignoreVars{
+        "BASHOPTS",
+        "HOME", // FIXME: don't ignore in pure mode?
+        "NIX_BUILD_TOP",
+        "NIX_ENFORCE_PURITY",
+        "NIX_LOG_FD",
+        "NIX_REMOTE",
+        "PPID",
+        "SHELL",
+        "SHELLOPTS",
+        "SSL_CERT_FILE", // FIXME: only want to ignore /no-cert-file.crt
+        "TEMP",
+        "TEMPDIR",
+        "TERM",
+        "TMP",
+        "TMPDIR",
+        "TZ",
+        "UID",
+    };
+
+    std::vector<std::pair<std::string, std::string>> redirects;
+
     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;

@@ -616,6 +638,106 @@ struct CmdPrintDevEnv : Common, MixJSON
             ? buildEnvironment.toJSON()
             : makeRcScript(store, buildEnvironment));
     }
+
+    std::pair<BuildEnvironment, std::string>
+    getBuildEnvironment(ref<Store> store, ref<Installable> installable)
+    {
+        auto shellOutPath = getShellOutPath(store, installable);
+
+        auto strPath = store->printStorePath(shellOutPath);
+
+        updateProfile(shellOutPath);
+
+        debug("reading environment file '%s'", strPath);
+
+        return {BuildEnvironment::fromJSON(readFile(store->toRealPath(shellOutPath))), strPath};
+    }
+
+    StorePath getShellOutPath(ref<Store> store, ref<Installable> installable)
+    {
+        auto path = installable->getStorePath();
+        if (path && hasSuffix(path->to_string(), "-env"))
+            return *path;
+        else {
+            auto drvs = Installable::toDerivations(store, {installable});
+
+            if (drvs.size() != 1)
+                throw Error("'%s' needs to evaluate to a single derivation, but it evaluated to %d derivations",
+                    installable->what(), drvs.size());
+
+            auto & drvPath = *drvs.begin();
+
+            return getDerivationEnvironment(store, getEvalStore(), drvPath);
+        }
+    }
+
+    std::string makeRcScript(
+        ref<Store> store,
+        const BuildEnvironment & buildEnvironment,
+        const Path & outputsDir = absPath(".") + "/outputs")
+    {
+        // A list of colon-separated environment variables that should be
+        // prepended to, rather than overwritten, in order to keep the shell usable.
+        // Please keep this list minimal in order to avoid impurities.
+        static const char * const savedVars[] = {
+            "PATH",          // for commands
+            "XDG_DATA_DIRS", // for loadable completion
+        };
+
+        std::ostringstream out;
+
+        out << "unset shellHook\n";
+
+        for (auto & var : savedVars) {
+            out << fmt("%s=${%s:-}\n", var, var);
+            out << fmt("nix_saved_%s=\"$%s\"\n", var, var);
+        }
+
+        buildEnvironment.toBash(out, ignoreVars);
+
+        for (auto & var : savedVars)
+            out << fmt("%s=\"$%s${nix_saved_%s:+:$nix_saved_%s}\"\n", var, var, var, var);
+
+        out << "export NIX_BUILD_TOP=\"$(mktemp -d -t nix-shell.XXXXXX)\"\n";
+        for (auto & i : {"TMP", "TMPDIR", "TEMP", "TEMPDIR"})
+            out << fmt("export %s=\"$NIX_BUILD_TOP\"\n", i);
+
+        out << "eval \"$shellHook\"\n";
+
+        auto script = out.str();
+
+        /* Substitute occurrences of output paths. */
+        auto outputs = buildEnvironment.vars.find("outputs");
+        assert(outputs != buildEnvironment.vars.end());
+
+        // FIXME: properly unquote 'outputs'.
+        StringMap rewrites;
+        for (auto & outputName : BuildEnvironment::getStrings(outputs->second)) {
+            auto from = buildEnvironment.vars.find(outputName);
+            assert(from != buildEnvironment.vars.end());
+            // FIXME: unquote
+            rewrites.insert({BuildEnvironment::getString(from->second), outputsDir + "/" + outputName});
+        }
+
+        /* Substitute redirects. */
+        for (auto & [installable_, dir_] : redirects) {
+            auto dir = absPath(dir_);
+            auto installable = parseInstallable(store, installable_);
+            auto builtPaths = Installable::toStorePaths(
+                getEvalStore(), store, Realise::Nothing, OperateOn::Output, {installable});
+            for (auto & path: builtPaths) {
+                auto from = store->printStorePath(path);
+                if (script.find(from) == std::string::npos)
+                    warn("'%s' (path '%s') is not used by this build environment", installable->what(), from);
+                else {
+                    printInfo("redirecting '%s' to '%s'", from, dir);
+                    rewrites.insert({from, dir});
+                }
+            }
+        }
+
+        return rewriteStrings(script, rewrites);
+    }
 };

 static auto rCmdPrintDevEnv = registerCommand<CmdPrintDevEnv>("print-dev-env");

@Ericson2314

Priorities

Add 👍 to issues you find important.

@manveru manveru added the bug label May 9, 2023
@dermetfan
Copy link
Member

dermetfan commented May 9, 2023

doesn't work on derivations and store paths anymore. It seems to require a flake reference.

Sounds like this could be the same bug that causes #8284.

EDIT: #8310 but did not fix it so it's a different bug.

Ericson2314 added a commit to obsidiansystems/nix that referenced this issue May 9, 2023
Fixes NixOS#8309

This regression was because both `CmdDevelop` and `CmdPrintDevEnv` were
switched to be `InstallableValueCommand` subclasses, but only
`CmdDevelop` should have been.
@Ericson2314
Copy link
Member

Try out #8310

Ericson2314 added a commit to obsidiansystems/nix that referenced this issue May 10, 2023
Fixes NixOS#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.
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue May 10, 2023
Fixes NixOS#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.
github-actions bot pushed a commit that referenced this issue May 11, 2023
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.

(cherry picked from commit a93110a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants