Skip to content
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

Jump to bottom

chore: dogfood Cargo -Zlints table feature #12178

Merged
merged 6 commits into from
Nov 16, 2023
Merged

chore: dogfood Cargo -Zlints table feature #12178

merged 6 commits into from
Nov 16, 2023

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

-Zlints unstable feature just landed on nightly-2023-05-25 (rustc 1.71.0-nightly). It would be awesome if Cargo starts dogfooding itself.

How should we test and review this PR?

A shell script is added to check lint rules on nightly channel. This is expected to fail at this time. We probably want to fix lint errors in this PR or in follow-ups, depending on the review.

This does nothing on stable channel, which Cargo sticks to. Merely that cargo build becomes noisy because -Zlints triggers lots of unused manifest key: lints warnings, and has no way to turn them off. If people feel unease about this, we can wait.

Additional information

@rustbot
Copy link
Collaborator

rustbot commented May 25, 2023

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-cfg-expr Area: Platform cfg expressions A-cli Area: Command-line interface, option parsing, etc. A-cli-help Area: built-in command-line help A-dependency-resolution Area: dependency resolution and the resolver A-environment-variables Area: environment variables A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. A-interacts-with-crates.io Area: interaction with registries A-registry-authentication Area: registry authentication and authorization (authn authz) A-semver Area: semver specifications, version matching, etc. A-testing-cargo-itself Area: cargo's tests S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 25, 2023
@weihanglo weihanglo added the A-lints-table Area: [lints] table label May 25, 2023
epage
epage reviewed May 25, 2023
View reviewed changes
Cargo.toml

[workspace.lints.clippy]
all = { level = "allow", priority = -1 }
disallowed_methods = "warn"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming we'll either

  • Need to find a way to make the clippy config only apply to the cargo package
  • Only use this on the cargo lib (via an attribute)

Copy link
Member Author

Choose a reason for hiding this comment

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

The second one seems easier, though I assume most cargo-* crates should follow the same rule of disallowed_methods (if we split more subcrates…).

epage
epage reviewed May 25, 2023
View reviewed changes
tests/testsuite/main.rs Outdated Show resolved Hide resolved
This was referenced May 25, 2023
Closed
Merged
@weihanglo weihanglo marked this pull request as draft May 25, 2023 13:56
@ehuss
Copy link
Contributor

ehuss commented Jun 5, 2023

Just checking in to make sure this isn't waiting on me. This was marked as a Draft, but I'm not clear what's blocking or needs to be done.

@weihanglo
Copy link
Member Author

I am not sure if we want to move on.

  • Remove lint attributes? At the cost of building with stable toolchain all clippy waring popping up.
  • Fix all lint errors for sub crates maybe? The change should be smaller without touching cargo crate.

@weihanglo weihanglo removed A-interacts-with-crates.io Area: interaction with registries A-dependency-resolution Area: dependency resolution and the resolver A-environment-variables Area: environment variables A-testing-cargo-itself Area: cargo's tests A-cli Area: Command-line interface, option parsing, etc. A-cfg-expr Area: Platform cfg expressions A-semver Area: semver specifications, version matching, etc. A-cli-help Area: built-in command-line help A-registry-authentication Area: registry authentication and authorization (authn authz) labels Aug 26, 2023
@weihanglo
lint: allow self_named_module_files
48ccb1d 
There is a false positive in `src/bin/cargo/commands/build.rs`
@weihanglo weihanglo force-pushed the dogfood-lints-table branch 2 times, most recently from 9ea575f to 91a812a Compare November 16, 2023 16:37
@weihanglo weihanglo marked this pull request as ready for review November 16, 2023 16:54
@weihanglo
Copy link
Member Author

This is ready for review 🚀

@weihanglo
chore: enable clippy check for all workspace members
68af509 
This also remove `RUSTFLAGS: -D warnings` for test,
as it seems to be redundant and clippy covers everythings
@weihanglo
chore: bump version for member crates
e2f5925
@epage
Copy link
Contributor

epage commented Nov 16, 2023

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Nov 16, 2023

📌 Commit e2f5925 has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 16, 2023
@bors
Copy link
Collaborator

bors commented Nov 16, 2023

⌛ Testing commit e2f5925 with merge e46d97c...

bors added a commit that referenced this pull request Nov 16, 2023
@bors
Auto merge of #12178 - weihanglo:dogfood-lints-table, r=epage
e46d97c 
chore: dogfood Cargo `-Zlints` table feature
@bors
Copy link
Collaborator

bors commented Nov 16, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 16, 2023
@Muscraft
Copy link
Member

The failure was caused by a crates.io deployment. It's being rolled back, see this Zulip comment.

@ehuss
Copy link
Contributor

ehuss commented Nov 16, 2023

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 16, 2023
@Turbo87
Copy link
Member

Turbo87 commented Nov 16, 2023

The failure was caused by a crates.io deployment.

clearly this was all a plot to get my two PRs processed earlier in the queue... 😂

@bors
Copy link
Collaborator

bors commented Nov 16, 2023

⌛ Testing commit e2f5925 with merge 9f3b488...

@bors
Copy link
Collaborator

bors commented Nov 16, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 9f3b488 to master...

@bors bors merged commit 9f3b488 into rust-lang:master Nov 16, 2023
22 checks passed
@weihanglo weihanglo mentioned this pull request Nov 18, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 18, 2023
@bors
Auto merge of rust-lang#118037 - weihanglo:update-cargo, r=weihanglo
95a5c59 
Update cargo

11 commits in 2c03e0e2dcd05dd064fcf10cc1050d342eaf67e3..9765a449d9b7341c2b49b88da41c2268ea599720
2023-11-16 04:21:44 +0000 to 2023-11-17 20:58:23 +0000
- refactor(toml): Clean up workspace inheritance (rust-lang/cargo#12971)
- docs: Recommend a wider selection of libsecret-compatible password managers (rust-lang/cargo#12993)
- feat(cli): add color output for `cargo --list` (rust-lang/cargo#12992)
- refactor: log when loading config from file (rust-lang/cargo#12991)
- Link to rustc lint levels (rust-lang/cargo#12990)
- chore(ci): Catch naive use of AtomicU64 early (rust-lang/cargo#12988)
- cargo-credential-1password: Add missing `--account` argument to `op signin` command (rust-lang/cargo#12985)
- chore: dogfood Cargo `-Zlints` table feature (rust-lang/cargo#12178)
- cargo-credential-1password: Fix README (rust-lang/cargo#12986)
- Fix a rustflags test using a wrong buildfile name (rust-lang/cargo#12987)
- Fix some test output validation. (rust-lang/cargo#12982)

r? ghost
@weihanglo weihanglo deleted the dogfood-lints-table branch November 19, 2023 12:16
@ehuss ehuss added this to the 1.76.0 milestone Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@epage epage epage left review comments

Assignees

@ehuss ehuss

Labels
A-building-cargo-itself Area: issues with building cargo A-cfg-expr Area: Platform cfg expressions A-cli Area: Command-line interface, option parsing, etc. A-cli-help Area: built-in command-line help A-credential-provider Area: credential provider for storing and retreiving credentials A-dependency-resolution Area: dependency resolution and the resolver A-environment-variables Area: environment variables A-home Area: the `home` crate A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. A-interacts-with-crates.io Area: interaction with registries A-lints-table Area: [lints] table A-registry-authentication Area: registry authentication and authorization (authn authz) A-semver Area: semver specifications, version matching, etc. A-testing-cargo-itself Area: cargo's tests S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.76.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@weihanglo @rustbot @ehuss @bors @epage @Muscraft @Turbo87