Use object crate from crates.io to fix windows build error
#143492
Conversation
|
rustbot has assigned @compiler-errors. Use |
rustbot
added
S-waiting-on-review
|
Some changes occurred in compiler/rustc_codegen_gcc |
|
Can you please confirm that this is sufficient to fix the build on Windows? |
|
It certainly fixed With this change this error does not occur. |
| @@ -27,7 +27,6 @@ | |||
| #![allow(clippy::needless_lifetimes, clippy::uninlined_format_args)] | |||
|
|
|||
| // Some "regular" crates we want to share with rustc | |||
| extern crate object; | |||
| extern crate smallvec; | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Can do - what's the policy here? Should only rustc_* crates be pulled from sysroot?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
These aren't very big crates we are talking about, so compile time shouldn't be a big deal.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
I don't have any suggestions/comment but just want to say thank you @Diggsey for doing this. |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
Rollup of 4 pull requests Successful merges: - #143252 (Rewrite empty attribute lint for new attribute parser) - #143492 (Use `object` crate from crates.io to fix windows build error) - #143514 (Organize macro tests a bit more) - #143518 (rustc_builtin_macros: Make sure registered attributes stay sorted) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #143492 - Diggsey:db-fix-object-ambiguity, r=RalfJung Use `object` crate from crates.io to fix windows build error See https://rust-lang.zulipchat.com/#narrow/channel/386786-rustc-codegen-gcc/topic/Cannot.20compile.20rustc.20on.20Windows.2010/with/527240094
See https://rust-lang.zulipchat.com/#narrow/channel/386786-rustc-codegen-gcc/topic/Cannot.20compile.20rustc.20on.20Windows.2010/with/527240094