Skip to content

Use object crate from crates.io to fix windows build error #143492

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

Merged
merged 2 commits into from
Jul 6, 2025
Merged

Use object crate from crates.io to fix windows build error #143492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a839b07
Use `object` crate from crates.io to fix windows build error
Diggsey Jul 5, 2025
ea7b756
Make tempfile a normal dependency
Diggsey Jul 5, 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
10 changes: 10 additions & 0 deletions compiler/rustc_codegen_gcc/Cargo.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,15 @@ dependencies = [
"libc",
]

[[package]]
name = "object"
version = "0.37.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "03fd943161069e1768b4b3d050890ba48730e590f57e56d4aa04e7e090e61b4a"
dependencies = [
"memchr",
]

[[package]]
name = "once_cell"
version = "1.20.2"
Expand Down Expand Up @@ -179,6 +188,7 @@ dependencies = [
"boml",
"gccjit",
"lang_tester",
"object",
"tempfile",
]

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_gcc/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ master = ["gccjit/master"]
default = ["master"]

[dependencies]
object = { version = "0.37.0", default-features = false, features = ["std", "read"] }
tempfile = "3.20"
gccjit = "2.7"
#gccjit = { git = "https://github.com/rust-lang/gccjit.rs" }

Expand All @@ -31,7 +33,6 @@ gccjit = "2.7"
[dev-dependencies]
boml = "0.3.1"
lang_tester = "0.8.0"
tempfile = "3.20"

[profile.dev]
# By compiling dependencies with optimizations, performing tests gets much faster.
Expand Down
7 changes: 2 additions & 5 deletions compiler/rustc_codegen_gcc/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,9 @@
#![deny(clippy::pattern_type_mismatch)]
#![allow(clippy::needless_lifetimes, clippy::uninlined_format_args)]

// Some "regular" crates we want to share with rustc
extern crate object;
// These crates are pulled from the sysroot because they are part of
// rustc's public API, so we need to ensure version compatibility.
extern crate smallvec;
Copy link
Member

Choose a reason for hiding this comment

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

So, we apparently run the risk of eventually having the same problem with smallvec. Should we just make this a crates.io dependency as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do - what's the policy here? Should only rustc_* crates be pulled from sysroot?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we have a policy, we just discovered this problem.^^

tracing needs to be pulled from the sysroot since it manages a singleton. Other than that, the main thing is avoiding duplicates... but also, if there's a SmallVec in a rustc type, then ofc it needs to use the sysroot version of the crate.

We can also just wait if one of the other crates becomes a problem. But it's going to be hard to notice that if we don't even build this on CI. So maybe the more important thing is to ensure these crates get check-built on more targets. @Kobzol might have an idea how to do that?

Copy link
Contributor

@antoyo antoyo Jul 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I used to pull as much as I could from the sysroot to save on compile time, but unfortunately, we have to stop doing this for Windows it seems.

Copy link
Member

Choose a reason for hiding this comment

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

These aren't very big crates we are talking about, so compile time shouldn't be a big deal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@antoyo this is not a windows-specific problem in general, it just so happens that in the windows build, the compiler doesn't load object beforehand, and there are multiple candidates.

That's why I asked about a policy - we need to say what crates are guaranteed to exist exactly once in the sysroot, as these can be pulled in safely, and no others.

Copy link
Member

@RalfJung RalfJung Jul 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It's hard to say which ones are guaranteed to exist only once (other than the rustc crates of course). OTOH, this in principle affects all rustc drivers (clippy, Miri, rustdoc) and hardly ever seems to be a problem... but also, at least in Miri we hardly use extern crate, so maybe that's why.

Copy link
Member

Choose a reason for hiding this comment

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

We can also just wait if one of the other crates becomes a problem. But it's going to be hard to notice that if we don't even build this on CI. So maybe the more important thing is to ensure these crates get check-built on more targets. @Kobzol might have an idea how to do that?

We don't check almost anything on CI, but we could build the GCC backend on more OSes. We haven't done that so far because it's not even in blocking (auto) CI on Linux yet, but that's for tests. For just building the backend, I guess we could try to add it to more runners.

Copy link
Member

Choose a reason for hiding this comment

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

Building might be harder than checking and require a full GCC... but maybe it can be done. Just seems like overkill when the main goal is to allow contributors to fix compilation failures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would make sense to do any further changes like re-exporting smallvec as a separate PR, so would be great if I could get an approval on this.

// FIXME(antoyo): clippy bug: remove the #[allow] when it's fixed.
#[allow(unused_extern_crates)]
extern crate tempfile;
#[macro_use]
extern crate tracing;

Expand Down
Loading