Correctly infer ranlib/ar from cross-gcc #163
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The GCC convention is that if the toolchain is named `$target-gnu-gcc`,
then ranlib and ar will be available as `$target-gnu-gcc-ranlib` and
`$target-gnu-gcc-ar` respectively. The code as written would infer them
to be `$target-gnu-{ranlib,ar}`, which won't work.
I'm not sure why the code that existed was written the way it was -- I
don't know of any GCC toolchains where the `-gcc` is stripped out in the
name of ranlib/ar. The file listing on Debian's [GCC 6.x] and [GCC 10.x]
all show the binaries using the `$target-gnu-gcc-$bin` format, as does
Arch Linux's [GCC 12.x].
This error appears to also be present in the `cc` crate. There, the
`-gcc` prefix is always stripped ([1][cc1], [2][cc2]), and then `-ar` is
[appended]. But its saving grace is that it also checks if the resulting
binary name is executable, and if it isn't falls back to the default AR
instead, which means the bad heuristic is likely often masked by the
presence of another working default AR. This crate does not (but perhaps
it should?).
[GCC 6.x]: https://packages.debian.org/stretch/gcc
[GCC 10.x]: https://packages.debian.org/stable/devel/gcc
[GCC 12.x]: https://archlinux.org/packages/core/x86_64/gcc/
[cc1]: https://github.com/rust-lang/cc-rs/blob/8daff16ce11e2753961c9b0ce3398fdeada6d941/src/lib.rs#L2623-L2626
[cc2]: https://github.com/rust-lang/cc-rs/blob/8daff16ce11e2753961c9b0ce3398fdeada6d941/src/lib.rs#L2769
[appended]: https://github.com/rust-lang/cc-rs/blob/8daff16ce11e2753961c9b0ce3398fdeada6d941/src/lib.rs#L2602-L2604
jonhoo
pushed a commit
to jonhoo/cc-rs
that referenced
this pull request
Oct 26, 2022
The GCC convention is that if the toolchain is named `$target-gnu-gcc`, then ar will be available as `$target-gnu-gcc-ar`. The code as written will infer it to be `$target-gnu-ar`, which won't work. I'm not sure why the code that existed was written the way it was -- I don't know of any GCC toolchains where the `-gcc` is stripped out in the name of ar. The file listing on Debian's [GCC 6.x] and [GCC 10.x] all show the binaries using the `$target-gnu-gcc-ar` format, as does Arch Linux's [GCC 12.x]. It may seem odd that the code always adds `-gcc` for that match arm, but this matches what we do for finding `$CC` and `$CXX` where we always inject the GNU prefix when target != host. Also note that there isn't a special `ar` for C++, so even when `self.cpp == true`, we should use `-gcc-ar`. Our saving grace, and likely the reason bug reports haven't come in about this, is that we also checks if the resulting binary name is executable, and if it isn't we fall back to the default AR instead. This means the bad heuristic is likely often masked by the presence of another working default AR. See also alexcrichton/openssl-src-rs#163. [GCC 6.x]: https://packages.debian.org/stretch/gcc [GCC 10.x]: https://packages.debian.org/stable/devel/gcc [GCC 12.x]: https://archlinux.org/packages/core/x86_64/gcc/
|
|
alexcrichton
added a commit
that referenced
this pull request
Oct 26, 2022
This reverts commit 31ef083.
|
Oh sorry I misinterpreted this and misunderstood what's happening, I've pushed a revert to I'll follow what the |
thomcc
pushed a commit
to rust-lang/cc-rs
that referenced
this pull request
Oct 26, 2022
The GCC convention is that if the toolchain is named `$target-gnu-gcc`, then ar will be available as `$target-gnu-gcc-ar`. The code as written will infer it to be `$target-gnu-ar`, which won't work. I'm not sure why the code that existed was written the way it was -- I don't know of any GCC toolchains where the `-gcc` is stripped out in the name of ar. The file listing on Debian's [GCC 6.x] and [GCC 10.x] all show the binaries using the `$target-gnu-gcc-ar` format, as does Arch Linux's [GCC 12.x]. It may seem odd that the code always adds `-gcc` for that match arm, but this matches what we do for finding `$CC` and `$CXX` where we always inject the GNU prefix when target != host. Also note that there isn't a special `ar` for C++, so even when `self.cpp == true`, we should use `-gcc-ar`. Our saving grace, and likely the reason bug reports haven't come in about this, is that we also checks if the resulting binary name is executable, and if it isn't we fall back to the default AR instead. This means the bad heuristic is likely often masked by the presence of another working default AR. See also alexcrichton/openssl-src-rs#163. [GCC 6.x]: https://packages.debian.org/stretch/gcc [GCC 10.x]: https://packages.debian.org/stable/devel/gcc [GCC 12.x]: https://archlinux.org/packages/core/x86_64/gcc/
|
Hmm, interesting. I still think trying with the gcc prefix is the right thing to do, but maybe we should also make sure we continue to support the binutils ar. |
jonhoo
pushed a commit
to jonhoo/cc-rs
that referenced
this pull request
Oct 26, 2022
sfackler [observed] that the `binutils` package [provides] binaries named `$target-ar` (with no `-gcc`). This patch augments the change made in rust-lang#735 to continue picking those up too. [observed]: alexcrichton/openssl-src-rs#163 (comment) [provides]: https://packages.debian.org/bullseye/amd64/binutils-aarch64-linux-gnu/filelist
jonhoo
pushed a commit
to jonhoo/cc-rs
that referenced
this pull request
Oct 26, 2022
sfackler [observed] that the `binutils` package [provides] binaries named `$target-ar` (with no `-gcc`). This patch augments the change made in rust-lang#735 to continue picking those up too. [observed]: alexcrichton/openssl-src-rs#163 (comment) [provides]: https://packages.debian.org/bullseye/amd64/binutils-aarch64-linux-gnu/filelist
jonhoo
pushed a commit
to jonhoo/openssl-src-rs
that referenced
this pull request
Oct 26, 2022
A second try at alexcrichton#163. The GCC convention is that if the toolchain is named `$target-gnu-gcc`, then ranlib and ar will be available as `$target-gnu-gcc-ranlib` and `$target-gnu-gcc-ar` respectively. The code as written would infer them to be `$target-gnu-{ranlib,ar}`, which will only work if the tools from `binutils` (which follow that convention) are on `$PATH`. I've also updated the logic in line with the `cc` crate to check that the inferred `AR`/`RANLIB` is actually executable before setting it as an override. See also rust-lang/cc-rs#736.
|
Second attempt in #164 that retains binutils support. |
thomcc
pushed a commit
to rust-lang/cc-rs
that referenced
this pull request
Oct 26, 2022
sfackler [observed] that the `binutils` package [provides] binaries named `$target-ar` (with no `-gcc`). This patch augments the change made in #735 to continue picking those up too. [observed]: alexcrichton/openssl-src-rs#163 (comment) [provides]: https://packages.debian.org/bullseye/amd64/binutils-aarch64-linux-gnu/filelist
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The GCC convention is that if the toolchain is named
$target-gnu-gcc, then ranlib and ar will be available as$target-gnu-gcc-ranliband$target-gnu-gcc-arrespectively. The code as written would infer them to be$target-gnu-{ranlib,ar}, which won't work.I'm not sure why the code that existed was written the way it was -- I don't know of any GCC toolchains where the
-gccis stripped out in the name of ranlib/ar. The file listing on Debian's GCC 6.x and GCC 10.x all show the binaries using the$target-gnu-gcc-$binformat, as does Arch Linux's GCC 12.x.This error appears to also be present in the
cccrate. There, the-gccprefix is always stripped (1, 2), and then-aris appended. But its saving grace is that it also checks if the resulting binary name is executable, and if it isn't falls back to the default AR instead, which means the bad heuristic is likely often masked by the presence of another working default AR. This crate does not (but perhaps it should?).