update clippy to fix toolstate #66158
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
r? @Manishearth |
|
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ |
|
📌 Commit 8ec83cf has been approved by |
bors
added
S-waiting-on-bors
|
@bors p=2 |
|
cc @msizanoen1 in case they are interested. |
pietroalbini
added a commit
to pietroalbini/rust
that referenced
this pull request
Nov 6, 2019
update clippy to fix toolstate Close rust-lang#66150. Close rust-lang#62558 thanks to @msizanoen1 .
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@bors r- |
bors
added
S-waiting-on-author
|
This is because some of our tests import @Mark-Simulacrum we've got some tests for internal lints so we'd like to keep doing this, is there a way for us to continue to |
|
Opened rust-lang/rust-clippy#4784 , however this means that we're unable to test internal lints. Is there a reason why clippy is getting loaded as a plugin there? I can't reproduce this locally with rustup-tooclhain-install-master. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
I think I know why: the (now removed) deprecated clippy plugin causes clippy_lints to be build twice - once in the host deps directory and the target deps directory. |
|
The best path forward might be to only run the internals test on our CI and
not rust
…On Wed, Nov 6, 2019, 6:02 PM msizanoen1 ***@***.***> wrote:
I think I know why: the deprecated clippy plugin causes clippy_lints to be
build twice - once in the host deps directory and the target deps directory.
After the plugin removal, clippy_lints is only built once into the target
deps directory. However clippy tests expect it to be in the host deps
directory.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#66158?email_source=notifications&email_token=AAMK6SHDOUMCP6OLCVQ3HD3QSNZJVA5CNFSM4JJV7NNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDIUINA#issuecomment-550585396>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMK6SBNPCEZYWG65ERR7YLQSNZJVANCNFSM4JJV7NNA>
.
|
|
Could we add a new feature gate in clippy_lints for solely testing? |
|
How about this: diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index 60f0dccdb07..76a5c88b5f0 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -569,6 +569,7 @@ impl Step for Clippy {
cargo.env("RUSTC_LIB_PATH", builder.rustc_libdir(compiler));
let host_libs = builder
.stage_out(compiler, Mode::ToolRustc)
+ .join(&self.host)
.join(builder.cargo_dir());
cargo.env("HOST_LIBS", host_libs);
// clippy tests need to find the driver |
|
Testing that patch locally. |
|
It still failed with another error: diff of stderr:
+error[E0463]: can't find crate for `clippy_mini_macro_test`
+ --> $DIR/procedural_macro.rs:4:1
+ |
+LL | extern crate clippy_mini_macro_test;
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0463`.
+ |
|
After rust-lang/rust-clippy#4786 is merged, reupdate clippy and apply this patch: diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index 60f0dccdb07..a858ed42bad 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -570,7 +570,12 @@ impl Step for Clippy {
let host_libs = builder
.stage_out(compiler, Mode::ToolRustc)
.join(builder.cargo_dir());
+ let target_libs = builder
+ .stage_out(compiler, Mode::ToolRustc)
+ .join(&self.host)
+ .join(builder.cargo_dir());
cargo.env("HOST_LIBS", host_libs);
+ cargo.env("TARGET_LIBS", target_libs);
// clippy tests need to find the driver
cargo.env("CLIPPY_DRIVER_PATH", clippy);
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Blocked on rust-lang/rust-clippy#4786 |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-blocked
bors
added a commit
to rust-lang/rust-clippy
that referenced
this pull request
Nov 7, 2019
Remove `extern crate clippy_lints` from tests This causes rustc's build system to fail because it still tries to load the crate as a plugin: rust-lang/rust#66158 (comment) . I'm not sure _why_ this happens, but for a short term fix we should remove these. In one case it was just a convenient crate to use so i picked a different test. In another it was load-bearing, I had to delete the test. Idk if there's a better way around this. changelog: none
|
superseded by #66207 |
bors
added a commit
to rust-lang/rust-clippy
that referenced
this pull request
Nov 8, 2019
Add the TARGET_LIBS environment variable for rustc CI testing Needed to fix the test failure in rust-lang/rust#66158. See rust-lang/rust#66158 (comment) r? @Manishearth
bors
added a commit
to rust-lang/rust-clippy
that referenced
this pull request
Nov 8, 2019
Add the TARGET_LIBS environment variable for rustc CI testing Needed to fix the test failure in rust-lang/rust#66158. See rust-lang/rust#66158 (comment) r? @Manishearth changelog: none
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close #66150.
Close #62558 thanks to @msizanoen1 .