rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.17k stars 12.56k forks source link

RUSTFLAGS sometimes isn't passed to all rustc command lines #127727

Closed yurivict closed 2 months ago

yurivict commented 2 months ago

In some projects the RUSTFLAGS are ignored and not passed to rustc, at least on the i386 architecture.

Testcase: https://github.com/drahnr/cargo-spellcheck cargo is called with the RUSTFLAGS environment variable containing "-C target-feature=+sse,+sse2". Yet this RUSTFLAGS value is not passed to rustc.

Build log on i386. It shows the build failure because the above RUSTFLAGS wasn't passed to the rustc command line.

Meta

$ rustc --version --verbose
rustc 1.79.0 (129f3b996 2024-06-10) (built from a source tarball)
binary: rustc
commit-hash: 129f3b9964af4d4a709d1383930ade12dfe7c081
commit-date: 2024-06-10
host: x86_64-unknown-freebsd
release: 1.79.0
LLVM version: 18.1.7

OS: FreeBSD 14.1 The builds are done in the FreeBSD ports framework.

saethlin commented 2 months ago

cargo is called with the RUSTFLAGS environment variable containing "-C target-feature=+sse,+sse2".

I cannot find mention of target-feature anywhere in the build log or that repo. Where is RUSTFLAGS being set?

yurivict commented 2 months ago

The RUSTFLAGS environment variable is set to "-C target-feature=+sse,+sse2" when cargo is called. This isn't shown in the log.

workingjubilee commented 2 months ago

This build log is for an entirely different repo.

workingjubilee commented 2 months ago

@yurivict Would you link me to your complete build setup for these? Your log suggests you are applying patches, using a .cargo/config.toml, and more, and there is a strong possibility you are setting something that cargo interprets as deliberately overriding the RUSTFLAGS env var. A log for a different repo and without the build-relevant env vars being used is inadequate.

yurivict commented 2 months ago

@workingjubilee

Sorry for the log confusion.

Here is the full build log including the full cargo invocation.

It shows that "-C target-feature=+sse,+sse2" argument is passed to some rustc commands but not to all of them, and the build failure is caused by the lack of these target features.

ehuss commented 2 months ago

When using the --target flag, cargo will not pass RUSTFLAGS to things built for the host (such as build scripts and proc-macros). Unfortunately the option to fix that is still unstable, but you can read more about the host config option.

It's not clear from the build logs exactly why ring does not build for x86_64-unknown-freebsd (assuming that is your host) without the given options. That seems like a limitation of ring that you might want to investigate.

yurivict commented 2 months ago

It's not clear from the build logs exactly why ring does not build for x86_64-unknown-freebsd (assuming that is your host) without the given options.

Both host and target architectures are i386 in this case. The build is performed in the i386 VM (FreeBSD jail) on the amd64 VM host, but amd64 isn't visible to the build.

The build fails because sse2 is disabled on i386 by default, and needs to be enabled.

yurivict commented 2 months ago

Unfortunately the option to fix that is still unstable, but you can read more about the host config option.

What would be the working (though potentially unstable) option to pass to CARGO to fix this? We have many rust-based ports failing on i386 for the same reason.

yurivict commented 2 months ago

@ehuss

The host config option appears to be for a cross-compilation situation, while we run the i386 build in the i386 VM, so that there is no cross-compilation.

ehuss commented 2 months ago

"Cross compilation" in this instance is equivalent to passing --target, even if the target is the same as the host. If you don't want to put cargo into this "cross compilation" mode, don't pass --target.

yurivict commented 2 months ago

@ehuss

Thank you. It looks like we need to disable cross-compilation in the FreeBSD ports framework.

I suggested this here.

Thank you for your help.

workingjubilee commented 2 months ago

@yurivict I am perplexed. Why does SSE2 have to be enabled for the host?

workingjubilee commented 2 months ago

i686-unknown-freebsd uses the pentium4 so this assertion should pass.

yurivict commented 2 months ago

@workingjubilee

I am perplexed. Why does SSE2 have to be enabled for the host?

I believe that this is an LLVM setup that sse2 isn't enabled by default on i386.

workingjubilee commented 2 months ago

But the actual --target passed is --target i686-unknown-freebsd, and the host and the target are "the same", and I don't see a patch in FreeBSD to add an i386 target. So all that you said doesn't matter.

Ah, I see, FreeBSD patches their i686 target to be the Pentium Pro and has not upstreamed this patch, it seems.

https://github.com/freebsd/freebsd-ports/blob/94c86dc918dc3051f23d3b05a33ff4b45b05ec78/lang/rust/files/patch-compiler_rustc__target_src_spec_targets_i686__unknown__freebsd.rs

@yurivict Has FreeBSD considered:

yurivict commented 2 months ago

upstreaming a patch to ring to support non-SSE2 CPUs?

According to the FreeBSD policy, the lowest supported CPU supports SSE2.

[...] so they can access the internet securely at build time?

Accessing the internet at build time isn't allowed during port build for security reasons.

workingjubilee commented 2 months ago

That is an understandable limitation, but if so, I'm not sure how a build is failing for the above-described reasons? The only reason I can think of to build ring during a build script is the thing I mentioned.

Anyway, your build should work if the patch I linked is removed from the FreeBSD ports repo. Or... a patch is added which unpatches the patch, I suppose? I don't know how you all prefer your software over there, exactly.

yurivict commented 2 months ago

Anyway, your build should work if [the patch I linked] is removed from the FreeBSD ports repo.

It also succeeds when the --target options are removed. Default target should be used, no cross-compilation is allowed to happen by default.

workingjubilee commented 2 months ago

That is also fine!

I'm just not sure why, if FreeBSD mandates an SSE2 minimum, that y'all would then patch your 32-bit x86 target to not have SSE2.