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.

Sign up for GitHub

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

Jump to bottom

Do not link -ldl and -lpthread unconditionally #2949

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Do not link -ldl and -lpthread unconditionally #2949

merged 1 commit into from
Jun 12, 2024

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 •
edited
Loading

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

@P-E-P P-E-P mentioned this pull request May 28, 2024
Draft
@CohenArthur CohenArthur force-pushed the do-not-link-against-dl-pthread-unconditionally branch from c34b0b2 to f8d2770 Compare May 31, 2024 10:19
@CohenArthur
rust: Do not link with libdl and libpthread unconditionally
00669b6 
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

@CohenArthur CohenArthur mentioned this pull request Jun 13, 2024
Closed
@CohenArthur CohenArthur added this to the GCC 15.1 milestone Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@P-E-P P-E-P P-E-P approved these changes

@tschwinge tschwinge Awaiting requested review from tschwinge

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
GCC 15.1
Development

Successfully merging this pull request may close these issues.
3 participants
@CohenArthur @tschwinge @P-E-P