cachix / devenv

Fast, Declarative, Reproducible, and Composable Developer Environments
https://devenv.sh
Apache License 2.0
3.56k stars 259 forks source link

feat: use mkNakedShell from devshell for less noisy shell activation #105

Closed thenonameguy closed 1 year ago

thenonameguy commented 1 year ago

Fixes #79

After this PR, this kind of interface is trivial:

nix-tree $(./result/bin/devenv input-derivation)

With a simple change like:

+    input-derivation)
+      assemble
+      echo $($CUSTOM_NIX/bin/nix $NIX_FLAGS build --no-link --print-out-paths '.#devShell.${pkgs.system}.inputDerivation' --impure)
+      ;;

This can be used for any kind of binary substitution push (adding a convenient cachix layer on top is left as an exercise to the reader :wink: )

thenonameguy commented 1 year ago

devenv print-dev-env has regressed with this change Missing: +DEVENV_DOTFILE +DEVENV_ROOT +DEVENV_STATE env vars. Investigating.

thenonameguy commented 1 year ago

@domenkozar ready for review :pray:

domenkozar commented 1 year ago

I'm not a huge fan of using buildEnv because that tends to create file collisions. pkgs.mkShell uses just a list for packages.

cc @zimbatm, how do you want to handle copyright for this?

domenkozar commented 1 year ago

I think I screwed up the merge conflict and there's an indentation issue now.

domenkozar commented 1 year ago

@thenonameguy I'll take it from here, thanks!

zimbatm commented 1 year ago

License is MIT so attribution please

thenonameguy commented 1 year ago

Can I maybe help here with the PR? @domenkozar I can gladly fix the merge conflict + do the licensing attribution.

https://github.com/cachix/devenv/pull/105#issuecomment-1323452982 wrt buildEnv, I haven't personally had issues with conflicts with my devshell usage of it. Is there any alternative you had in mind? It seems to me given NixOS/home-manager usage that this is the proper way to construct a full environment.

domenkozar commented 1 year ago

That would be much appreciated!

I'm fine if we just pass ignoreCollisions = true; to buildEnv for now :)

thenonameguy commented 1 year ago

@domenkozar ready for review :pray:

domenkozar commented 1 year ago

I think I'd like to revert this until we figure out how to support system libraries properly.

For example setting LD_LIBRARY_PATH, PKGCONFIG_PATH, etc.

thenonameguy commented 1 year ago

I think I'd like to revert this until we figure out how to support system libraries properly.

For example setting LD_LIBRARY_PATH, PKGCONFIG_PATH, etc.

Sure! I'll take a look at it later. Can you add a test for an example for the expected env vars?

domenkozar commented 1 year ago

If you for example add pkgs.imagemagick6 to packages, then PKG_CONFIG_PATH should be set accordingly so that when you compile a ruby pckage for example it can link against it.

thenonameguy commented 1 year ago

$ git rev-parse HEAD
2316585d14b256a77044ebef5d3e61a14339eecd

$ env | grep PKG_CONFIG

``` @domenkozar for reference the current head branch does not fulfill this test AFAICT. Nor does doing `nix-shell -p imagemagick6` nor in `devshell`. I'll look for a different example. EDIT: I had to do `nix-shell -p pkg-config imagemagick6` to get a set `PKG_CONFIG_PATH_FOR_TARGET` With the current `main` commit you can do: ``` [kszabo@paks3:~/projects/devenv]$ git diff diff --git a/devenv.nix b/devenv.nix index bcc27f6..1127134 100644 --- a/devenv.nix +++ b/devenv.nix @@ -6,6 +6,8 @@ pkgs.python3Packages.virtualenv pkgs.python3Packages.cairocffi pkgs.yaml2json + pkgs.pkg-config + pkgs.imagemagick6 ]; # bin/mkdocs serve --config-file mkdocs.insiders.yml [kszabo@paks3:~/projects/devenv]$ echo $PKG_CONFIG_PATH /nix/store/zdba9frlxj2ba8ca095win3nphsiiqhb-python3-3.10.8/lib/pkgconfig: ``` `imagemagick6` does not work by itself either. [Probably suboptimal experience for the users.](https://github.com/NixOS/nixpkgs/issues/64530#issuecomment-1030313034)
thenonameguy commented 1 year ago

After spending too much time understanding how derivation -> stdenvNoCC.mkDerivation -> stdenv.mkDerivation -> stdenv.mkShell differ, I am somewhat conflicted on how to proceed.

  1. the setup-hook mechanism of the nixpkgs stdenv bash script is used to construct extensions to the runtime environment of the installed derivation.
  2. we are (ab-)using this on the current main commit via mkShell, since the script is written for managing a nix build environment, with all the cruft around it (hence the many env vars).

Now dropping all that sweet setup-hook.sh logic / re-implementing seems like a ton of work. Sadly, there are tons of them for multiple packages and they depend on a huge set of the 'nixpkgs bash standard lib' functions, which we don't get with a naked derivation.

devshell suffers from the same problem (as we inherited the code from it), although this hasn't been mentioned yet in the issue tracker. It seems it is not a high priority problem for most users. After scouring GitHub I found just a few examples solving this issue. For the remainder of the users, the environment clarity benefits + faster evaluation/build time should outweigh the pkg-config/ld_library_path issues.

The 2 options I see going forward:

  1. keep using this PR's approach and extend devenv with modules that do these environment variable patches explicitly
  2. go with mkShell and extend the shellHook to unset all unrelated env vars at the end. This will still run all the setup hooks though, which increases devenv shell eval time.
domenkozar commented 1 year ago

I'm happy with having 2) today and we can solve eval time at the source of origin in nixpkgs.

thenonameguy commented 1 year ago

The main problem with 2) is that you still have to add 'pkgs.pkg-config' to get the setup-hook to run = suprised non-technical nix users.

1) with shell activation time env var prepends to 'PKG_CONFIG_PATH' has the best DX. It is more cumbersome to maintain though, but I would be open do it.

I'll propose a PR in the evening.

thenonameguy commented 1 year ago

Additional note on the test while trying to test between main and this branch:

➜  tmp nix-shell -p ruby pkg-config imagemagick6

[nix-shell:~/tmp]$ gem install rmagick
Building native extensions. This could take a while...
ERROR:  Error installing rmagick:
    ERROR: Failed to build gem native extension.

    current directory: /home/kszabo/.local/share/gem/ruby/2.7.0/gems/rmagick-5.1.0/ext/RMagick
/nix/store/g2n4v2lhxgq5kvk8m4aqc79agjrajj57-ruby-2.7.6/bin/ruby -I /nix/store/g2n4v2lhxgq5kvk8m4aqc79agjrajj57-ruby-2.7.6/lib/ruby/2.7.0 extconf.rb
checking for brew... no
checking for pacman... no
checking for Ruby version >= 2.3.0... yes
*** extconf.rb failed ***
Could not create Makefile due to some reason, probably lack of necessary
libraries and/or headers.  Check the mkmf.log file for more details.  You may
need configuration options.

Provided configuration options:
    --with-opt-dir
    --without-opt-dir
    --with-opt-include
    --without-opt-include=${opt-dir}/include
    --with-opt-lib
    --without-opt-lib=${opt-dir}/lib
    --with-make-prog
    --without-make-prog
    --srcdir=.
    --curdir
    --ruby=/nix/store/g2n4v2lhxgq5kvk8m4aqc79agjrajj57-ruby-2.7.6/bin/$(RUBY_BASE_NAME)
    --with-pkg-config
    --without-pkg-config
    --with-override-variables
    --without-override-variables
/home/kszabo/.local/share/gem/ruby/2.7.0/gems/pkg-config-1.5.1/lib/pkg-config.rb:500:in `parse_pc': .pc doesn't exist: <MagickCore> (PackageConfig::NotFoundError)
    from /home/kszabo/.local/share/gem/ruby/2.7.0/gems/pkg-config-1.5.1/lib/pkg-config.rb:344:in `declaration'
    from /home/kszabo/.local/share/gem/ruby/2.7.0/gems/pkg-config-1.5.1/lib/pkg-config.rb:285:in `requires'
    from /home/kszabo/.local/share/gem/ruby/2.7.0/gems/pkg-config-1.5.1/lib/pkg-config.rb:541:in `block in required_packages'
    from /home/kszabo/.local/share/gem/ruby/2.7.0/gems/pkg-config-1.5.1/lib/pkg-config.rb:370:in `collect_requires'
    from /home/kszabo/.local/share/gem/ruby/2.7.0/gems/pkg-config-1.5.1/lib/pkg-config.rb:540:in `required_packages'
    from /home/kszabo/.local/share/gem/ruby/2.7.0/gems/pkg-config-1.5.1/lib/pkg-config.rb:448:in `collect_libs'
    from /home/kszabo/.local/share/gem/ruby/2.7.0/gems/pkg-config-1.5.1/lib/pkg-config.rb:306:in `libs'
    from /home/kszabo/.local/share/gem/ruby/2.7.0/gems/pkg-config-1.5.1/lib/pkg-config.rb:587:in `libs'
    from extconf.rb:340:in `assert_has_dev_libs!'
    from extconf.rb:317:in `assert_can_compile!'
    from extconf.rb:46:in `initialize'
    from extconf.rb:441:in `new'
    from extconf.rb:441:in `<main>'

To see why this extension failed to compile, please check the mkmf.log which can be found here:

  /home/kszabo/.local/share/gem/ruby/2.7.0/extensions/x86_64-linux/2.7.0/rmagick-5.1.0/mkmf.log

extconf failed, exit code 1

Gem files will remain installed in /home/kszabo/.local/share/gem/ruby/2.7.0/gems/rmagick-5.1.0 for inspection.
Results logged to /home/kszabo/.local/share/gem/ruby/2.7.0/extensions/x86_64-linux/2.7.0/rmagick-5.1.0/gem_make.out

[nix-shell:~/tmp]$ export PKG_CONFIG_PATH=$PKG_CONFIG_PATH_FOR_TARGET

[nix-shell:~/tmp]$ gem install rmagick
Building native extensions. This could take a while...
Successfully installed rmagick-5.1.0
Parsing documentation for rmagick-5.1.0
Done installing documentation for rmagick after 0 seconds
1 gem installed

The setup-hook provided environment variables are not enough to be able to compile the native extensions for rmagick (since the PKG_CONFIG_PATH_FOR_TARGET assume a newer/nix-specific pkg-config with cross-compilation support). So currently the main branch does not fulfill the test criteria. This means the revert was probably made unnecessarily (at least for Ruby folks).