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

Fix archiver detection for musl cross compilation #1404

Merged
merged 5 commits into from
Feb 24, 2025
Merged

Fix archiver detection for musl cross compilation #1404

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1005cd7
Fix archiver detection for musl cross compilation
NobodyXu Feb 15, 2025
a64bb2f
Replace use of array::into_iter with iter
NobodyXu Feb 15, 2025
98eb42b
Fix tests
NobodyXu Feb 15, 2025
8e226da
Fix import of Stdio
NobodyXu Feb 15, 2025
32665bc
Pass --version when detecting archiver
NobodyXu Feb 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ use std::io::{self, Write};
use std::path::{Component, Path, PathBuf};
#[cfg(feature = "parallel")]
use std::process::Child;
use std::process::Command;
use std::process::{Command, Stdio};
use std::sync::{
atomic::{AtomicU8, Ordering::Relaxed},
Arc, RwLock,
Expand Down Expand Up @@ -3226,7 +3226,6 @@ impl Build {
}
});

let default = tool.to_string();
let tool = match tool_opt {
Some(t) => t,
None => {
Expand Down Expand Up @@ -3287,32 +3286,39 @@ impl Build {
self.cmd(&name)
} else if self.get_is_cross_compile()? {
match self.prefix_for_target(&self.get_raw_target()?) {
Some(p) => {
Some(prefix) => {
// GCC uses $target-gcc-ar, whereas binutils uses $target-ar -- try both.
// Prefer -ar if it exists, as builds of `-gcc-ar` have been observed to be
// outright broken (such as when targeting freebsd with `--disable-lto`
// toolchain where the archiver attempts to load the LTO plugin anyway but
// fails to find one).
//
// The same applies to ranlib.
let mut chosen = default;
for &infix in &["", "-gcc"] {
let target_p = format!("{}{}-{}", p, infix, tool);
if Command::new(&target_p).output().is_ok() {
chosen = target_p;
break;
}
}
let chosen = ["", "-gcc"]
.iter()
.filter_map(|infix| {
let target_p = format!("{prefix}{infix}-{tool}");
let status = Command::new(&target_p)
.arg("--version")
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::null())
.status()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason why we don't invoke which $target_p instead? It seems like that would have the same effect of determining if the command exists (and it would be easier to optimize/understand, since we could implement the which algorithm ourselves).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, because which might not be available?

https://docs.rs/which/latest/which/

There's a crate that we could use for it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Which also has quite a few deps unfortunately. The core logic would be re-implementable in cc though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Which also has quite a few deps unfortunately. The core logic would be re-implementable in cc though.

Just slightly worried about having more code to be maintained.

.ok()?;
status.success().then_some(target_p)
})
.next()
.unwrap_or_else(|| tool.to_string());
name = chosen.into();
self.cmd(&name)
}
None => {
name = default.into();
name = tool.into();
self.cmd(&name)
}
}
} else {
name = default.into();
name = tool.into();
self.cmd(&name)
}
}
Expand Down