Skip to content
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.

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

Problems updating the cc crate version in bootstrap #124565

Open
jfgoog opened this issue Apr 30, 2024 · 2 comments
Open

Problems updating the cc crate version in bootstrap #124565

jfgoog opened this issue Apr 30, 2024 · 2 comments
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jfgoog
Copy link
Contributor

jfgoog commented Apr 30, 2024

This report is to document the issues I encountered in trying to update the version of the cc crate in src/bootstrap/Cargo.toml to a more recent version.

This is essentially a summary of the commentary in #122504 so the information is preserved.

As of April 2024, the cc crate was pinned to v1.0.73, which is about 2 years old. Pinning was added in 8d0f888 due to problems encountered when updating to a newer version.

In the course of my work on this, I identified the following changes to the cc crate that require fixes to the bootstrap code or tests:

In cc v1.0.78, object files are hash-prefixed

As of cc v1.0.78, object files are prefixed with a 16-character hash. The code in src/bootstrap/src/core/build_steps/llvm.rs assumed that foo.c compiles to foo.o, and as a result we are unable tobuild libunwind and libcrt. #122504 fixes this, but note that the fix requires cc v1.0.86, which introduces the compile_intermediates method. Without this method, we would need to scan all the files in the directory.

In cc v1.0.86, -mmacosx-version-min is used when compiling on MacOS

Improvements to MacOS support in cc v1.0.86 included passing the -mmacosx-version-min flag when compiling. The value of this flag is determined by running xcrun --show-sdk-platform-version --sdk macosx.

Unfortunately, a long-standing bug in the CMake rules for compiler-rt causes architecture detection to be skipped when this flag is passed: llvm/llvm-project#88780

A workaround to suppress this flag is in #122504

In cc v1.0.91, target parsing requires that targets contain a '-'

rust-lang/cc-rs@abf67d7 introduces new validation code when parsing targets that requires them to contain a dash character.

In src/bootstrap/src/core/builder/tests.rs, we use targets named "A", "B", and "C", which the cc crate fails to parse.

To address this, we can change target "A" to "A-A", which is sufficient to make the tests pass.

In cc v1.0.74, the value of SDKROOT is used on Darwin, if present

The value of the -isysroot flag on Darwin normally determined by xcrun --show-sdk-path. However, in cc v1.0.74, the value of the SDKROOT environment variable is preferred, if it is set.

Darwin's system Python interpreter sets a value for SDKROOT if one is not provided. So when x.py executes the bootstrap code, SDKROOT is guaranteed to always be set.

But clearing SDKROOT in x.py or bootstrap.py causes problems for rustc's CI on GitHub. As explained in src/ci/scripts/install-clang.sh:

    # macOS 10.15 onwards doesn't have libraries in /usr/include anymore: those
    # are now located deep into the filesystem, under Xcode's own files. The
    # native clang is configured to use the correct path, but our custom one
    # doesn't. This sets the SDKROOT environment variable to the SDK so that
    # our own clang can figure out the correct include path on its own.
    ciCommandSetEnv SDKROOT "$(xcrun --sdk macosx --show-sdk-path)"

So SDKROOT is always set, and always wrong when we are cross-compiling. As a result, there are multiple places where we detect and workaround values of SDKROOT that seem to be wrong: compiler/rustc_target/src/spec/base/apple/mod.rs and compiler/rustc_codegen_ssa/src/back/link.rs.

Handling of SDKROOT is fixed in cc v1.0.97, by rejecting values that are clearly wrong

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 30, 2024
@jieyouxu jieyouxu added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Apr 30, 2024
@jfgoog
Copy link
Contributor Author

jfgoog commented Apr 30, 2024

Patch for fixing target names in src/bootstrap/src/core/builder/tests.rs:

fake-targets.patch

@onur-ozkan
Copy link
Member

Thank you for providing a comprehensive report on this!

@onur-ozkan onur-ozkan removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

4 participants