rustPlatform.buildRustPackage: Pass env.RUSTFLAGS only when specified or needed to fix builds#464707
Conversation
|
@gepbird I just noticed that the package is an ou-of-tree NUR package. Is the shadowing mechanism part of the package definition or part of the rustc behavior? |
I'm fairly confident it's a rustc behaviour, but I'm not sure how to fully confirm that. The environment variable is not specific to the package, but for compiling to windows, see #139966 (comment). However in that issue it looks like people are compiling a package to only a single platform, while for |
Caused by NixOS/nixpkgs#435278 RUSTFLAGS is now always set and overrides whatever we specify in .cargo/config.toml TODO: Keep an eye on NixOS/nixpkgs#464707 NixOS/nixpkgs#464992 and possibly revert or adjust to the new status quo in nixpkgs.
3dfd0be to
947fda0
Compare
… or needed Prevent shadowing RUSTFLAGS-like variables with lower priorities, such as mint-mod-mamanager's CARGO_TARGET_X86_64_PC_WINDOWS_GNU_RUSTFLAGS Co-authored-by: Gergő Gutyina <gutyina.gergo.2@gmail.com>
947fda0 to
978d298
Compare
|
Rebased from |
| ${if args ? RUSTFLAGS || isDarwinDebug then "RUSTFLAGS" else null} = | ||
| lib.optionalString isDarwinDebug "-C split-debuginfo=packed " | ||
| # Workaround the existing RUSTFLAGS specified as a list. | ||
| + interpolateString (args.RUSTFLAGS or ""); |
There was a problem hiding this comment.
This is a good fix, but if you feel like putting in a bit more, even better would be not doing this in buildRustPackage to begin with — it's not the right place for it because plenty of Rust builds don't go through it. The rustc wrapper would be a much better place — it's universally used and doesn't need any environment variables to be modified.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
I'm leaning toward addressing the simplification in a subsequent PR, as such change could introduce new incompatibility. (The existing change is a partial revert )
Would it be okay if we merge and backport this fix as-is?
There was a problem hiding this comment.
I think it's okay, at least it shouldn't be worse than the previous situation
There was a problem hiding this comment.
@ShamrockLee still feel like making that subsequent PR?
There was a problem hiding this comment.
@ShamrockLee still feel like making that subsequent PR?
After looking into the buildRustPackage, I found the RUSTFLAG for buildRustPackagesituation to be quite different from GOFLAG for buildGoModule.
buildGoModule invokes cgo to build, which is also where GOFLAGS apply. We could specify the flags directly with our own flag lists (such as goBuildFlags).
buildRustPackage invokes cargo to build, but RUSTFLAGS apply to rustc (invoked by cargo). We cannot replace it with a flag list without stringifying to RUSTFLAGS first.
There was a problem hiding this comment.
My suggestion was to put it in the rustc wrapper. Cargo invokes rustc through that wrapper.
|
Successfully created backport PR for |
…flags (NixOS#464981)" This reverts commit d2c6221. PR NixOS#464981 changed libcosmicAppHook to use the global RUSTFLAGS instead of target-specific CARGO_TARGET_*_RUSTFLAGS as a workaround for issue PR NixOS#464392. However, this broke cross-compilation for packages using the crabtime procedural macro (like cosmic-initial-setup). The root cause was that buildRustPackage was always setting env.RUSTFLAGS, which shadowed the target-specific flags. This has been fixed in commit PR NixOS#464707, which only sets RUSTFLAGS when explicitly needed. Fixes cross-compilation of cosmic-initial-setup and other cosmic packages that use crabtime proc macros. Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>
…flags (NixOS#464981)" This reverts commit d2c6221. PR NixOS#464981 changed libcosmicAppHook to use the global RUSTFLAGS instead of target-specific CARGO_TARGET_*_RUSTFLAGS as a workaround for issue PR NixOS#464392. However, this broke cross-compilation for packages using the crabtime procedural macro (like cosmic-initial-setup). The root cause was that buildRustPackage was always setting env.RUSTFLAGS, which shadowed the target-specific flags. This has been fixed in commit PR NixOS#464707, which only sets RUSTFLAGS when explicitly needed. Fixes cross-compilation of cosmic-initial-setup and other cosmic packages that use crabtime proc macros. Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>
…flags (NixOS#464981)" This reverts commit d2c6221. PR NixOS#464981 changed libcosmicAppHook to use the global RUSTFLAGS instead of target-specific CARGO_TARGET_*_RUSTFLAGS as a workaround for issue PR NixOS#464392. However, this broke cross-compilation for packages using the crabtime procedural macro (like cosmic-initial-setup). The root cause was that buildRustPackage was always setting env.RUSTFLAGS, which shadowed the target-specific flags. This has been fixed in commit PR NixOS#464707, which only sets RUSTFLAGS when explicitly needed. Fixes cross-compilation of cosmic-initial-setup and other cosmic packages that use crabtime proc macros. Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>
…flags (NixOS#464981)" This reverts commit d2c6221. PR NixOS#464981 changed libcosmicAppHook to use the global RUSTFLAGS instead of target-specific CARGO_TARGET_*_RUSTFLAGS as a workaround for issue PR NixOS#464392. However, this broke cross-compilation for packages using the crabtime procedural macro (like cosmic-initial-setup). The root cause was that buildRustPackage was always setting env.RUSTFLAGS, which shadowed the target-specific flags. This has been fixed in commit PR NixOS#464707, which only sets RUSTFLAGS when explicitly needed. Fixes cross-compilation of cosmic-initial-setup and other cosmic packages that use crabtime proc macros. Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>
…flags (NixOS#464981)" This reverts commit d2c6221. PR NixOS#464981 changed libcosmicAppHook to use the global RUSTFLAGS instead of target-specific CARGO_TARGET_*_RUSTFLAGS as a workaround for issue PR NixOS#464392. However, this broke cross-compilation for packages using the crabtime procedural macro (like cosmic-initial-setup). The root cause was that buildRustPackage was always setting env.RUSTFLAGS, which shadowed the target-specific flags. This has been fixed in commit PR NixOS#464707, which only sets RUSTFLAGS when explicitly needed. Fixes cross-compilation of cosmic-initial-setup and other cosmic packages that use crabtime proc macros. Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>
…flags (NixOS#464981)" This reverts commit d2c6221. PR NixOS#464981 changed libcosmicAppHook to use the global RUSTFLAGS instead of target-specific CARGO_TARGET_*_RUSTFLAGS as a workaround for issue PR NixOS#464392. However, this broke cross-compilation for packages using the crabtime procedural macro (like cosmic-initial-setup). The root cause was that buildRustPackage was always setting env.RUSTFLAGS, which shadowed the target-specific flags. This has been fixed in commit PR NixOS#464707, which only sets RUSTFLAGS when explicitly needed. Fixes cross-compilation of cosmic-initial-setup and other cosmic packages that use crabtime proc macros. Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>
Prevent shadowing
RUSTFLAGS-like variables with lower priorities, such as mint-mod-mamanager'sCARGO_TARGET_X86_64_PC_WINDOWS_GNU_RUSTFLAGS.Fixes #435278 (comment)
Fixes #464392
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.