feat(shell): revert from naked shell to pkgs.mkShell + unsetting irrelevant env vars

[thenonameguy]

closes https://github.com/cachix/devenv/pull/498 fixes https://github.com/cachix/devenv/issues/497

[aaronmondal]

Also fixes https://github.com/cachix/devenv/issues/486.

[aaronmondal]

Ok I tested this against the flake in rules_ll and things seem to work fine. This change of course leads to more environment variables being set than before, but at least we know that it won't break existing more "exotic" C++ toolchains 😊

[domenkozar]

It seems that Nim is broken in nixpkgs, not here: https://hydra.nixos.org/build/213226629

Possible fix https://github.com/NixOS/nixpkgs/pull/222285

[thenonameguy]

It seems that Nim is broken in nixpkgs, not here: https://hydra.nixos.org/build/213226629

Possible fix NixOS/nixpkgs#222285

Thanks, I removed the irrelevant change.

[sandangel]

Also my neovim could not find tsserver installed by languages.typescript.enable = true;. It was working when I use the standard mkShell.

[hurricanehrndz]

Although this patch addresses a lot of issues I am still seeing some issues on nix-darwin, specifically with Foundation, maybe an issue with nativebuildinputs not propagating.

These vars seem to fix build, most of them are available though through this patch. So not sure:

export NIX_CFLAGS_COMPILE=' -frandom-seed=jbm4rgq3l4 -isystem /nix/store/1wamlhkl3q15b7pdim85vqkmz053fwlg-libcxx-11.1.0-dev/include -isystem /nix/store/21knak232h33cha4a16k6gzgai23c0ss-libcxxabi-11.1.0-dev/include -isystem /nix/store/h2hd63vq9js32ggqx9nvhsjqx4b2ckss-compiler-rt-libc-11.1.0-dev/include -iframework /nix/store/f7p955pdwhgirijk33mash7rmlkzklkx-apple-framework-Foundation-11.0.0/Library/Frameworks -iframework /nix/store/z7n2ibxkj7fsggcfgy55mvzxwxsc2h96-apple-framework-ApplicationServices-11.0.0/Library/Frameworks -iframework /nix/store/qjy23mrisdwl3khmkz55mvinwjxmi7vr-apple-framework-ColorSync-11.0.0/Library/Frameworks -iframework /nix/store/4al6fm8vlmv3v7749sfqvj34vnhsfbsq-apple-framework-CoreGraphics-11.0.0/Library/Frameworks -iframework /nix/store/sx4lwrgyay49ynkpp2pq71s0l40mj1f9-apple-framework-Accelerate-11.0.0/Library/Frameworks -iframework /nix/store/d6q9q9gqrpbl799j70jk0rp3sff2andd-apple-framework-CoreWLAN-11.0.0/Library/Frameworks -iframework /nix/store/i62176fkd5yg2qbxrqb4gd1bbijzmh2m-apple-framework-SecurityFoundation-11.0.0/Library/Frameworks -iframework /nix/store/q57wwyd23i6msdawg6casxc3cgvxdsaa-apple-framework-Security-11.0.0/Library/Frameworks -iframework /nix/store/5ncnv0kgz3fzf3bnq5bl35lw18yvw706-apple-framework-IOKit-11.0.0/Library/Frameworks -isystem /nix/store/7p8qgl96a2grvwaaxnpw678m4f99l86v-apple-lib-libDER/include -iframework /nix/store/n8bxvnxzfwv5qi55m8arqmq6i6ih3bcf-apple-framework-IOBluetooth-11.0.0/Library/Frameworks -iframework /nix/store/g6h0632636m2r38hkm16gsl459hdlm4b-apple-framework-CoreBluetooth-11.0.0/Library/Frameworks -iframework /nix/store/sllbiczqxnlg732nj0sn21b1rgsac5mg-apple-framework-IOSurface-11.0.0/Library/Frameworks -iframework /nix/store/5ij3n8sd1wkjsfayidkg2yxpm2b40jw5-apple-framework-SystemConfiguration-11.0.0/Library/Frameworks -iframework /nix/store/fkd0lx594kgns98p53n6p3hj01isjsal-apple-framework-CoreServices-11.0.0/Library/Frameworks -iframework /nix/store/hagvvvlp3cw03xx794sw3q4m2dz450zj-apple-framework-CFNetwork-11.0.0/Library/Frameworks -iframework /nix/store/656hkg3497rx0wcssd895k4pvwc4jzag-apple-framework-CoreAudio-11.0.0/Library/Frameworks -iframework /nix/store/33w6vyz4gd3idqg5qy2lmpkn2l8hbb7i-apple-framework-CoreAudioTypes-11.0.0/Library/Frameworks -iframework /nix/store/45dl40acmas1wkh2kva1db5bry1rwszs-apple-framework-CoreData-11.0.0/Library/Frameworks -iframework /nix/store/90pcbb199w35fw42lbk6y4cc61lmmy15-apple-framework-CloudKit-11.0.0/Library/Frameworks -iframework /nix/store/hzjxfgdxac2bvmkw40bsyxm3kh73kl70-apple-framework-CoreLocation-11.0.0/Library/Frameworks -iframework /nix/store/scz844iwh8nw8vcc2sr3n5vkkya24byi-apple-framework-CoreFoundation-11.0.0/Library/Frameworks -isystem /nix/store/3pd5fbv4did9ya9mjbcj25rfmdk021sm-libobjc-11.0.0/include -iframework /nix/store/byzgp03rhiybvnr9n42i8b4931kaqchb-apple-framework-DiskArbitration-11.0.0/Library/Frameworks -iframework /nix/store/hxrcjxyvcxzpd8k6v8fyqx6c9y89nkfk-apple-framework-NetFS-11.0.0/Library/Frameworks -iframework /nix/store/2hl6k0mx0bl167kf4ggjaz0pcxbsz1bi-apple-framework-OpenDirectory-11.0.0/Library/Frameworks -iframework /nix/store/p0dyxxz38dr7iy9a9q71cxkahpdafz2a-apple-framework-ServiceManagement-11.0.0/Library/Frameworks -iframework /nix/store/nrlivsnsj6b90frl21gsbxc72fc8j6h9-apple-framework-CoreText-11.0.0/Library/Frameworks -iframework /nix/store/swhsihvli1wfa0xm975vzx7diz3kfv34-apple-framework-ImageIO-11.0.0/Library/Frameworks -iframework /nix/store/8ar7mqa5hqdw2xrw6a0i7p272rkvlgg1-apple-framework-Combine-11.0.0/Library/Frameworks -iframework /nix/store/21fn46279sjpqqqqsy2cnkxr3rqzbwq6-apple-framework-CoreFoundation-11.0.0/Library/Frameworks -isystem /nix/store/1wamlhkl3q15b7pdim85vqkmz053fwlg-libcxx-11.1.0-dev/include -isystem /nix/store/21knak232h33cha4a16k6gzgai23c0ss-libcxxabi-11.1.0-dev/include -isystem /nix/store/h2hd63vq9js32ggqx9nvhsjqx4b2ckss-compiler-rt-libc-11.1.0-dev/include -iframework /nix/store/f7p955pdwhgirijk33mash7rmlkzklkx-apple-framework-Foundation-11.0.0/Library/Frameworks -iframework /nix/store/z7n2ibxkj7fsggcfgy55mvzxwxsc2h96-apple-framework-ApplicationServices-11.0.0/Library/Frameworks -iframework /nix/store/qjy23mrisdwl3khmkz55mvinwjxmi7vr-apple-framework-ColorSync-11.0.0/Library/Frameworks -iframework /nix/store/4al6fm8vlmv3v7749sfqvj34vnhsfbsq-apple-framework-CoreGraphics-11.0.0/Library/Frameworks -iframework /nix/store/sx4lwrgyay49ynkpp2pq71s0l40mj1f9-apple-framework-Accelerate-11.0.0/Library/Frameworks -iframework /nix/store/d6q9q9gqrpbl799j70jk0rp3sff2andd-apple-framework-CoreWLAN-11.0.0/Library/Frameworks -iframework /nix/store/i62176fkd5yg2qbxrqb4gd1bbijzmh2m-apple-framework-SecurityFoundation-11.0.0/Library/Frameworks -iframework /nix/store/q57wwyd23i6msdawg6casxc3cgvxdsaa-apple-framework-Security-11.0.0/Library/Frameworks -iframework /nix/store/5ncnv0kgz3fzf3bnq5bl35lw18yvw706-apple-framework-IOKit-11.0.0/Library/Frameworks -isystem /nix/store/7p8qgl96a2grvwaaxnpw678m4f99l86v-apple-lib-libDER/include -iframework /nix/store/n8bxvnxzfwv5qi55m8arqmq6i6ih3bcf-apple-framework-IOBluetooth-11.0.0/Library/Frameworks -iframework /nix/store/g6h0632636m2r38hkm16gsl459hdlm4b-apple-framework-CoreBluetooth-11.0.0/Library/Frameworks -iframework /nix/store/sllbiczqxnlg732nj0sn21b1rgsac5mg-apple-framework-IOSurface-11.0.0/Library/Frameworks -iframework /nix/store/5ij3n8sd1wkjsfayidkg2yxpm2b40jw5-apple-framework-SystemConfiguration-11.0.0/Library/Frameworks -iframework /nix/store/fkd0lx594kgns98p53n6p3hj01isjsal-apple-framework-CoreServices-11.0.0/Library/Frameworks -iframework /nix/store/hagvvvlp3cw03xx794sw3q4m2dz450zj-apple-framework-CFNetwork-11.0.0/Library/Frameworks -iframework /nix/store/656hkg3497rx0wcssd895k4pvwc4jzag-apple-framework-CoreAudio-11.0.0/Library/Frameworks -iframework /nix/store/33w6vyz4gd3idqg5qy2lmpkn2l8hbb7i-apple-framework-CoreAudioTypes-11.0.0/Library/Frameworks -iframework /nix/store/45dl40acmas1wkh2kva1db5bry1rwszs-apple-framework-CoreData-11.0.0/Library/Frameworks -iframework /nix/store/90pcbb199w35fw42lbk6y4cc61lmmy15-apple-framework-CloudKit-11.0.0/Library/Frameworks -iframework /nix/store/hzjxfgdxac2bvmkw40bsyxm3kh73kl70-apple-framework-CoreLocation-11.0.0/Library/Frameworks -iframework /nix/store/scz844iwh8nw8vcc2sr3n5vkkya24byi-apple-framework-CoreFoundation-11.0.0/Library/Frameworks -isystem /nix/store/3pd5fbv4did9ya9mjbcj25rfmdk021sm-libobjc-11.0.0/include -iframework /nix/store/byzgp03rhiybvnr9n42i8b4931kaqchb-apple-framework-DiskArbitration-11.0.0/Library/Frameworks -iframework /nix/store/hxrcjxyvcxzpd8k6v8fyqx6c9y89nkfk-apple-framework-NetFS-11.0.0/Library/Frameworks -iframework /nix/store/2hl6k0mx0bl167kf4ggjaz0pcxbsz1bi-apple-framework-OpenDirectory-11.0.0/Library/Frameworks -iframework /nix/store/p0dyxxz38dr7iy9a9q71cxkahpdafz2a-apple-framework-ServiceManagement-11.0.0/Library/Frameworks -iframework /nix/store/nrlivsnsj6b90frl21gsbxc72fc8j6h9-apple-framework-CoreText-11.0.0/Library/Frameworks -iframework /nix/store/swhsihvli1wfa0xm975vzx7diz3kfv34-apple-framework-ImageIO-11.0.0/Library/Frameworks -iframework /nix/store/8ar7mqa5hqdw2xrw6a0i7p272rkvlgg1-apple-framework-Combine-11.0.0/Library/Frameworks -iframework /nix/store/21fn46279sjpqqqqsy2cnkxr3rqzbwq6-apple-framework-CoreFoundation-11.0.0/Library/Frameworks'
export NIX_LDFLAGS=' -L/nix/store/y56hqp2dgrh9hi9xyd88n0pcl6gn895d-libcxx-11.1.0/lib -L/nix/store/gs6iqzg3cswfsxwnwcvsnf6fn76wpbr2-libcxxabi-11.1.0/lib -L/nix/store/81jnzlpii91vz9y8rwb7g095zk2jj2qr-compiler-rt-libc-11.1.0/lib -L/nix/store/3pd5fbv4did9ya9mjbcj25rfmdk021sm-libobjc-11.0.0/lib -L/nix/store/y56hqp2dgrh9hi9xyd88n0pcl6gn895d-libcxx-11.1.0/lib -L/nix/store/gs6iqzg3cswfsxwnwcvsnf6fn76wpbr2-libcxxabi-11.1.0/lib -L/nix/store/81jnzlpii91vz9y8rwb7g095zk2jj2qr-compiler-rt-libc-11.1.0/lib -L/nix/store/3pd5fbv4did9ya9mjbcj25rfmdk021sm-libobjc-11.0.0/lib'
#export stdenv=/nix/store/5pq0sy4haiy4rb2chya6qq72w95ci2dp-stdenv-darwin
#export builder=/nix/store/ka6rabx4lz7m3habrjhh8hvbgxbz8r98-bash-5.2-p15/bin/bash
export NIX_CC=/nix/store/cy78am69kj3d2r286rd7wg0cv48gqa3z-clang-wrapper-11.1.0
export NIX_BINTOOLS_WRAPPER_TARGET_HOST_aarch64_apple_darwin=1
# export NIX_BINTOOLS=/nix/store/v3ks390idh1xqnh3r0zlkqc86pcy7lva-cctools-binutils-darwin-wrapper-973.0.1
#export NIX_DONT_SET_RPATH=1
#export LD_DYLD_PATH=/usr/lib/dyld
#export NIX_DONT_SET_RPATH_FOR_BUILD=1
export NIX_CC_WRAPPER_TARGET_HOST_aarch64_apple_darwin=1

[sandangel]

Hi, sorry may I ask is there any update on this PR?

[domenkozar]

I mainly would like people to test it thoroughly before it's merged.

[sandangel]

I think it will be easier for people to test if we merge after all the CI check passed. If there is any issue, we can create a PR to fix. WDYT?

[aaronmondal]

I was actually wondering: What are the benefits of unsetting all those environment variables? I didn't really get the "keep it lean" comment.

[sandangel]

Hi, how long do we expect people to test this out before merging?

[bobvanderlinden]

I've updated this branch in my fork: https://github.com/bobvanderlinden/devenv/tree/feat/stripped-shell

There will be some regressions when merging this. Question is whether that is a good thing (honestly do not know).

With mkNamedShell, LD_LIBRARY_PATH was set based on packages. This is not the case anymore with mkShell. This is partly a good thing, as I think potentially 'injecting' many libraries using LD_LIBRARY_PATH will cause problems like https://github.com/cachix/devenv/issues/555.

This is why the python-poetry example needed a change:

https://github.com/bobvanderlinden/devenv/commit/b669f22a471e06fd356e04b5b63cbb2da5fe5e2e#diff-3ea46e5d7bf8933097511b155be31bf25b2c6e01081c6d9a6dfe61624717082aR4

Question is whether setting LD_LIBRARY_PATH inside mkShell should now be added again.

Maybe as an alternative we should introduce a different option. Something like runtimeLibraries = [ pkgs.zlib ] instead of packages = [ pkgs.zlib ], which is meant to solely set LD_LIBRARY_PATH and DYLD_LIBRARY_PATH. People can still use it, but need to do so more explicitly.

The real solution is for people to not rely on pre-built binaries, but I think that's the world we live in right now. devenv is a good middleground for that.

[aaronmondal]

LD_LIBRARY_PATH should not be set at all if possible. I think the linked python-poetry change is desirable.

Maybe as an alternative we should introduce a different option. Something like runtimeLibraries = [ pkgs.zlib ] instead of packages = [ pkgs.zlib ], which is meant to solely set LD_LIBRARY_PATH and DYLD_LIBRARY_PATH. People can still use it, but need to do so more explicitly.

IMO it's risky to let users unknowningly manipulate these variables, but if it's explicit it could be a viable option. Though I wonder whether such an option is actually necessary. Personally, I think I'd prefer explicitly setting LD_LIBRARY_PATH="${somepackage}/lib/" manually or do something similar to what you did in python-poetry.

[dzmitry-lahoda]

  pkgs.mkShell {
    buildInputs = with pkgs; [
        openssl
    ];
    nativeBuildInputs = with pkgs; [
        pkg-config
    ];
  }

is it possible now to add openssl dev deps?

[bobvanderlinden]

is it possible now to add openssl dev deps?

Yes, the packages are added as nativeBuildInputs.

See https://github.com/NixOS/nixpkgs/blob/3ed18a352739221ebaba0d11c38a2158faa29887/pkgs/build-support/mkshell/default.nix#L37

Personally, I think I'd prefer explicitly setting LD_LIBRARY_PATH="${somepackage}/lib/" manually or do something similar to what you did in python-poetry.

There are people already relying on just adding runtime libraries as packages. Having a separate option for runtimeLibraries will make it a bit clearer how to migrate. It also allows to document what it does and why it is a bad idea to use it. The specific use-cases where it's the only option is also good to mention in the docs.

I agree it's a bad idea in general, but shipping binaries is the underlying problem. That's sometimes quite hard to get around when using ruby/python/node package managers.

[asymmetric]

This seems like the right thing to do. Setting LD_LIBRARY_PATH comes with troubling consequences, which are additionally quite hard to debug unless one knows already what's going on.

[domenkozar]

I've done testing today on examples on top of #745 and python-poetry example fails with

+ poetry run python -c 'import numpy'
Traceback (most recent call last):
  File "/tmp/python-poetry_nj6_mkml/.venv/lib/python3.10/site-packages/numpy/core/__init__.py", line 23, in <module>
    from . import multiarray
  File "/tmp/python-poetry_nj6_mkml/.venv/lib/python3.10/site-packages/numpy/core/multiarray.py", line 10, in <module>
    from . import overrides
  File "/tmp/python-poetry_nj6_mkml/.venv/lib/python3.10/site-packages/numpy/core/overrides.py", line 6, in <module>
    from numpy.core._multiarray_umath import (
ImportError: libz.so.1: cannot open shared object file: No such file or directory

We'll need to fix this one, any ideas? I tried adding pkgconfig to the deps and explicilty setting zlib.dev as a package.

[domenkozar]

I've merged this into #745, but we'll need to fix the errors before it can be merged.

[thenonameguy]

Great, thanks for bringing this forward. Closing this PR.

[domenkozar]

Thank you for making this happen :heart:

[cameronraysmith]

https://github.com/cachix/devenv/pull/745#issuecomment-1846536413