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

in flake mkShell default RUSTFLAGS to an empty string if unset #10880

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

plul
Copy link
Contributor

@plul plul commented Jun 4, 2024

This allows activation of the devshell on systems that don't have a system wide Rust installation.

@the-mikedavis
Copy link
Member

What error does this fix? I run nixos without a global Rust installation and no RUSTFLAGS var set and the dev shell works for me without issue

@pascalkuthe
Copy link
Member

Yeah rust installations don't even set RUSTFLAGS. An empty environment variable just expands to an empty string so it should work as is

@plul
Copy link
Contributor Author

plul commented Jun 4, 2024

Maybe it's a direnv thing, but

❯ direnv reload
direnv: loading ~/repos/github.com/helix-editor/helix/.envrc
direnv: using flake
do you want to allow configuration setting 'extra-substituters' to be set to 'https://helix.cachix.org' (y/N)?
do you want to permanently mark this value as untrusted (y/N)?
warning: ignoring untrusted flake configuration setting 'extra-substituters'.
Pass '--accept-flake-config' to trust it
do you want to allow configuration setting 'extra-trusted-public-keys' to be set to 'helix.cachix.org-1:ejp9KQpR1FBI2onstMQ34yogDm4OgU2ru6lIwPvuCVs=' (y/N)?
do you want to permanently mark this value as untrusted (y/N)?
warning: ignoring untrusted flake configuration setting 'extra-trusted-public-keys'.
Pass '--accept-flake-config' to trust it
direnv: nix-direnv: renewed cache
/home/<me>/.config/direnv/lib/hm-nix-direnv.sh:2475: RUSTFLAGS: unbound variable
direnv: error exit status 127

whereas with export RUSTFLAGS="" ahead of time it works.

@plul
Copy link
Contributor Author

plul commented Jun 4, 2024

Yeah nix develop works so looks like a direnv thing. Anyway this fixes it (for me)

@the-mikedavis
Copy link
Member

I'm also using direnv but I don't see that error

@plul
Copy link
Contributor Author

plul commented Jun 4, 2024

hm weird. Might be messed up on my end somehow then

❯ direnv --version
2.34.0

@plul
Copy link
Contributor Author

plul commented Jun 4, 2024

Figured out what causes the error. I have in ~/.config/direnv/direnv.toml:

[global]
strict_env = true

and it is this strict_env = true that causes the error.
From the docs:

If set to true, the .envrc will be loaded with set -euo pipefail. This option will be the default in the future.

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little skeptical that set -u will ever become the default in direnv because of how much I suspect it would break (of user scripts). Explicitly defaulting the variable seems fine to me though. I just have a small change in mind

flake.nix Outdated
Comment on lines 119 to 120
then ''''${RUSTFLAGS:-""} -C link-arg=-fuse-ld=lld -C target-cpu=native -Clink-arg=-Wl,--no-rosegment''
else "$RUSTFLAGS";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else branch here will also need the explicit defaulting. Instead we can change the line below for exporting the custom rust flags to

export RUSTFLAGS="''${RUSTFLAGS:-""} ${rustFlagsEnv}"

and remove the $RUSTFLAGS from this variable altogether

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a cleaner approach indeed. Updated.

@plul plul force-pushed the devshell-default-RUSTFLAGS branch from faaf21b to ec8dc18 Compare June 10, 2024 07:33
@the-mikedavis the-mikedavis added the A-packaging Area: Packaging and bundling label Jun 10, 2024
@pascalkuthe pascalkuthe merged commit a1cda3c into helix-editor:master Jun 10, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-packaging Area: Packaging and bundling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants