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

Consider building/linking run-make-support as a dylib instead of a static lib #131810

Closed
Zalathar opened this issue Oct 17, 2024 · 5 comments · Fixed by #132225
Closed

Consider building/linking run-make-support as a dylib instead of a static lib #131810

Zalathar opened this issue Oct 17, 2024 · 5 comments · Fixed by #132225
Labels
A-run-make Area: port run-make Makefiles to rmake.rs C-discussion Category: Discussion or questions that doesn't represent real issues. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@Zalathar
Copy link
Contributor

While looking at the code that compiletest uses to build auxiliary crates as dylibs, it occurred to me that run-make tests could potentially benefit from linking to run-make-support dynamically.

This could perhaps result in less bloated rmake executables and faster builds/rebuilds, though that's pure conjecture as I haven't actually tried it. On the other hand, it's possible that the extra effort/complexity required to do so might not be worth it, especially if some of that complexity is in bootstrap.

@Zalathar Zalathar added A-run-make Area: port run-make Makefiles to rmake.rs C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 17, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 17, 2024
@Zalathar
Copy link
Contributor Author

I mostly wanted to make this issue to get the idea out of my head. It's a neat idea, but might not be worth spending time on.

@jieyouxu jieyouxu added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 17, 2024
@jieyouxu
Copy link
Member

This is possible (though note that some targets don't support dynamic linking, but ig they can fallback to static in that case). run-make-support can also not be built as a stage0 tool lib, it can be built as a regular lib by stage N compiler for --stage N. It was implemented the way it is currently because that was the easiest at the time when I initially added it.

@jieyouxu jieyouxu added E-needs-design This issue needs exploration and design to see how and if we can fix/implement it C-discussion Category: Discussion or questions that doesn't represent real issues. and removed E-needs-design This issue needs exploration and design to see how and if we can fix/implement it C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Oct 17, 2024
@bjorn3
Copy link
Member

bjorn3 commented Oct 17, 2024

It should be a matter of adding [lib] crate-type = ["lib", "dylib"] to run-make-support/Cargo.toml and passing -Cprefer-dynamic to all rmake.rs compilations. This should seamlessly handle host triples where dylibs are not supported because cargo ignores crate types not supported by the target. It may be necessary to also explicitly pass -Cprefer-dynamic when compiling run-make-support.

@lolbinarycat
Copy link
Contributor

isn't the lib crate type already basically dylib+staticlib? i'm not sure we don't just need -Cprefer-dynamic, tbh.

@bjorn3
Copy link
Member

bjorn3 commented Oct 19, 2024

No, the lib crate types is an alias for the rlib crate type. You can't dynamically link an rlib. You need to use the dylib crate type for that. And in addition in the case of run-make-support we need the lib/rlib crate type to support targets that don't have dynamic linking support.

@bors bors closed this as completed in afc93a9 Oct 28, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 28, 2024
Rollup merge of rust-lang#132225 - clubby789:run-make-dynamic, r=jieyouxu

Dynamically link run-make support

Fixes rust-lang#131810

r? `@jieyouxu`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs C-discussion Category: Discussion or questions that doesn't represent real issues. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants