From 9ad7db6164b791f98efec0487c955b3dca0f0802 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 18 Dec 2022 18:15:44 +0100 Subject: [PATCH 1/3] POC: devShell interface The idea here is that future Nix first tries to "`nix run`" the `pkg.devShell` package, and only if that fails, fall back to the legacy `nix develop` (or `nix-shell`) behavior. This allows the development shell to evolve with stdenv, and it allows packages to individually customize the `devShell` attribute, by setting `passthru.devShell`. Furthermore, these shell behaviors will be pinned to the expressions, allowing changes to be made in a more agile manner, unlike Nix, which has to be very careful not to break old expressions, as users can not revert Nix. To give it a try: nix run .#hello.devShell In the future this will be equivalent to: nix develop .#hello Isn't this the responsibility of Nix? It is not. `nix-shell` and `nix develop` are a great user interface, that everyone loves, but their implementation is a pile of hacks on top of stdenv. Instead of coercing stdenv to do what `nix-shell` needs it to, we can ask stdenv politely to provide a shell. Now that Nix doesn't have to assume a package comes from stdenv, there's a possibility for experimental builders to provide shells too. What does this break? Only packages that define a `devShell` attribute (for some reason?) have to adapt to the suggested new Nix behavior. Note that a `devShell` value for the builder can be overridden by `passthru` without affecting the build or shell. Packages and shells pinned to older versions can still be loaded because Nix keeps the legacy behavior as a fallback. But this still relies on `nix-shell` to provide a shell??? Fair enough. This is only a proof of concept. The goal is to replace that invocation by a script or program with the same or better behavior, without relying on `nix-shell` as its implementation. Does this solve the need for `.env` for Haskell package shells? Not in this commit, but Haskell packages will be able to produce their own `devShell` attribute, which is derived from the .env derivation rather than the regular derivation. Refs - https://github.com/NixOS/nix/issues/7468 and a bunch of other issues where I've preached about this idea. --- pkgs/stdenv/adapters.nix | 2 +- pkgs/stdenv/cross/default.nix | 3 +++ pkgs/stdenv/default.nix | 1 + pkgs/stdenv/devShell.nix | 23 +++++++++++++++++++++++ pkgs/stdenv/generic/default.nix | 5 ++++- pkgs/stdenv/generic/make-derivation.nix | 15 +++++++++++++-- pkgs/stdenv/linux/default.nix | 3 +++ pkgs/top-level/all-packages.nix | 4 ++++ pkgs/top-level/default.nix | 1 + 9 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 pkgs/stdenv/devShell.nix diff --git a/pkgs/stdenv/adapters.nix b/pkgs/stdenv/adapters.nix index 85bd8d2087f66..29eb73f55f5f6 100644 --- a/pkgs/stdenv/adapters.nix +++ b/pkgs/stdenv/adapters.nix @@ -6,7 +6,7 @@ let # N.B. Keep in sync with default arg for stdenv/generic. - defaultMkDerivationFromStdenv = import ./generic/make-derivation.nix { inherit lib config; }; + defaultMkDerivationFromStdenv = import ./generic/make-derivation.nix { inherit lib config; inherit (pkgs) defaultDevShell; }; # Low level function to help with overriding `mkDerivationFromStdenv`. One # gives it the old stdenv arguments and a "continuation" function, and diff --git a/pkgs/stdenv/cross/default.nix b/pkgs/stdenv/cross/default.nix index bf410ec0a6841..2f1c909c94e28 100644 --- a/pkgs/stdenv/cross/default.nix +++ b/pkgs/stdenv/cross/default.nix @@ -1,5 +1,6 @@ { lib , localSystem, crossSystem, config, overlays, crossOverlays ? [] +, defaultDevShell }: let @@ -11,6 +12,8 @@ let # Ignore custom stdenvs when cross compiling for compatability config = builtins.removeAttrs config [ "replaceStdenv" ]; + + inherit defaultDevShell; }; in lib.init bootStages ++ [ diff --git a/pkgs/stdenv/default.nix b/pkgs/stdenv/default.nix index 7a2ad665e09d7..521ea9f585c1d 100644 --- a/pkgs/stdenv/default.nix +++ b/pkgs/stdenv/default.nix @@ -8,6 +8,7 @@ lib # Args to pass on to the pkgset builder, too , localSystem, crossSystem, config, overlays, crossOverlays ? [] +, defaultDevShell } @ args: let diff --git a/pkgs/stdenv/devShell.nix b/pkgs/stdenv/devShell.nix new file mode 100644 index 0000000000000..8b233c8546a9c --- /dev/null +++ b/pkgs/stdenv/devShell.nix @@ -0,0 +1,23 @@ +{ pkgs }: +{ drv, ... }: + +# stdenvDevShell only supports simple derivations, not generalized packages or anything else. +assert drv?drvPath && drv.type or null == "derivation"; + +# TODO: fork and improve nix-shell logic +(pkgs.buildPackages.writeScriptBin "devShell" '' + #!${pkgs.buildPackages.runtimeShell} + ${pkgs.buildPackages.nix}/bin/nix-shell ${builtins.unsafeDiscardOutputDependency drv.drvPath} +'').overrideAttrs (finalAttrs: prevAttrs: { + passthru = prevAttrs.passthru or {} // { + shellData = pkgs.runCommandLocal "devShell-data" { + # inputDerivation can only produce one file without breaking back compat + # hence, we only use its implementation and improve its interface. + # TODO: expose the structured attrs files? + inherit (drv) inputDerivation; + } '' + mkdir $out + cp $inputDerivation $out/environment.sh + ''; + }; +}) diff --git a/pkgs/stdenv/generic/default.nix b/pkgs/stdenv/generic/default.nix index 81255726284b5..0ff8c8eac7cc8 100644 --- a/pkgs/stdenv/generic/default.nix +++ b/pkgs/stdenv/generic/default.nix @@ -52,7 +52,10 @@ argsStdenv@{ name ? "stdenv", preHook ? "", initialPath , # The implementation of `mkDerivation`, parameterized with the final stdenv so we can tie the knot. # This is convient to have as a parameter so the stdenv "adapters" work better - mkDerivationFromStdenv ? import ./make-derivation.nix { inherit lib config; } + mkDerivationFromStdenv ? import ./make-derivation.nix { inherit lib config defaultDevShell; } + +, # See ./make-derivation.nix + defaultDevShell ? _: null }: let diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix index 510537aac9f39..b2fa9c6388a78 100644 --- a/pkgs/stdenv/generic/make-derivation.nix +++ b/pkgs/stdenv/generic/make-derivation.nix @@ -1,4 +1,8 @@ -{ lib, config }: +{ lib, + config, + # Function to construct the default devShell attribute; when not set via passthru. + defaultDevShell ? _: null, +}: stdenv: @@ -473,6 +477,8 @@ else let else true); }; + drv = derivation derivationArg; + in lib.extendDerivation @@ -503,13 +509,18 @@ lib.extendDerivation args = [ "-c" "export > $out" ]; }); + devShell = defaultDevShell { + # TODO bring `finalPackage` into scope here. + drv = overrideAttrs (o: {}); + }; + inherit meta passthru overrideAttrs; } // # Pass through extra attributes that are not inputs, but # should be made available to Nix expressions using the # derivation (e.g., in assertions). passthru) - (derivation derivationArg); + drv; in fnOrAttrs: diff --git a/pkgs/stdenv/linux/default.nix b/pkgs/stdenv/linux/default.nix index dbaff342fb1af..9b6153a2c04a9 100644 --- a/pkgs/stdenv/linux/default.nix +++ b/pkgs/stdenv/linux/default.nix @@ -40,6 +40,7 @@ files = archLookupTable.${localSystem.system} or (if getCompatibleTools != null then getCompatibleTools else (abort "unsupported platform for the pure Linux stdenv")); in files +, defaultDevShell }: assert crossSystem == localSystem; @@ -461,6 +462,8 @@ in inherit (prevStage) binutils binutils-unwrapped; gcc = cc; }; + + inherit defaultDevShell; }; }) diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix index 4b906f8c5f42f..76cd2cf98112d 100644 --- a/pkgs/top-level/all-packages.nix +++ b/pkgs/top-level/all-packages.nix @@ -112,6 +112,10 @@ with pkgs; tests = callPackages ../test {}; + stdenvDevShell = import ../stdenv/devShell.nix { inherit pkgs; }; + + defaultDevShell = stdenvDevShell; + ### Nixpkgs maintainer tools nix-generate-from-cpan = callPackage ../../maintainers/scripts/nix-generate-from-cpan.nix { }; diff --git a/pkgs/top-level/default.nix b/pkgs/top-level/default.nix index b32aabc3a1fdf..a1e8378d8168c 100644 --- a/pkgs/top-level/default.nix +++ b/pkgs/top-level/default.nix @@ -124,6 +124,7 @@ in let stages = stdenvStages { inherit lib localSystem crossSystem config overlays crossOverlays; + inherit (pkgs) defaultDevShell; }; pkgs = boot stages; From 0999d997a1e3bc0bdd9530f8bd6efb92c43d1dee Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 18 Dec 2022 20:06:52 +0100 Subject: [PATCH 2/3] Add devShell attribute to darwin too --- pkgs/stdenv/darwin/default.nix | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkgs/stdenv/darwin/default.nix b/pkgs/stdenv/darwin/default.nix index 9a7cd9aa9dee5..a9f2c420d1b77 100644 --- a/pkgs/stdenv/darwin/default.nix +++ b/pkgs/stdenv/darwin/default.nix @@ -34,6 +34,7 @@ cpio = fetch { file = "cpio"; sha256 = "sha256-SWkwvLaFyV44kLKL2nx720SvcL4ej/p2V/bX3uqAGO0="; }; tarball = fetch { file = "bootstrap-tools.cpio.bz2"; sha256 = "sha256-kRC/bhCmlD4L7KAvJQgcukk7AinkMz4IwmG1rqlh5tA="; executable = false; }; } +, defaultDevShell }: assert crossSystem == localSystem; @@ -149,7 +150,7 @@ rec { thisStdenv = import ../generic { name = "${name}-stdenv-darwin"; - inherit config shell extraBuildInputs; + inherit config shell extraBuildInputs defaultDevShell; extraNativeBuildInputs = extraNativeBuildInputs ++ lib.optionals doUpdateAutoTools [ last.pkgs.updateAutotoolsGnuConfigScriptsHook @@ -668,7 +669,7 @@ rec { import ../generic rec { name = "stdenv-darwin"; - inherit config; + inherit config defaultDevShell; inherit (pkgs.stdenv) fetchurlBoot; buildPlatform = localSystem; From 628563aa57633027542c13cd4f3c1a8a2c064fe7 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 24 Dec 2022 15:23:39 +0100 Subject: [PATCH 3/3] Flatten the devShell derivation --- pkgs/stdenv/devShell.nix | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/pkgs/stdenv/devShell.nix b/pkgs/stdenv/devShell.nix index 8b233c8546a9c..4cbaf1751253e 100644 --- a/pkgs/stdenv/devShell.nix +++ b/pkgs/stdenv/devShell.nix @@ -7,17 +7,24 @@ assert drv?drvPath && drv.type or null == "derivation"; # TODO: fork and improve nix-shell logic (pkgs.buildPackages.writeScriptBin "devShell" '' #!${pkgs.buildPackages.runtimeShell} - ${pkgs.buildPackages.nix}/bin/nix-shell ${builtins.unsafeDiscardOutputDependency drv.drvPath} -'').overrideAttrs (finalAttrs: prevAttrs: { - passthru = prevAttrs.passthru or {} // { - shellData = pkgs.runCommandLocal "devShell-data" { - # inputDerivation can only produce one file without breaking back compat - # hence, we only use its implementation and improve its interface. - # TODO: expose the structured attrs files? - inherit (drv) inputDerivation; - } '' - mkdir $out - cp $inputDerivation $out/environment.sh - ''; - }; + + # TODO replicate in bash + exec ${pkgs.buildPackages.nix}/bin/nix-shell ${builtins.unsafeDiscardOutputDependency drv.drvPath} + +'').overrideAttrs (prevAttrs: { + buildCommand = '' + # Write the bin/devShell command + ${prevAttrs.buildCommand} + mkdir -p $out + + # environment.sh exposes the environment, but no bash functions + cp $inputDerivation $out/environment.sh + + # TODO implement this and use it in the devShell command + # should this have a different name? + # setup.sh loads the derivation-like environment variables and stdenv/setup.sh + echo '# TODO' >$out/setup.sh + ''; + # TODO not sure if these must be equal; may want to reimplement this, maybe? + inherit (drv) inputDerivation; })