Skip to content

rustPlatform.buildRustPackage: Pass env.RUSTFLAGS only when specified or needed to fix builds #464707

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.

Sign up for GitHub

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

Merged
alyssais merged 1 commit into from
Nov 30, 2025
Merged

rustPlatform.buildRustPackage: Pass env.RUSTFLAGS only when specified or needed to fix builds #464707

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions pkgs/build-support/rust/build-rust-package/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,20 @@ lib.extendMkDerivation {
"buildRustPackage: `useFetchCargoVendor` is non‐optional and enabled by default as of 25.05, remove it"
true;
{
env = {
PKG_CONFIG_ALLOW_CROSS = if stdenv.buildPlatform != stdenv.hostPlatform then 1 else 0;
RUST_LOG = logLevel;
RUSTFLAGS =
lib.optionalString (
stdenv.hostPlatform.isDarwin && buildType == "debug"
) "-C split-debuginfo=packed "
# Workaround the existing RUSTFLAGS specified as a list.
+ interpolateString (args.RUSTFLAGS or "");
}
// args.env or { };
env =
let
isDarwinDebug = stdenv.hostPlatform.isDarwin && buildType == "debug";
in
{
PKG_CONFIG_ALLOW_CROSS = if stdenv.buildPlatform != stdenv.hostPlatform then 1 else 0;
RUST_LOG = logLevel;
# Prevent shadowing *_RUSTFLAGS environment variables
${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 "");
Copy link
Member

@alyssais alyssais Nov 28, 2025

Choose a reason for hiding this comment

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

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.

Sign in to view

This comment was marked as resolved.

Sign in to view

Copy link
Contributor Author

@ShamrockLee ShamrockLee Nov 30, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Member

@Aleksanaa Aleksanaa Nov 30, 2025

Choose a reason for hiding this comment

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

I think it's okay, at least it shouldn't be worse than the previous situation

Copy link
Member

@alyssais alyssais Feb 2, 2026

Choose a reason for hiding this comment

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

@ShamrockLee still feel like making that subsequent PR?

Copy link
Contributor Author

@ShamrockLee ShamrockLee Feb 3, 2026

Choose a reason for hiding this comment

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

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

Copy link
Member

@alyssais alyssais Feb 3, 2026

Choose a reason for hiding this comment

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

My suggestion was to put it in the rustc wrapper. Cargo invokes rustc through that wrapper.

}
// args.env or { };

cargoDeps =
if cargoVendorDir != null then
Expand Down
Loading