Homebrew / brew

🍺 The missing package manager for macOS (or Linux)
https://brew.sh
BSD 2-Clause "Simplified" License
41.13k stars 9.66k forks source link

RUSTFLAGS override .cargo/config.toml and result in manual workarounds #18556

Open cho-m opened 5 days ago

cho-m commented 5 days ago

brew doctor output

CI run https://github.com/Homebrew/homebrew-core/actions/runs/10208208304/job/28244359400#step:3:264

==> brew doctor
All steps passed!

Verification

brew config output

==> brew config
HOMEBREW_VERSION: 4.3.12-52-g53e3632
ORIGIN: https://github.com/Homebrew/brew
HEAD: 53e363258adfced393b6ec99fce66f6ea1ff7687
Last commit: 7 hours ago
Core tap HEAD: e3a6fd41d57b0b82402f33dfb352fd8e2ab23473
Core tap last commit: 3 minutes ago
Core tap JSON: 02 Aug 01:08 UTC
HOMEBREW_PREFIX: /home/linuxbrew/.linuxbrew
HOMEBREW_BOOTSNAP: set
HOMEBREW_CASK_OPTS: []
HOMEBREW_COLOR: set
HOMEBREW_CURL_PATH: /usr/bin/curl
HOMEBREW_DEVELOPER: set
HOMEBREW_FAIL_LOG_LINES: 150
HOMEBREW_GITHUB_API_TOKEN: set
HOMEBREW_GIT_EMAIL: 1589480+BrewTestBot@users.noreply.github.com
HOMEBREW_GIT_NAME: BrewTestBot
HOMEBREW_GIT_PATH: /usr/bin/git
HOMEBREW_LOGS: /__w/homebrew-core/homebrew-core/logs
HOMEBREW_MAKE_JOBS: 4
HOMEBREW_NO_AUTO_UPDATE: set
HOMEBREW_NO_COLOR: set
HOMEBREW_NO_EMOJI: set
HOMEBREW_NO_ENV_HINTS: set
HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK: set
HOMEBREW_NO_INSTALL_FROM_API: set
HOMEBREW_SORBET_RUNTIME: set
Homebrew Ruby: 3.3.4 => /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/portable-ruby/3.3.4/bin/ruby
CPU: quad-core 64-bit zen3
Clang: N/A
Git: 2.46.0 => /usr/bin/git
Curl: 7.81.0 => /usr/bin/curl
Kernel: Linux 6.5.0-1024-azure x86_64 GNU/Linux
OS: Ubuntu 22.04.4 LTS
Host glibc: 2.35
/usr/bin/gcc: 11.4.0
/usr/bin/ruby: N/A
glibc: N/A
gcc@11: N/A
gcc: N/A
xorg: N/A

What were you trying to do (and why)?

Building bottle in CI, e.g. https://github.com/Homebrew/homebrew-core/pull/179360

What happened (include all command output)?

Failure when building due to overridden .cargo/config.toml:

  ==> cargo install --locked --root=/home/linuxbrew/.linuxbrew/Cellar/veilid/0.3.4 --path=veilid-cli
...
  error[E0433]: failed to resolve: could not find `Builder` in `task`
     --> veilid-tools/src/spawn.rs:58:54
      |
  58  |                     MustJoinHandle::new(tokio::task::Builder::new().name(name).spawn(future).unwrap())
      |                                                      ^^^^^^^ could not find `Builder` in `task`
      |
  note: found an item that was configured out

What did you expect to happen?

Successful build without any extra workarounds.

Step-by-step reproduction instructions (by running brew commands)

1. Remove `ENV["RUSTFLAGS"]` workaround from `veilid`
2. `brew install [--build-from-source | --build-bottle] veilid` on Linux
cho-m commented 5 days ago

Opening issue to align to the comment I updated in aptos after looking into the issue https://github.com/Homebrew/homebrew-core/blob/572efea83d28844a65079681915760edb4284b2d/Formula/a/aptos.rb#L42-L43

    # FIXME: Look into a different way to specify extra RUSTFLAGS in superenv as they override .cargo/config.toml
    # Ref: https://github.com/Homebrew/brew/blob/master/Library/Homebrew/extend/ENV/super.rb#L65

Specifically caused by superenv now setting RUSTFLAGS which takes precedence over config file https://github.com/Homebrew/brew/blob/3291ad4fc78f33f8e9bb7ce37745d2a0e697f5fb/Library/Homebrew/extend/ENV/super.rb#L65

I recall there was an ask in rust or cargo to introduce additive flags but can't find discussion right now. EDIT: https://github.com/rust-lang/cargo/issues/5376

Not sure what is best way to handle this to avoid manual workarounds (which increase formula maintenance to align with upstream) while also optimizing to specific microarchitecture.

MikeMcQuaid commented 4 days ago

ENV.no_rustflags or something feels like the right fit here.

while also optimizing to specific microarchitecture.

To be clear: we're producing the same bottle regardless of where it's built, right?

cho-m commented 4 days ago

To be clear: we're producing the same bottle regardless of where it's built, right?

Yes. Microarchitecture as in our superenv oldest CPU architecture (Core2 and Nehalem), not native arch.

cho-m commented 1 day ago

Cargo issue was https://github.com/rust-lang/cargo/issues/5376. There doesn't seem to be a clean way to add extra RUSTFLAGS yet.

May need to be done via a rustc shim instead.

MikeMcQuaid commented 1 day ago

May need to be done via a rustc shim instead.

Would ENV.no_rustflags not work here to say "don't set RUSTFLAGS"?

cho-m commented 1 day ago

Would ENV.no_rustflags not work here to say "don't set RUSTFLAGS"?

Using a shim may allow us to still pass in --codegen target-cpu=[core2 | nehalem | westmere] and hopefully just work without extra modifications to the formula.

Bo98 commented 23 hours ago

Yeah a rustc shim seems to be ultimately what is necessary here and would directly align with how we handle C/C++. Debian uses a cargo wrapper for as they similarly concluded RUSTFLAGS to be useless.

They even went as far as ship it as a part of their cargo package: https://salsa.debian.org/rust-team/cargo/-/blob/debian/sid/debian/bin/cargo (we won't do this - just demonstrating how others have had to resort to wrappers too)

carlocab commented 5 hours ago

Shim is probably the easiest, and is kind of supported by the RUSTC_WRAPPER environment variable: https://doc.rust-lang.org/cargo/reference/environment-variables.html