Use ENV.append_to_rustflags
#232566
Use ENV.append_to_rustflags
#232566
Conversation
c1e08bb to
2660433
Compare
Co-authored-by: Carlo Cabrera <github@carlo.cab>
2660433 to
42c64a5
Compare
|
Caution Please do not push to this PR branch before the bottle commits have been pushed, as this results in a state that is difficult to recover from. If you need to resolve a merge conflict, please use a merge commit. Do not force-push to this PR branch. |
| ENV["RUSTFLAGS"] = "-C target-cpu=apple-m1" if OS.mac? && Hardware::CPU.arm? | ||
| ENV.append_to_rustflags "-C target-cpu=apple-m1" if OS.mac? && Hardware::CPU.arm? |
There was a problem hiding this comment.
It seems this broke arm64 macOS builds, unfortunately: https://github.com/Homebrew/homebrew-core/actions/runs/16884130215
There was a problem hiding this comment.
@carlocab I saw that but I didn't see anything that looked consistent with the changes here. I just managed to build qsv locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install -s qsv on an ARM Mac.
Unrelatedly but seems confusing that that workflow doesn't actually show failed builds as 🔴?
There was a problem hiding this comment.
There was a problem hiding this comment.
@carlocab I'm talking about https://github.com/Homebrew/homebrew-core/actions/runs/16884130215 which you linked where upload is the only failing job but yet one of the bottles failed to build.
There was a problem hiding this comment.
Ah, yes. They actually all failed to build. But, as for why the builds themselves aren't 🔴: brew test-bot doesn't treat build failures as errors when there is no existing bottle (except for new formulae).
We could probably adjust that for dispatch-build-bottle, but it's worked fine as-is for quite a while now, so I haven't put in the time to change it.
There was a problem hiding this comment.
We could probably adjust that for
dispatch-build-bottle, but it's worked fine as-is for quite a while now, so I haven't put in the time to change it.
Yeh, think that would be nice 👍🏻
| # Explicitly set linker to avoid Cargo defaulting to | ||
| # incorrect or outdated linker (e.g. x86_64-apple-darwin14-clang) | ||
| ENV["RUSTFLAGS"] = "-C linker=#{ENV.cc}" | ||
| ENV.append_to_rustflags "-C linker=#{ENV.cc}" |
There was a problem hiding this comment.
Looks like these RUSTFLAGS aren't actually making it to rustc for some reason:
https://github.com/Homebrew/homebrew-core/actions/runs/16884114006/job/47827335082#step:4:255
There was a problem hiding this comment.
@carlocab How can it be that these RUSTFLAGS are both breaking builds and not making it through?
Which of these can you reproduce locally?
There was a problem hiding this comment.
How can it be that these RUSTFLAGS are both breaking builds and not making it through?
Because they aren't breaking builds, they are making them succeed (so their not making it through results in build failure).
Which of these can you reproduce locally?
Haven't been able to try a local build; I won't probably be able to get to it until tomorrow.
I think this shouldn't be needed anymore, and will fix the build failure we're now seeing after #232566.
They are definitely making it through to $ brew uninstall -f qsv; HOMEBREW_NO_INSTALL_FROM_API=1 brew install -sv qsvwith diff --git i/Library/Homebrew/extend/ENV/shared.rb w/Library/Homebrew/extend/ENV/shared.rb
index 28e66d480e2..a41fa701e0f 100644
--- i/Library/Homebrew/extend/ENV/shared.rb
+++ w/Library/Homebrew/extend/ENV/shared.rb
@@ -116,6 +116,7 @@ module SharedEnvExtension
sig { params(rustflags: String).void }
def append_to_rustflags(rustflags)
append("HOMEBREW_RUSTFLAGS", rustflags)
+ p [:append_to_rustflags, self["HOMEBREW_RUSTFLAGS"]]
end
# Prepends a directory to `PATH`.
diff --git i/Library/Homebrew/extend/ENV/std.rb w/Library/Homebrew/extend/ENV/std.rb
index f4b5f34f113..88d44509c23 100644
--- i/Library/Homebrew/extend/ENV/std.rb
+++ w/Library/Homebrew/extend/ENV/std.rb
@@ -36,6 +36,7 @@ module Stdenv
self["MAKEFLAGS"] = "-j#{make_jobs}"
self["RUSTC_WRAPPER"] = "#{HOMEBREW_SHIMS_PATH}/super/rustc_wrapper"
self["HOMEBREW_RUSTFLAGS"] = Hardware.rustflags_target_cpu(effective_arch)
+ p [:std_setup_build_environment, self["HOMEBREW_RUSTFLAGS"]]
if HOMEBREW_PREFIX.to_s != "/usr/local"
# /usr/local is already an -isystem and -L directory so we skip it
diff --git i/Library/Homebrew/extend/ENV/super.rb w/Library/Homebrew/extend/ENV/super.rb
index bf083f31654..88b4f58edb0 100644
--- i/Library/Homebrew/extend/ENV/super.rb
+++ w/Library/Homebrew/extend/ENV/super.rb
@@ -64,6 +64,7 @@ module Superenv
self["MAKEFLAGS"] ||= "-j#{determine_make_jobs}"
self["RUSTC_WRAPPER"] = "#{HOMEBREW_SHIMS_PATH}/super/rustc_wrapper"
self["HOMEBREW_RUSTFLAGS"] = Hardware.rustflags_target_cpu(effective_arch)
+ p [:super_setup_build_environment, self["HOMEBREW_RUSTFLAGS"]]
self["PATH"] = determine_path
self["PKG_CONFIG_PATH"] = determine_pkg_config_path
self["PKG_CONFIG_LIBDIR"] = determine_pkg_config_libdir || ""and they are set as expected: If I add: You can see they are also being passed to |
|
I think I have a fix. PR up shortly. I don't have time to test until tomorrow, so feel free to try it out and merge if ready. |
If we pass it first, it can be overridden by other arguments passed on the command line. We don't want that. See discussion at Homebrew/homebrew-core#232566.
Let's see if we can recover the bottles lost from #232566.
Let's see if we can recover the bottles lost from #232566.
Let's see if we can recover the bottles lost from #232566.
Provided as part of Homebrew/brew#20380
Don't merge until Homebrew/brew#20380 in a stable release.