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

Prefer -ar to -gcc-ar #741

Merged
merged 1 commit into from
Oct 31, 2022
Merged

Prefer -ar to -gcc-ar #741

merged 1 commit into from
Oct 31, 2022

Conversation

nagisa
Copy link
Member

@nagisa nagisa commented Oct 30, 2022

As observed in cross-rs/cross#1100, in some situations *-gcc-ar might actually be just broken.

I believe this is most likely just a plain bug in gcc (failing to consider --disable-lto configuration option somewhere in their build setup,) but for the time being *-ar tends to avoid this problem altogether.

Code added in #736 appears to be preferring *-gcc-ar, but no strong rationale is given to prefer one over the other. *-gcc-ar being outright broken sometimes seems like a rationale strong enough to continue preferring binutils’ ar.

As observed in cross-rs/cross#1100, in some
situations `*-gcc-ar` might actually be just broken.

I believe this is most likely just a plain bug in gcc (failing to
consider `--disable-lto` configuration option somewhere in their build
setup,) but for the time being `*-ar` tends to avoid this problem
altogether.

Code added in rust-lang#736 appears to be
preferring `*-gcc-ar`, but no strong rationale is given to prefer one
over the other. `*-gcc-ar` being outright broken sometimes seems like a
rationale strong enough to continue preferring binutils’ `ar`.
Copy link

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ideally we'd prefer whichever one is "newer", but yeah, given that you've observed gcc-ar being broken, seems fine to me to swap the preference 👍

jonhoo pushed a commit to jonhoo/openssl-src-rs that referenced this pull request Oct 31, 2022
Sometimes `gcc-ar` is broken, so when both are available we should
prefer non-gcc-ar. See also rust-lang/cc-rs#741.
@jonhoo
Copy link

jonhoo commented Oct 31, 2022

I've also updated alexcrichton/openssl-src-rs#164 to have the updated preference.

Copy link
Member

@thomcc thomcc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, seems good to me.

@thomcc
Copy link
Member

thomcc commented Oct 31, 2022

I'll get this in a release this weekend (hopefully sooner), sorry for the issues.

@Emilgardis
Copy link

What's the status on getting this released?

@thomcc
Copy link
Member

thomcc commented Nov 8, 2022

Very soon. Release notes might be delayed a little but should be published within the next day, probably (still not automated and for various reasons is a multi-person ordeal)

@thomcc
Copy link
Member

thomcc commented Nov 8, 2022

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.

4 participants