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

Do not link -ldl and -lpthread unconditionally #2949

Conversation

CohenArthur
Copy link
Member

No description provided.

@CohenArthur CohenArthur force-pushed the do-not-link-against-dl-pthread-unconditionally branch from bd03e12 to 9721ca7 Compare April 12, 2024 14:56
@tschwinge
Copy link
Member

Yes, that's going in the right direction. 👍

At least partially, this again (like #2942 "Check for cargo when building rust language") needs to go into the top-level configure, in order to be able to gracefully disable GCC/Rust for --enable-languages=all configurations.

@CohenArthur CohenArthur force-pushed the do-not-link-against-dl-pthread-unconditionally branch from 9721ca7 to 6d95466 Compare April 12, 2024 15:04
@CohenArthur
Copy link
Member Author

Yes, that's going in the right direction. 👍

At least partially, this again (like #2942 "Check for cargo when building rust language") needs to go into the top-level configure, in order to be able to gracefully disable GCC/Rust for --enable-languages=all configurations.

yeah, I saw that Jakub had mentioned this but it does seem a bit too late to make such a change, no? Jakub had also mentioned erroring out, which is what I chose to do here

@CohenArthur CohenArthur marked this pull request as ready for review April 12, 2024 15:05
@tschwinge
Copy link
Member

It's late no matter whether in the top-level configure, or gcc/configure. ;-|

@tschwinge
Copy link
Member

Gracefully disable for implicit "rust" in --enable-languages=all, error out for explicit --enable-languages=rust.

@tschwinge
Copy link
Member

Only crab1 should then be linked against these libraries, not all GCC things generally.

@CohenArthur
Copy link
Member Author

Only crab1 should then be linked against these libraries, not all GCC things generally.

right, so add -ldl -lpthread to something like CRAB1_LIBS instead? I think we have RUST_LIBS for this already, I'll check.

Gracefully disable for implicit "rust" in --enable-languages=all, error out for explicit --enable-languages=rust.

so you're forcing me to do more autotools work, do you like seeing me suffer or something? :P will work on this next

@tschwinge
Copy link
Member

The implicit vs. explicit thing should be really easy ;-) in the top-level configure: just like how #2942 "Check for cargo when building rust language" does this, and similarly for GCC/D if gdc or GCC/Ada if gnat is not available in a suitable version.

@CohenArthur CohenArthur force-pushed the do-not-link-against-dl-pthread-unconditionally branch 3 times, most recently from df478ee to 7399ae7 Compare April 16, 2024 13:02
@CohenArthur
Copy link
Member Author

@tschwinge I've made the required changes and pushed them :) tested locally and it seems to work fine on a system where glibc < 2.34. tested with --enable-languages=all and --enable-languages=rust

@CohenArthur CohenArthur force-pushed the do-not-link-against-dl-pthread-unconditionally branch 5 times, most recently from 67cad72 to c34b0b2 Compare April 19, 2024 09:44
@P-E-P
Copy link
Member

P-E-P commented May 21, 2024

Should we merge this ? Has it already been merged upstream ? It requires a rebase anyway.

@CohenArthur CohenArthur force-pushed the do-not-link-against-dl-pthread-unconditionally branch from c34b0b2 to f8d2770 Compare May 31, 2024 10:19
ChangeLog:

	* Makefile.tpl: Add CRAB1_LIBS variable.
	* Makefile.in: Regenerate.
	* configure: Regenerate.
	* configure.ac: Check if -ldl and -lpthread are needed, and if so, add
	them to CRAB1_LIBS.

gcc/rust/ChangeLog:

	* Make-lang.in: Remove overazealous LIBS = -ldl -lpthread line, link
	crab1 against CRAB1_LIBS.
@CohenArthur CohenArthur force-pushed the do-not-link-against-dl-pthread-unconditionally branch from f8d2770 to 00669b6 Compare June 10, 2024 08:32
@CohenArthur
Copy link
Member Author

I'm going to merge this PR because it has been merged upstream, sadly it introduce a regression because I'm too stupid for autotools and shell scripting in general, so I'll open a follow-up PR shortly

@CohenArthur CohenArthur added this pull request to the merge queue Jun 12, 2024
Merged via the queue into Rust-GCC:master with commit a45362e Jun 12, 2024
9 checks passed
@CohenArthur
Copy link
Member Author

turns out that the bug/issue does not actually prevent the build from completing, it's just noisy but the build completes. I'll still push the fix ASAP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants