uv icon indicating copy to clipboard operation
uv copied to clipboard

Revisit LTO for ppc64le binaries

Open zanieb opened this issue 1 year ago • 4 comments

In #5904, LTO was added to our release profile. However, in #5984, this failed for just the ppc64le musl cross build (which uses the nightly toolchain):

error: failed to get bitcode from object file for LTO (could not find requested section)

error: could not compile `uv` (bin "uv") due to 1 previous error
💥 maturin failed
  Caused by: Failed to build a native library through cargo
  Caused by: Cargo build finished with "exit status: 101": `env -u CARGO "cargo" "rustc" "--features" "self-update" "--target" "powerpc64le-unknown-linux-musl" "--message-format" "json-render-diagnostics" "--locked" "--manifest-path" "/home/runner/work/uv/uv/crates/uv/Cargo.toml" "--release" "--bin" "uv" "--" "-C" "link-arg=-s"`

I unsuccessfully tried using -Cembed-bitcode=yes (https://github.com/rust-osdev/cargo-xbuild/pull/73) to workaround this, before creating a separate release profile (release-no-lto) with LTO disabled and using it for just this build (https://github.com/astral-sh/uv/pull/5984/commits/f0ffda4039f80d49810dd37ac067c373bb26c7f8).

I'm not anywhere near an expert at Rust compilation options. Someone else should validate this approach and explore alternatives.

zanieb avatar Aug 09 '24 23:08 zanieb

Hey!

First of all, sorry for making this an issue right at the time of release. Wish I could have caught this earlier.

Now, to the issue at hand, it seems like your attempts to pass -Cembed-bitcode=yes using RUSTFLAGS didn't make it through. I dont think docker containers inherit the environment of the runner. Something that could be tried is adding

maturin_docker_options: -e RUSTFLAGS="-Cembed-bitcode=yes"

to the matrix task for ppc64le, which would pass the environment flag to the container.

I am unable to test it until late night today or potentially tomorrow (I'm CEST), so dropping it here in case someone can give it a shot. Otherwise, will attempt to look more into it myself by running the docker containers locally.

Again, sorry for the issues this has caused!

davfsa avatar Aug 10 '24 08:08 davfsa

First of all, sorry for making this an issue right at the time of release. Wish I could have caught this earlier.

That's not your fault, we should have tested the release builds with your change :)

I dont think docker containers inherit the environment of the runner.

I think Maturin specifically uses -e RUSTFLAGS to pass the variable to the container, I can see this passed to docker run in the logs — that said, I'm not a Maturin expert and cannot confirm the flag was used.

Thanks for following up!

zanieb avatar Aug 10 '24 13:08 zanieb

You were right!

It seems like it does get passed correctly, but maturin does seem to ignore them, as the way to specify them is in the toml (which doesn't allow specifying per type flags)

I'll keep investigating. Getting the docker image to run locally is being quite the struggle

davfsa avatar Aug 11 '24 10:08 davfsa

Might be easiest to run the action in a fork and just comment out the rest of the jobs.

zanieb avatar Aug 11 '24 14:08 zanieb