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.

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

Revert "bootstrap: Get rid of tail_args in stream_cargo" #106387

Merged
merged 1 commit into from
Jan 10, 2023

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jan 2, 2023

This reverts commit 9dfe504. (Note: that merged as part of #106305, but #106305 contains more commits than just 9dfe504.)

Fixes x clippy. It turns out clippy was the only one using tail_args 🤦 sorry for not testing this earlier.

r? @Mark-Simulacrum

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 2, 2023
@jyn514
Copy link
Member Author

jyn514 commented Jan 2, 2023

The error this fixes:

> x clippy
Checking stage0 library artifacts (aarch64-unknown-linux-gnu -> aarch64-unknown-linux-gnu)
error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `/home/gh-jyn514/.local/lib/rustup/toolchains/nightly-aarch64-unknown-linux-gnu/bin/clippy-driver rustc - --crate-name ___ --print=file-names --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=cfg` (exit status: 1)
  --- stderr
  error: Unrecognized option: 'message-format'

@Mark-Simulacrum
Copy link
Member

r=me if we want to fix this, but it's not clear to me why the change didn't work.

@jyn514
Copy link
Member Author

jyn514 commented Jan 3, 2023

@Mark-Simulacrum check::args is passing -- -Aclippy::xyz for a bunch of lints: https://github.com/rust-lang/rust/blob/6f00f08cfb402f41ca7bc78fb1c0e63100995464/src/bootstrap/check.rs#L53
-- is the separator that tells cargo-clippy to pass the arguments through to clippy-driver instead treating them as cargo arguments.

We might be able to pass those using environment variables instead? But I think it will break caching for std, since RUSTFLAGS applies to dependencies and not just crates in the workspace.

@jyn514
Copy link
Member Author

jyn514 commented Jan 3, 2023

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Jan 3, 2023

📌 Commit 6f00f08cfb402f41ca7bc78fb1c0e63100995464 has been approved by Mark-Simulacrum

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 Jan 3, 2023
@Mark-Simulacrum
Copy link
Member

Ah, I see. I guess it could get passed via RUSTFLAGS (CLIPPYFLAGS?) in the same way we pass other lint arguments, but this seems fine for now.

The caching does seem hard. Maybe we can always pass clippy flags; doesn't rustc just ignore them?

@jyn514
Copy link
Member Author

jyn514 commented Jan 3, 2023

No, unfortunately, cargo gives a hard error unless you use cargo rustc instead of cargo check.

; cargo check -- --cap-lints warn
error: Found argument '--cap-lints' which wasn't expected, or isn't valid in this context

Usage: cargo check [OPTIONS]

For more information try '--help'

@jyn514
Copy link
Member Author

jyn514 commented Jan 3, 2023

Oh, you meant with RUSTFLAGS. Yes, that would work:

; RUSTFLAGS='--cap-lints=warn -Wclippy::too_many_arguments' cargo check
    Checking example v0.1.0 (/home/gh-jyn514/example)
    Finished dev [unoptimized + debuginfo] target(s) in 0.06s

@bors
Copy link
Contributor

bors commented Jan 3, 2023

☔ The latest upstream changes (presumably #105609) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 3, 2023
@jyn514
Copy link
Member Author

jyn514 commented Jan 3, 2023

Rebased - I haven't reapproved on your behalf in case you want me to use RUSTFLAGS instead of tail_args.

@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 3, 2023
@rust-log-analyzer

This comment has been minimized.

@Mark-Simulacrum
Copy link
Member

I'm not worried about doing that as part of this PR.

@bors r+

@bors
Copy link
Contributor

bors commented Jan 7, 2023

📌 Commit 23d3ee8 has been approved by Mark-Simulacrum

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 Jan 7, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 10, 2023
Rollup of 9 pull requests

Successful merges:

 - rust-lang#105034 (Add example for iterator_flatten)
 - rust-lang#105708 (Enable atomic cas for bpf targets)
 - rust-lang#106175 (Fix bad import suggestion with nested `use` tree)
 - rust-lang#106204 (No need to take opaques in `check_type_bounds`)
 - rust-lang#106387 (Revert "bootstrap: Get rid of `tail_args` in `stream_cargo`")
 - rust-lang#106636 (Accept old spelling of Fuchsia target triples)
 - rust-lang#106639 (update Miri)
 - rust-lang#106640 (update test for inductive canonical cycles)
 - rust-lang#106647 (rustdoc: merge common CSS for `a`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b102673 into rust-lang:master Jan 10, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 10, 2023
@jyn514 jyn514 deleted the clippy branch February 25, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants