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

Bloated environment variables with 1.0 #1084

Open
martinjlowm opened this issue Mar 31, 2024 · 5 comments
Open

Bloated environment variables with 1.0 #1084

martinjlowm opened this issue Mar 31, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@martinjlowm
Copy link

Describe the bug
With the introduction of cf9d122 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. 1909832

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

@martinjlowm martinjlowm added the bug Something isn't working label Mar 31, 2024
@domenkozar
Copy link
Member

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 #1069 that removes more variables, feel free to explore if we can remove more of those.

@martinjlowm
Copy link
Author

The absence of the following variables broke our setup:

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-}"
# these provide shell completions / default config options
export XDG_DATA_DIRS="$DEVENV_PROFILE/share:''${XDG_DATA_DIRS-}"
export XDG_CONFIG_DIRS="$DEVENV_PROFILE/etc/xdg:''${XDG_CONFIG_DIRS-}"

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
Copy link
Author

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
Copy link
Member

@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
Copy link
Member

sandydoo commented Apr 25, 2024

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 #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants