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

Introduce RUSTC_STATIC_CLANG_RT_PATH and RUSTC_STATIC_UNWIND_PATH envs #91765

Closed
wants to merge 1 commit into from

Conversation

catap
Copy link

@catap catap commented Dec 11, 2021

The goal of this envirements variable is easy statically link libclang_rt or libunwind into rust.

It also introduces a way to easy extend rust build in a way which allows to inject more static builds for some rare system if it required.

It haven't been documented because it seems as very deep hack and the user who needs it will discover this hack by reading sources ;)

Closes: #59164

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @davidtwco (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 11, 2021
@catap catap force-pushed the rustc-llvm-libclang-rt branch 2 times, most recently from 38b6b55 to 354d5f9 Compare December 11, 2021 02:17
@catap catap changed the title Introduce RUSTC_LLVM_STATIC_CLANG_RT_PATH env Introduce RUSTC_STATIC_CLANG_RT_PATH env Dec 11, 2021
@catap catap force-pushed the rustc-llvm-libclang-rt branch 2 times, most recently from 9501f5e to c539830 Compare December 13, 2021 10:55
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 13, 2021
@rust-log-analyzer

This comment has been minimized.

@catap catap force-pushed the rustc-llvm-libclang-rt branch from c539830 to 7df5d02 Compare December 13, 2021 12:20
@catap catap changed the title Introduce RUSTC_STATIC_CLANG_RT_PATH env Introduce RUSTC_STATIC_CLANG_RT_PATH and RUSTC_STATIC_UNWIND_PATH env Dec 13, 2021
@rust-log-analyzer

This comment has been minimized.

@catap catap force-pushed the rustc-llvm-libclang-rt branch from 7df5d02 to 0952de6 Compare December 13, 2021 12:32
@catap catap changed the title Introduce RUSTC_STATIC_CLANG_RT_PATH and RUSTC_STATIC_UNWIND_PATH env Introduce RUSTC_STATIC_CLANG_RT_PATH and RUSTC_STATIC_UNWIND_PATH envs Dec 13, 2021
@rust-log-analyzer

This comment has been minimized.

… envs

The goal of this envirements variable is easy statically link
`libclang_rt` or `libunwind` into rust.

It also introduces a way to easy extend rust build in a way which allows
to inject more static builds for some rare system if it required.

It haven't been documented because it seems as very deep hack and the
user who needs it will discover this hack by reading sources ;)
@catap catap force-pushed the rustc-llvm-libclang-rt branch from 0952de6 to 12bc843 Compare December 13, 2021 12:37
@davidtwco
Copy link
Member

This probably warrants a major change proposal as it adds a public-facing change (we'd have to support this environment variable going forwards).

@catap
Copy link
Author

catap commented Dec 13, 2021

@davidtwco not sure to be honest. This is very low-level hack that allows to inject some dependency when it needed and can't be discovered by cross building.

Another approach is adding (a lot) if's inside build.rs or start to use such hack.

From my point of view this hack is the best possible way because if I rebuild it to target platform where libunwind isn't discovered automatically the first place to start digging how to hack it (for me) is build.rs for library/unwind where I've discovered it and enjoyed it :)

But, still, I may not understand and estimate right the long term cost of supporting it but from my point of view it nothing. Am I wrong?

@davidtwco
Copy link
Member

I guess its more rustc developer/distribution-facing than user-facing, @Mark-Simulacrum do you know how we decide to add environment variables to build.rs for std/rustc?

@Mark-Simulacrum Mark-Simulacrum self-assigned this Dec 13, 2021
@Mark-Simulacrum
Copy link
Member

I agree this is distro/developer-facing. The requirement for adding this is mainly convincing someone, typically me or someone else familiar with our linking story (petrochenkov, often).

In this case, I'm not sure this is the right next step here -- can you elaborate on the motivation here? #59164 seems like a case where the library/crate being compiled should be including the library, rather than pushing for std to link it in.

libunwind already has llvm-libunwind option which, on some platforms, leads to a static link. clang_rt I don't think currently has such an option, but it seems plausible we could add it (rather than having an env variable ad-hoc providing that).

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 14, 2021
@davidtwco davidtwco removed their assignment Jan 5, 2022
@JohnCSimon
Copy link
Member

Ping from triage:
@catap can you please address the comment

In this case, I'm not sure this is the right next step here -- can you elaborate on the motivation here?

@catap
Copy link
Author

catap commented Jan 24, 2022

@JohnCSimon, @Mark-Simulacrum the root cause of motivation for this changes is attempt to build rust which can be run on macOS before 10.7.

Anyway, it still requires a few things, and right now I haven't got any time to finish this build.

If it fine, keep this PR open for a while and as soon as I achived that, I'll update/rebase this one and open a few more which is required.

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 13, 2022
@bors
Copy link
Contributor

bors commented Mar 5, 2022

☔ The latest upstream changes (presumably #94480) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon
Copy link
Member

@Mark-Simulacrum - @catap has replied to your question

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 23, 2022
@Mark-Simulacrum
Copy link
Member

I think if we're going to add configuration like this, it should go through config.toml and into the build scripts via rustbuild-set environment variables (or other means of passing configuration, perhaps existing ones).

For libunwind, it seems like we already try to support static linkage via crt-static and/or llvm-libunwind=in-tree. Maybe those options suffice here? If not, maybe this configuration should be gated on those rather than new configuration options?

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 24, 2022
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 22, 2022
@Dylan-DPC
Copy link
Member

@catap any update on this pr? thanks

@JohnCSimon
Copy link
Member

@catap
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you do please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Jun 19, 2022
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Jun 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C runtime library is not linked on macOS
9 participants