Skip to content

/crater

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

Add a new `clippy` mode to crater to test lint regressions #391

Merged
merged 4 commits into from Jan 26, 2019

Conversation

Reviewers

@pietroalbini pietroalbini

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@ecstatic-morse @pietroalbini @bors
@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented Jan 19, 2019 •
edited

Resolves #388.

This adds a new mode, clippy, to crater which runs clippy on crates (duh :).

I had to add some new options to minicrater, as well as change the invocation of docker create. See the commit messages for more info.

This is still a work in progress as I'd like to add some documentation for the feature, but should be enough for a meaningful review.

@ecstatic-morse ecstatic-morse force-pushed the ecstatic-morse:clippy-mode branch from 0681269 to c207b79 Jan 19, 2019

@ecstatic-morse ecstatic-morse changed the title [WIP] Add a new `clippy` mode to crater to test crater regressions [WIP] Add a new `clippy` mode to crater to test lint regressions Jan 19, 2019

@pietroalbini

pietroalbini requested changes Jan 21, 2019
View changes

Copy link
Member

pietroalbini left a comment

Everything except the mount flag change looks great! Thank you!

I'd prefer not to pass --security-opt=label:disable to Docker though (we're basically running untrusted code and every bit of extra security helps). An alternative is to store the toolchain name on the filesystem urlencoded with a custom encode set (like we do during report generation) that includes : in it.

ecstatic-morse added some commits Jan 19, 2019

@ecstatic-morse
Add `clippy` task 
This commit also adds a method, `Toolchain::install_rustup_component` to
install a rustup componenet on the given toolchain.
ec1877d
@ecstatic-morse
Add a `clippy` test to `minicrater` 
The test runs crater in `clippy` mode and compares `stable` with
`stable+rustflags=-Dclippy::all` so that a regression will appear when
clippy would emit a warning on a crate.

Also adds a new local crate which compiles succesfully but causes clippy
to emit a warning.
3c25c49

@ecstatic-morse ecstatic-morse force-pushed the ecstatic-morse:clippy-mode branch 2 times, most recently from e859fb3 to 1d89103 Jan 26, 2019

@ecstatic-morse ecstatic-morse force-pushed the ecstatic-morse:clippy-mode branch from 1d89103 to 3985d0c Jan 26, 2019

@ecstatic-morse

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

ecstatic-morse commented Jan 26, 2019 •
edited

I reverted the changes to docker.rs and started encoding paths using the url::percent_encode. Unicode code points (e.g. U+3D) are also an option.

There's some crossover here with Windows support, so I chose to escape all characters which are invalid as part of a Windows path, not just ':'.
@pietroalbini

pietroalbini approved these changes Jan 26, 2019
View changes

Copy link
Member

pietroalbini left a comment

This is great, thanks!

@pietroalbini pietroalbini changed the title [WIP] Add a new `clippy` mode to crater to test lint regressions Add a new `clippy` mode to crater to test lint regressions Jan 26, 2019

@pietroalbini

This comment has been minimized.

Sign in to view
Copy link
Member

pietroalbini commented Jan 26, 2019

@bors r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Collaborator

bors commented Jan 26, 2019

📌 Commit 3985d0c has been approved by pietroalbini
@bors

This comment has been minimized.

Sign in to view
Copy link
Collaborator

bors commented Jan 26, 2019

⌛️ Testing commit 3985d0c with merge a786b2a...

bors added a commit that referenced this pull request Jan 26, 2019

@bors
Auto merge of #391 - ecstatic-morse:clippy-mode, r=pietroalbini 
Add a new `clippy` mode to crater to test lint regressions

Resolves #388.

This adds a new mode, `clippy`, to crater which runs clippy on crates (duh :).

I had to add some new options to minicrater, as well as change the invocation of `docker create`. See the commit messages for more info.

This is still a work in progress as I'd like to add some documentation for the feature, but should be enough for a meaningful review.
Loading status checks…
a786b2a
@bors

This comment has been minimized.

Sign in to view
Copy link
Collaborator

bors commented Jan 26, 2019

💔 Test failed - checks-travis
@ecstatic-morse

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

ecstatic-morse commented Jan 26, 2019

💔
@bors

This comment has been minimized.

Sign in to view
Copy link
Collaborator

bors commented Jan 26, 2019

⌛️ Testing commit 3985d0c with merge b77d9f4...

bors added a commit that referenced this pull request Jan 26, 2019

@bors
Auto merge of #391 - ecstatic-morse:clippy-mode, r=pietroalbini 
Add a new `clippy` mode to crater to test lint regressions

Resolves #388.

This adds a new mode, `clippy`, to crater which runs clippy on crates (duh :).

I had to add some new options to minicrater, as well as change the invocation of `docker create`. See the commit messages for more info.

This is still a work in progress as I'd like to add some documentation for the feature, but should be enough for a meaningful review.
Loading status checks…
b77d9f4
@pietroalbini

This comment has been minimized.

Sign in to view
Copy link
Member

pietroalbini commented Jan 26, 2019

WTF bors.

@bors r- retry
@ecstatic-morse

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

ecstatic-morse commented Jan 26, 2019

Whoops, I forgot to add the results of the clippy minicrater test to the expected results for full. When they finish running locally I'll push again. Sorry!
@ecstatic-morse

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

ecstatic-morse commented Jan 26, 2019

Okay, should be good to go now.
@pietroalbini

This comment has been minimized.

Sign in to view
Copy link
Member

pietroalbini commented Jan 26, 2019

Thanks!

@bors r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Collaborator

bors commented Jan 26, 2019

📌 Commit 42174eb has been approved by pietroalbini
@bors

This comment has been minimized.

Sign in to view
Copy link
Collaborator

bors commented Jan 26, 2019

⌛️ Testing commit 42174eb with merge e39d131...

bors added a commit that referenced this pull request Jan 26, 2019

@bors
Auto merge of #391 - ecstatic-morse:clippy-mode, r=pietroalbini 
Add a new `clippy` mode to crater to test lint regressions

Resolves #388.

This adds a new mode, `clippy`, to crater which runs clippy on crates (duh :).

I had to add some new options to minicrater, as well as change the invocation of `docker create`. See the commit messages for more info.

This is still a work in progress as I'd like to add some documentation for the feature, but should be enough for a meaningful review.
Loading status checks…
e39d131
@bors

This comment has been minimized.

Sign in to view
Copy link
Collaborator

bors commented Jan 26, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: pietroalbini
Pushing e39d131 to master...

@bors bors merged commit 42174eb into rust-lang-nursery:master Jan 26, 2019
1 check passed

1 check passed

homu Test successful
Details

@ecstatic-morse ecstatic-morse deleted the ecstatic-morse:clippy-mode branch Jan 26, 2019

@ecstatic-morse ecstatic-morse referenced this pull request Jan 26, 2019

Merged

Add documentation for new clippy mode #395

bors added a commit that referenced this pull request Jan 27, 2019

@bors
Auto merge of #395 - ecstatic-morse:clippy-mode, r=pietroalbini 
Add documentation for new clippy mode

Instructions for populating `rustflags` are included elsewhere in the docs, so this single line should suffice.

This should have been included in #391 (whoops).
Loading status checks…
ac4726b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.