Skip to content

fix(timing)!: remove --timings=<FMT> optional format values #16420

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
Open
weihanglo wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(timing)!: remove --timings=<FMT> optional format values #16420

weihanglo wants to merge 1 commit into from

Conversation

@weihanglo
Copy link
Member

@weihanglo weihanglo commented Dec 19, 2025 •
edited
Loading

What does this PR try to resolve?

Arguments to remove it

  • The --timings=json is obsolete as -Zbuild-analysis logging is
    a more approachable option, which doesn't need passing
    --timings=json ahead of time.
  • There is no support infra built around --timings=json yet,
    while for -Zbuild-analysis we have cargo report timings already.
  • --timings=json is a UI feature inherently unstable, and has no tests.

Counterargument:

  • --timings=json outputs to stdout, but there is no alternative yet
    also outputs to stdout.
    feat(log): add output control #16418 was an attempt to add that back,
    but we then decide to punt until seeing requests or needs.

How to test and review this PR?

  • cargo help build and check the manpage
  • cargo build --help and cehck the help text
  • cargo build --timings and it works

@rustbot
Copy link
Collaborator

rustbot commented Dec 19, 2025

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-build-execution Area: anything dealing with executing the compiler A-cli Area: Command-line interface, option parsing, etc. A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation A-json-output Area: JSON message output A-timings Area: timings S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 19, 2025
weihanglo
weihanglo commented Dec 19, 2025
View reviewed changes
src/cargo/core/compiler/build_runner/mod.rs
Copy link
Member Author

@weihanglo weihanglo Dec 19, 2025

Choose a reason for hiding this comment

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

Does it require an FCP?

Copy link
Contributor

@epage epage Dec 19, 2025

Choose a reason for hiding this comment

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

Might be good to get a vibe check in case there is interest in build analysis having X amount of functionality before it can replace --timings=json

@weihanglo weihanglo added the T-cargo Team: Cargo label Dec 19, 2025
@weihanglo
fix(timing)!: remove --timings=<FMT> optional format values
f851173 
Arguments to remove it

* The `--timings=json` is obsolete as `-Zbuild-analysis` logging is
  a more approachable option, which doesn't need passing
  `--timings=json` ahead of time.
* There is no support infra built around `--timings=json` yet,
  while for `-Zbuild-analysis` we have `cargo report timings` already.
* `--timings=json` is a UI feature inherently unstable, and has no tests.

Counterargument:

* `--timings=json` outputs to stdout, but there is no  alternative yet
  also outputs to stdout.
@weihanglo

This comment was marked as outdated.

Sign in to view
@rust-rfcbot

This comment was marked as outdated.

Sign in to view
@weihanglo
Copy link
Member Author

weihanglo commented Dec 19, 2025

@rfcbot fcp merge T-cargo

This removes support for --timings=json, making --timings a plain boolean flag after JSON timing output was removed. See the PR descriptions for more.

@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Dec 19, 2025 •
edited by epage
Loading

Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Dec 19, 2025
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

At least 1 approving review is required to merge this pull request.

Assignees

@ehuss ehuss

Labels

A-build-execution Area: anything dealing with executing the compiler A-cli Area: Command-line interface, option parsing, etc. A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation A-json-output Area: JSON message output A-timings Area: timings disposition-merge FCP with intent to merge proposed-final-comment-period An FCP proposal has started, but not yet signed off. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cargo Team: Cargo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@weihanglo @rustbot @rust-rfcbot @epage @ehuss