cachix / devenv

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

Bloated environment variables with 1.0 #1084

Open martinjlowm opened 1 month ago

martinjlowm commented 1 month ago

Describe the bug With the introduction of https://github.com/cachix/devenv/commit/cf9d1229b814ac8d6aa2014a0ed9d6cd8d09e111 the new way of setting up the development shell introduces a lot of new variables, ones that were not there with the naked shell. I'm seeing 5x times the amount of variables being set, some of which are compiler-related variables, AR, CC, CXX etc. which forces which compiler anything in the project uses.

That same commit introduced unsetEnvVars, but that seems like a lot of work to maintain (which is actually only used if you don't override enterShell).

I'm sure at least some of the changes were intentional, but at least this part is a worse user experience than what we had before.

To reproduce

Pin the Flake input to a commit newer than the aforementioned, e.g. 19098329b4e79f60705d7fb241a3617266658f98

Before

+CARGO_INSTALL_ROOT
+CFLAGS
+C_INCLUDE_PATH
+DEVENV_DOTFILE
+DEVENV_PROFILE
+DEVENV_ROOT
+DEVENV_STATE
+GIT_EXTERNAL_DIFF
+IN_NIX_SHELL
+LD_LIBRARY_PATH
+LIBRARY_PATH
+NODE_OPTIONS
+PKG_CONFIG_PATH
+RUSTDOCFLAGS
+RUSTFLAGS
+RUST_SRC_PATH
+SWCRC
+name
~PATH
~TMPDIR
~XDG_CONFIG_DIRS
~XDG_DATA_DIRS

After

+AR
+AR_FOR_BUILD
+AS
+AS_FOR_BUILD
+CARGO_INSTALL_ROOT
+CC
+CC_FOR_BUILD
+CFLAGS
+CONFIG_SHELL
+CXX
+CXX_FOR_BUILD
+DETERMINISTIC_BUILD
+DEVENV_DOTFILE
+DEVENV_PROFILE
+DEVENV_ROOT
+DEVENV_RUNTIME
+DEVENV_STATE
+GETTEXTDATADIRS_FOR_BUILD
+GIT_EXTERNAL_DIFF
+HOST_PATH
+IN_NIX_SHELL
+LD
+LD_DYLD_PATH
+LD_FOR_BUILD
+MACOSX_DEPLOYMENT_TARGET
+NIX_BINTOOLS
+NIX_BINTOOLS_FOR_BUILD
+NIX_BINTOOLS_WRAPPER_TARGET_BUILD_x86_64_apple_darwin
+NIX_BINTOOLS_WRAPPER_TARGET_HOST_aarch64_unknown_linux_gnu
+NIX_BINTOOLS_WRAPPER_TARGET_HOST_x86_64_apple_darwin
+NIX_BINTOOLS_WRAPPER_TARGET_HOST_x86_64_unknown_linux_gnu
+NIX_BUILD_CORES
+NIX_BUILD_TOP
+NIX_CC
+NIX_CC_FOR_BUILD
+NIX_CC_WRAPPER_TARGET_BUILD_x86_64_apple_darwin
+NIX_CC_WRAPPER_TARGET_HOST_aarch64_unknown_linux_gnu
+NIX_CC_WRAPPER_TARGET_HOST_x86_64_apple_darwin
+NIX_CC_WRAPPER_TARGET_HOST_x86_64_unknown_linux_gnu
+NIX_CFLAGS_COMPILE
+NIX_CFLAGS_COMPILE_FOR_BUILD
+NIX_COREFOUNDATION_RPATH
+NIX_DONT_SET_RPATH
+NIX_DONT_SET_RPATH_FOR_BUILD
+NIX_ENFORCE_NO_NATIVE
+NIX_HARDENING_ENABLE
+NIX_IGNORE_LD_THROUGH_GCC
+NIX_LDFLAGS
+NIX_LDFLAGS_FOR_BUILD
+NIX_NO_SELF_RPATH
+NIX_PKG_CONFIG_WRAPPER_TARGET_HOST_x86_64_apple_darwin
+NIX_STORE
+NM
+NM_FOR_BUILD
+NODE_OPTIONS
+NODE_PATH
+OBJCOPY
+OBJDUMP
+PATH_LOCALE
+PKG_CONFIG
+PKG_CONFIG_PATH
+PYTHONHASHSEED
+PYTHONNOUSERSITE
+PYTHONPATH
+RANLIB
+RANLIB_FOR_BUILD
+READELF
+RUSTDOCFLAGS
+RUSTFLAGS
+RUST_SRC_PATH
+SIZE
+SIZE_FOR_BUILD
+SOURCE_DATE_EPOCH
+STRINGS
+STRINGS_FOR_BUILD
+STRIP
+STRIP_FOR_BUILD
+SWCRC
+TEMP
+TEMPDIR
+TMP
+WINDRES
+ZERO_AR_DATE
+__darwinAllowLocalNetworking
+__impureHostDeps
+__propagatedImpureHostDeps
+__propagatedSandboxProfile
+__sandboxProfile
+__structuredAttrs
+buildInputs
+buildPhase
+builder
+cmakeFlags
+configureFlags
+depsBuildBuild
+depsBuildBuildPropagated
+depsBuildTarget
+depsBuildTargetPropagated
+depsHostHost
+depsHostHostPropagated
+depsTargetTarget
+depsTargetTargetPropagated
+doCheck
+doInstallCheck
+dontAddDisableDepTrack
+mesonFlags
+name
+nativeBuildInputs
+out
+outputs
+patches
+phases
+preferLocalBuild
+propagatedBuildInputs
+propagatedNativeBuildInputs
+shell
+shellHook
+stdenv
+strictDeps
+system
~PATH
~TMPDIR
~XDG_DATA_DIRS

Version

1.0.2

domenkozar commented 1 month ago

We know about this and originally the naked shell existed just to improve bloated environment.

We reverted back to how Nix works, because unfortunately it broke too many things.

There's https://github.com/cachix/devenv/pull/1069 that removes more variables, feel free to explore if we can remove more of those.

martinjlowm commented 1 month ago

The absence of the following variables broke our setup: https://github.com/cachix/devenv/blob/c81069c83ab242dd831e74a992c1d38d32b77d14/src/modules/mkNakedShell.nix#L102-L109

on top of the specific compiler flags, that would force the wrong one in many cases.

One example is the Rust service integration - on Darwin it depends on libiconv, but that is no longer added to any searchable path (LD_LIBRARY_PATH which used to point to DEVENV_PROFILE/lib).

Not sure what the end goal is here, but would it make sense to have the integration specify which variables it needs (seeing that we cannot distinguish derivation build state from env vars)?

martinjlowm commented 1 month ago

Just a quick update - for reference, I updated our enterShell with:

for env in shellHook AR AR_FOR_BUILD AS AS_FOR_BUILD CC CC_FOR_BUILD CONFIG_SHELL CXX \
           CXX_FOR_BUILD DETERMINISTIC_BUILD DEVENV_RUNTIME GETTEXTDATADIRS_FOR_BUILD LD \
           LD_DYLD_PATH LD_FOR_BUILD MACOSX_DEPLOYMENT_TARGET NIX_BINTOOLS \
           NIX_BINTOOLS_FOR_BUILD NIX_BINTOOLS_WRAPPER_TARGET_BUILD_x86_64_apple_darwin \
           NIX_BINTOOLS_WRAPPER_TARGET_HOST_aarch64_unknown_linux_gnu \
           NIX_BINTOOLS_WRAPPER_TARGET_HOST_x86_64_apple_darwin \
           NIX_BINTOOLS_WRAPPER_TARGET_HOST_x86_64_unknown_linux_gnu NIX_CC \
           NIX_CC_FOR_BUILD NIX_CC_WRAPPER_TARGET_BUILD_x86_64_apple_darwin \
           NIX_CC_WRAPPER_TARGET_HOST_aarch64_unknown_linux_gnu \
           NIX_CC_WRAPPER_TARGET_HOST_x86_64_apple_darwin \
           NIX_CC_WRAPPER_TARGET_HOST_x86_64_unknown_linux_gnu NIX_CFLAGS_COMPILE \
           NIX_CFLAGS_COMPILE_FOR_BUILD NIX_COREFOUNDATION_RPATH NIX_DONT_SET_RPATH \
           NIX_DONT_SET_RPATH_FOR_BUILD NIX_ENFORCE_NO_NATIVE NIX_HARDENING_ENABLE \
           NIX_IGNORE_LD_THROUGH_GCC NIX_LDFLAGS NIX_LDFLAGS_FOR_BUILD NIX_NO_SELF_RPATH \
           NIX_PKG_CONFIG_WRAPPER_TARGET_HOST_x86_64_apple_darwin NIX_STORE NM NM_FOR_BUILD \
           NODE_PATH OBJCOPY OBJDUMP PATH_LOCALE PKG_CONFIG PYTHONHASHSEED PYTHONNOUSERSITE \
           PYTHONPATH RANLIB RANLIB_FOR_BUILD READELF SIZE SIZE_FOR_BUILD SOURCE_DATE_EPOCH \
           STRINGS STRINGS_FOR_BUILD STRIP STRIP_FOR_BUILD WINDRES ZERO_AR_DATE \
           __darwinAllowLocalNetworking __impureHostDeps __propagatedImpureHostDeps \
           __propagatedSandboxProfile __sandboxProfile cmakeFlags configureFlags \
           dontAddDisableDepTrack mesonFlags system; do
  unset $env;
done

export PKG_CONFIG_PATH="$DEVENV_PROFILE/lib/pkgconfig:''${PKG_CONFIG_PATH-}"
export LD_LIBRARY_PATH="$DEVENV_PROFILE/lib:''${LD_LIBRARY_PATH-}"
export LIBRARY_PATH="$DEVENV_PROFILE/lib:''${LIBRARY_PATH-}"
export C_INCLUDE_PATH="$DEVENV_PROFILE/include:''${C_INCLUDE_PATH-}"
export XDG_DATA_DIRS="$DEVENV_PROFILE/share:''${XDG_DATA_DIRS-}"
export XDG_CONFIG_DIRS="$DEVENV_PROFILE/etc/xdg:''${XDG_CONFIG_DIRS-}"

to get back the behavior we were expecting. In case anyone else gets into the same situation.

On the topic of unsetEnvVars I suppose disregarding any __-based variables should be a good place to start.

domenkozar commented 1 month ago

@martinjlowm can you provide a minimal example of how to reproduce this issue in a separate issue? I'll find a way to fix it.

sandydoo commented 3 weeks ago

on top of the specific compiler flags, that would force the wrong one in many cases.

If this is about macOS specifically, i.e. using gcc instead of clang, then that was fixed in https://github.com/cachix/devenv/pull/1162. If you have a different case, please open an issue with the specifics.

One example is the Rust service integration - on Darwin it depends on libiconv, but that is no longer added to any searchable path (LD_LIBRARY_PATH which used to point to DEVENV_PROFILE/lib).

Do you have an example to show us where this breaks? This might also be related to #1162.

LD_LIBRARY_PATH is a terrible idea. It will eventually break programs in your shell that dynamically link to things like glibc. There's probably a hundred issues on devenv that can be eventually traced to dynamic linking. We're talking about issues like "devenv's shell makes git crash".

With 1.0, we're back to the full Nix shell. Libraries are not added to LD_LIBRARY_PATH, but rather to NIX_LDFLAGS, and I'm sure you'll find libiconv in there. These paths are picked up by the linkers Nix provides in stdenv.

On the topic of unsetEnvVars I suppose disregarding any __-based variables should be a good place to start.

Perhaps. But we have to be careful here, and remove with intent and a complete understanding of what the variable does.