-
Notifications
You must be signed in to change notification settings - Fork 484
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
Compilation failure on LLVM 19 with *-gnullvm
#1167
Comments
cc @ChrisDenton Do we want to remove the |
Hmm, it looks like Details--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1939,7 +1939,10 @@ impl Build {
cmd.push_opt_unless_duplicate(format!("-O{}", opt_level).into());
}
- if cmd.is_like_clang() && target.contains("windows") {
+ if cmd.is_like_clang()
+ && target.contains("windows")
+ && !target.ends_with("-gnullvm")
+ {
// Disambiguate mingw and msvc on Windows. Problem is that
// depending on the origin clang can default to a mismatchig
// run-time.
@@ -2190,6 +2193,10 @@ impl Build {
}
cmd.push_cc_arg(format!("--target={}", target).into());
+ } else if target.ends_with("-gnullvm") {
+ cmd.push_cc_arg(
+ format!("--target={}", target.strip_suffix("llvm").unwrap()).into(),
+ );
} else {
cmd.push_cc_arg(format!("--target={}", target).into());
}
@@ -3467,6 +3474,7 @@ impl Build {
"hexagon-unknown-linux-musl" => Some("hexagon-linux-musl"),
"i586-unknown-linux-musl" => Some("musl"),
"i686-pc-windows-gnu" => Some("i686-w64-mingw32"),
+ "i686-pc-windows-gnullvm" => Some("i686-w64-mingw32"),
"i686-uwp-windows-gnu" => Some("i686-w64-mingw32"),
"i686-unknown-linux-gnu" => self.find_working_gnu_prefix(&[
"i686-linux-gnu", (the |
Thanks, that looks good, can you open a PR? I would review it and also ask @ChrisDenton to review it, since they are more familiar with windows than me. |
I think using Rust triple is unfortunate. Many other triples won't match LLVM, like Win7 triple if we stick to Windows. That said I don't know if it's possible to obtain LLVM triple from rustc so adding workarounds for all the triples might be the only way. |
On nightly you can use I guess a test could compare the cc-rs inferred LLVM targets against the nightly |
cc @kleisauke @mati865 @ChrisDenton if it is available in nightly, then we can pre-generate it like we did for riscv:
gen-target-info is run on CI every week and create a PR if something changes, and it also can be run locally. The generated info would then be committed and included in crate publish. |
Yeah, it's been available in night for over a year. I don't know how |
Basically, it creates a rust source file in It's usually run in CI per week, or on the developer machine. If the generated source file differs, then you need to commit and open a PR. |
Sounds good but I won't be able to look into it probably until the weekend. |
@kleisauke how can I test this issue easily? |
@mati865 Thanks for opening PR #1176! AFAIK, you can reproduce this error when building a crate with Testing this with librsvg is a bit difficult because you have to cross-compile its dependencies as well. Also, you need to disable any Otherwise, you'll encounter these errors: Note that in the starting post Lines 1941 to 1946 in 5863b31
(perhaps this whole block can be removed?) |
Oh, this complicates things as I won't be able to easily test the fix with Import errors were discussed a bit in msys2/MINGW-packages#21017, it affects all windows targets other than
I doubt adding the argument twice causes any problems; the second occurrence should override the first one. It can be probably removed, but I'm not planning to spend time on testing it. |
I can confirm PR #1176 fixes this, tested with commit libvips/build-win64-mxe@5b050d1.
Interesting, thanks for linking and investigating that issue. |
Great, thanks! |
The logic at:
cc-rs/src/lib.rs
Lines 1930 to 1935 in dcd8ed3
(introduced in PR #825)
Breaks with LLVM 19 (rust-lang/rust#127513) when targeting
*-pc-windows-gnullvm
due to PR llvm/llvm-project#78655.Details
/cc @mati865 FYI.
The text was updated successfully, but these errors were encountered: