Restrict sysroot crate imports to those defined in this repo. #143548
Conversation
rustbot
added
A-attributes
|
Some changes occurred in compiler/rustc_codegen_ssa Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in src/tools/compiletest cc @jieyouxu Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt Some changes occurred in compiler/rustc_attr_data_structures Some changes occurred in compiler/rustc_attr_parsing Some changes occurred in compiler/rustc_codegen_gcc The Miri subtree was changed cc @rust-lang/miri Some changes occurred in exhaustiveness checking cc @Nadrieril Some changes occurred in compiler/rustc_sanitizers cc @rcvalle
|
Diggsey
force-pushed
the
db-limit-extern-crate-usage
branch
from
8e8dd82 to
c613bce
Compare
|
The Miri subtree was changed cc @rust-lang/miri Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt Some changes occurred in compiler/rustc_codegen_gcc |
This comment has been minimized.
This comment has been minimized.
|
Do you think it is feasible to add an internal lint that rejects |
|
@oli-obk I'll look into this. How does testing work for sub-trees? Would each sub-tree need its own copy of the lint, or do the subtree repos have some way to run rust-wide lints? |
|
"internal" lints live in rustc like normal lints, but are gated behind a nightly-only flag. Miri enables them in their CI, I don't know if |
|
We have a list of internal lints that rustc has and enables everywhere. We'd have to limit it to crates with |
|
Not sure what you mean about restricting it, the lints need |
|
Ah right, my bad |
Restrict sysroot crate imports to those defined in this repo. It's common to import dependencies from the sysroot via `extern crate` rather than use an explicit cargo dependency, when it's necessary to use the same dependency version as used by rustc itself. However, this is dangerous for crates.io crates, since rustc may not pull in the dependency on some targets, or may pull in multiple versions. In both cases, the `extern crate` fails to resolve. To address this, re-export all such dependencies from the appropriate `rustc_*` crates, and use this alias from crates which would otherwise need to use `extern crate`. See #143492 for an example of the kind of issue that can occur.
This comment has been minimized.
This comment has been minimized.
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
It's common to import dependencies from the sysroot via `extern crate` rather than use an explicit cargo dependency, when it's necessary to use the same dependency version as used by rustc itself. However, this is dangerous for crates.io crates, since rustc may not pull in the dependency on some targets, or may pull in multiple versions. In both cases, the `extern crate` fails to resolve. To address this, re-export all such dependencies from the appropriate `rustc_*` crates, and use this alias from crates which would otherwise need to use `extern crate`.
Diggsey
force-pushed
the
db-limit-extern-crate-usage
branch
from
79d57fa to
c6a9521
Compare
|
Looks like we pull |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 4f08307 (parent) -> 28d0a4a (this PR) Test differencesShow 244 test diffsStage 0
Stage 1
(and 138 additional test diffs) Additionally, 6 doctest diffs were found. These are ignored, as they are noisy. Job group index Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 28d0a4a205f9e511ad2f51ee79a4aa19a704a455 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (28d0a4a): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 4.1%, secondary 2.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -3.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -1.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 476.148s -> 474.854s (-0.27%) |
| .unwrap_or(span) | ||
| .id() | ||
| .into_u64() | ||
| span.scope() |
There was a problem hiding this comment.
Uh, what is happening here?
This PR has introduced a bunch of nonsensical whitespace changes across various Miri files -- none of which rustfmt is able to fix, apparently.
There was a problem hiding this comment.
Can you please submit a follow-up PR to fix these? Preferably in the Miri repo.
There was a problem hiding this comment.
Ugh, I'm not sure what happened here? All the affected files are excluded from rustfmt, so presumably a rebase caused these issues and then they didn't get caught in CI, but I have no idea why rebasing would break the whitespace like this.
There was a problem hiding this comment.
(None of these files had conflicts during rebase...)
There was a problem hiding this comment.
Yeah rust-lang/miri#4635 is still in the queue. I guess we can also land it via rust-lang.
…RalfJung Revert unintentional whitespace changes to rustfmt-excluded file Accidentally introduced in rust-lang#143548
Revert unintentional whitespace changes to rustfmt-excluded file Accidentally introduced in rust-lang/rust#143548
…r=samueltardieu Fix whitespace in rustfmt-excluded clippy file Accidentally introduced in rust-lang#143548
…r=samueltardieu Fix whitespace in rustfmt-excluded clippy file Accidentally introduced in rust-lang#143548
12 participants
It's common to import dependencies from the sysroot via
extern craterather than use an explicit cargo dependency, when it's necessary to use the same dependency version as used by rustc itself. However, this is dangerous for crates.io crates, since rustc may not pull in the dependency on some targets, or may pull in multiple versions. In both cases, theextern cratefails to resolve.To address this, re-export all such dependencies from the appropriate
rustc_*crates, and use this alias from crates which would otherwise need to useextern crate.See #143492 for an example of the kind of issue that can occur.