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

fix(script): Be quiet on programmatic output #12305

Merged
merged 4 commits into from
Jun 24, 2023
Merged

Conversation

epage
Copy link
Contributor

@epage epage commented Jun 23, 2023

What does this PR try to resolve?

This is a part of #12207

The goal is that we shouldn't interfere with end-user output when
"cargo script"s are used programmatically. The only way to detect this
is when piping. CI will also look like this.

My thought is that if someone does want to do #!/usr/bin/env -S cargo -v, it
should have a consistent meaning between local development
(cargo run --manifest-path) and "script mode" (cargo), so I
effectively added a new verbosity level in these cases. To get normal
output in all cases, add a -v like the tests do. Do -vv if you want
the normal -v mode. If you want it always quiet, do --quiet.

I want to see the default verbosity for interactive "script mode" a bit
quieter to the point that all normal output cargo makes is cleared before
running the built binary. I am holding off on that now as that could
tie into bigger conversations / refactors
(see https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Re-thinking.20cargo's.20output).

How should we test and review this PR?

Initial writing of tests and refactors are split split out. The tests in the final commit will let you see what impact this had on behavior.

epage added 4 commits June 22, 2023 19:10
This will let the command have some sway in how we configure
The goal is that we shouldn't interefere with end-user output when
"cargo script"s are used programmatically.  The only way to detect this
is when piping.  CI will also look like this.

My thought is that if someone does want to do `#!/usr/bin/env -S cargo -v`, it
should have a consistent meaning between local development
(`cargo run --manifest-path`) and "script mode" (`cargo`), so I
effectively added a new verbosity level in these cases.  To get normal
output in all cases, add a `-v` like the tests do.  Do `-vv` if you want
the normal `-v` mode.  If you want it always quiet, do `--quiet`.

I want to see the default verbosity for interactive "script mode" a bit
quieter to the point that all normal output cargo makes is cleared before
running the built binary.  I am holding off on that now as that could
tie into bigger conversations / refactors
(see https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Re-thinking.20cargo's.20output).
@rustbot
Copy link
Collaborator

rustbot commented Jun 23, 2023

r? @ehuss

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

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 23, 2023
@rustbot rustbot added the A-documenting-cargo-itself Area: Cargo's documentation label Jun 23, 2023
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Clever refactor!

Looks pretty safe to merge since it changes only "cargo script".

src/bin/cargo/cli.rs Outdated Show resolved Hide resolved
@weihanglo
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Jun 23, 2023

📌 Commit 0e648c3 has been approved by weihanglo

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 Jun 23, 2023
@bors
Copy link
Contributor

bors commented Jun 23, 2023

⌛ Testing commit 0e648c3 with merge 03bc66b...

@bors
Copy link
Contributor

bors commented Jun 24, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 03bc66b to master...

@bors bors merged commit 03bc66b into rust-lang:master Jun 24, 2023
@epage epage deleted the quiet branch June 24, 2023 00:15
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jun 24, 2023
Update cargo

8 commits in 4cebd130ebca3bc219180a54f3e26cc1b14a91de..03bc66b55c290324bd46eb22e369c8fae1908f91
2023-06-21 18:59:29 +0000 to 2023-06-23 23:27:46 +0000
- fix(script): Be quiet on programmatic output (rust-lang/cargo#12305)
- docs(unstable): Update script documentation (rust-lang/cargo#12308)
- cargo script example needs nightly -Zscript feature (rust-lang/cargo#12287)
- fix(script): Process config relative to script, not CWD (rust-lang/cargo#12303)
- -Znext-lockfile-bump: Don't suggest using -Z on stable (rust-lang/cargo#12302)
- build(deps): bump openssl from 0.10.54 to 0.10.55 (rust-lang/cargo#12300)
- Add `.toml` file extension restriction for `-Zconfig-include` (rust-lang/cargo#12298)
- docs(unstable): Point stable-unstable docs to nightly docs (rust-lang/cargo#12299)

r? `@ghost`
@ehuss ehuss added this to the 1.72.0 milestone Jul 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants