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

bootstrap: Implement conditional tracing infra #135391

Merged
merged 4 commits into from
Jan 13, 2025

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Jan 12, 2025

Add a conditional tracing setup that is gated behind BOOTSTRAP_TRACING env var. This tracing infra is implemented by:

  • Introducing an optional tracing cargo feature in bootstrap.
  • Added optional tracing* dependencies which are gated behind the tracing cargo feature.
  • When BOOTSTRAP_TRACING is set, bootstrap.py will build bootstrap with --features=tracing.

There is a small trick here to share BOOTSTRAP_TRACING env var without having to add a separate env var:

  • BOOTSTRAP_TRACING=1 is not a registered tracing filter target, so that can be used to enable the tracing cargo feature yet not actually enable any tracing logs (useful for editor r-a setups without actually outputting any tracing logs).
  • BOOTSTRAP_TRACING=TRACE and such are actually valid tracing filters, but that sets BOOTSTRAP_TRACING anyway.

Example usage: #135299 (that experimental PR is not conditionally gated)
This PR is intentionally kept minimal to focus on the infra itself. To get actual mileage, instrumentations will need to be added to individual Steps and such.

r? @onur-ozkan (or reroll)

@rustbot rustbot added A-rustc-dev-guide Area: rustc-dev-guide S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jan 12, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 12, 2025

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol

@jieyouxu jieyouxu changed the title bootstrap: implement conditional tracing infra bootstrap: Implement conditional tracing infra Jan 12, 2025
Comment on lines -274 to +278
"regex-automata",
"regex-syntax",
"regex-automata 0.4.9",
"regex-syntax 0.8.5",
Copy link
Member

Choose a reason for hiding this comment

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

Seems like we duplicated few dependencies. Is it possible to avoid this?

Copy link
Member Author

@jieyouxu jieyouxu Jan 12, 2025

Choose a reason for hiding this comment

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

Possibly, I may have used tracing* versions that are too new. I'll double check.

Copy link
Member Author

Choose a reason for hiding this comment

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

This one is duplicated because

├── tracing-subscriber v0.3.19
│   ├── matchers v0.1.0
│   │   └── regex-automata v0.1.10
│   │       └── regex-syntax v0.6.29

I don't think this can be dedup'd because it's a transitive dep of tracing-subscriber when env-filter feature is enabled (which is used to filter like BOOTSTRAP_TRACING=bootstrap::core::build_steps::test=TRACE).

Copy link
Contributor

@klensy klensy Jan 12, 2025

Choose a reason for hiding this comment

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

tracing stuck with updating matchers almost half year :-(
And with nu-ansi-term too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Want to cargo have ability to simply apply patches to dependencies, like [patch] but with diff files.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh hey... Chris opened a PR for nu-ansi-term at tokio-rs/tracing#3049 and the matchers bump is stuck on maintainer bandwidth probably at tokio-rs/tracing#3033

@jieyouxu
Copy link
Member Author

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 12, 2025
@jieyouxu jieyouxu force-pushed the conditional-tracing branch from 768f397 to d3a6460 Compare January 12, 2025 05:37
@jieyouxu
Copy link
Member Author

@rustbot ready

@rustbot rustbot 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 12, 2025
@jieyouxu jieyouxu added the A-bootstrap-tracing Area: bootstrap tracing label Jan 12, 2025
@jieyouxu jieyouxu force-pushed the conditional-tracing branch from d3a6460 to b9f9976 Compare January 13, 2025 00:16
Copy link
Member Author

Choose a reason for hiding this comment

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

Remark: this output is from next doors over at #135299.

Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

Feel free to r=me with the nit

@jieyouxu jieyouxu force-pushed the conditional-tracing branch from b9f9976 to 3ae724e Compare January 13, 2025 05:46
@onur-ozkan
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jan 13, 2025

📌 Commit 3ae724e has been approved by onur-ozkan

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

Successful merges:

 - rust-lang#135355 (ci: added test log format for ci)
 - rust-lang#135386 (clean up code related to the rustdoc-js test suite)
 - rust-lang#135391 (bootstrap: Implement conditional `tracing` infra)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cc00936 into rust-lang:master Jan 13, 2025
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 13, 2025
Rollup merge of rust-lang#135391 - jieyouxu:conditional-tracing, r=onur-ozkan

bootstrap: Implement conditional `tracing` infra

Add a conditional `tracing` setup that is gated behind `BOOTSTRAP_TRACING` env var. This `tracing` infra is implemented by:

- Introducing an optional `tracing` cargo feature in bootstrap.
- Added optional `tracing*` dependencies which are gated behind the `tracing` cargo feature.
- When `BOOTSTRAP_TRACING` is set, `bootstrap.py` will build bootstrap with `--features=tracing`.

There is a small trick here to share `BOOTSTRAP_TRACING` env var without having to add a separate env var:

- `BOOTSTRAP_TRACING=1` is not a registered `tracing` filter target, so that can be used to enable the `tracing` cargo feature yet not actually enable any tracing logs (useful for editor r-a setups without actually outputting any tracing logs).
- `BOOTSTRAP_TRACING=TRACE` and such are actually valid `tracing` filters, but that sets `BOOTSTRAP_TRACING` anyway.

Example usage: rust-lang#135299 (that experimental PR is not conditionally gated)
This PR is intentionally kept minimal to focus on the infra itself. To get actual mileage, instrumentations will need to be added to individual `Step`s and such.

r? `@onur-ozkan` (or reroll)
@rustbot rustbot added this to the 1.86.0 milestone Jan 13, 2025
@jieyouxu jieyouxu deleted the conditional-tracing branch January 13, 2025 16:35
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 13, 2025
…rrors

bootstrap: fix outdated feature name in comment

Follow-up to rust-lang#135391 (comment).

I guess I updated everything else **except** the comment right next to the actual dependencies 💀

r? bootstrap
jhpratt added a commit to jhpratt/rust that referenced this pull request Jan 14, 2025
…rrors

bootstrap: fix outdated feature name in comment

Follow-up to rust-lang#135391 (comment).

I guess I updated everything else **except** the comment right next to the actual dependencies 💀

r? bootstrap
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 14, 2025
Rollup merge of rust-lang#135452 - jieyouxu:fix-comment, r=compiler-errors

bootstrap: fix outdated feature name in comment

Follow-up to rust-lang#135391 (comment).

I guess I updated everything else **except** the comment right next to the actual dependencies 💀

r? bootstrap
"--json-output",
"--build-dir=build-rust-analyzer"
],
```

Choose a reason for hiding this comment

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

On the latest master branch, BOOSTRACING=1 has no effect.

Copy link

@QingyaoLin QingyaoLin Jan 18, 2025

Choose a reason for hiding this comment

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

This is my current ra setup:

{
    "git.detectSubmodulesLimit": 20,
    "rust-analyzer.check.invocationStrategy": "once",
    "rust-analyzer.check.overrideCommand": [
        "BOOTSTRAP_TRACING=1",
        "python",
        "x.py",
        "check",
        // "compiler",
        "src/bootstrap",
        "--build-dir=build-rust-analyzer",
        "--json-output"
    ],
    "rust-analyzer.linkedProjects": [
        "Cargo.toml",
        "library/Cargo.toml",
        // "src/tools/x/Cargo.toml",
        "src/bootstrap/Cargo.toml",
        // "src/tools/rust-analyzer/Cargo.toml",
        // "compiler/rustc_codegen_cranelift/Cargo.toml",
        // "compiler/rustc_codegen_gcc/Cargo.toml"
    ],
    "rust-analyzer.rustfmt.overrideCommand": [
        "${workspaceFolder}/build-rust-analyzer/host/rustfmt/bin/rustfmt",
        "--edition=2021"
    ],
    "rust-analyzer.procMacro.server": "${workspaceFolder}/build-rust-analyzer/host/stage0/libexec/rust-analyzer-proc-macro-srv",
    "rust-analyzer.procMacro.enable": true,
    "rust-analyzer.cargo.buildScripts.enable": true,
    "rust-analyzer.cargo.buildScripts.invocationStrategy": "once",
    "rust-analyzer.cargo.buildScripts.overrideCommand": [
        "BOOTSTRAP_TRACING=1",
        "python",
        "x.py",
        "check",
        // "compiler",
        "src/bootstrap",
        "--build-dir=build-rust-analyzer",
        "--json-output"
    ],
    "rust-analyzer.cargo.sysrootSrc": "./library",
    "rust-analyzer.rustc.source": "./Cargo.toml",
    "rust-analyzer.cargo.extraEnv": {
        "RUSTC_BOOTSTRAP": "1",
    },
    "rust-analyzer.server.extraEnv": {
        "RUSTUP_TOOLCHAIN": "nightly"
    },
    "python.analysis.exclude": [
        "**"
    ]
}

ra result:
1737196429883

Choose a reason for hiding this comment

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

Copy link

@QingyaoLin QingyaoLin Jan 18, 2025

Choose a reason for hiding this comment

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

Additionally, I observed that the ra output:

2025-01-18T19:47:55.3706583+08:00 ERROR FetchBuildDataError: Failed to run build scripts for C:\Users\xxxx\source\rust-compiler: program not found
Failed to run build scripts for C:\Users\xxxx\source\rust-compiler\library: program not found
Failed to run build scripts for C:\Users\xxxx\source\rust-compiler\src\bootstrap: program not found

Copy link
Member Author

@jieyouxu jieyouxu Jan 18, 2025

Choose a reason for hiding this comment

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

On the latest master branch, BOOSTRACING=1 has no effect.

see PR description

This PR is intentionally kept minimal to focus on the infra itself. To get actual mileage, instrumentations will need to be added to individual Steps and such.

jhpratt added a commit to jhpratt/rust that referenced this pull request Jan 27, 2025
Add some tracing to core bootstrap logic

Follow-up to rust-lang#135391.

### Summary

Add some initial tracing logging to bootstrap, focused on the core logic (in this PR).

Also:

- Adjusted tracing-tree style to not use indent lines (I found that more distracting than helpful).
- Avoid glob-importing `tracing` items.
- Improve the rustc-dev-guide docs on bootstrap tracing.

### Example output

```bash
$ BOOTSTRAP_TRACING=bootstrap=TRACE ./x check src/bootstrap
```

![Example bootstrap tracing output](https://github.com/user-attachments/assets/0be39042-0822-44b6-9451-30427cfea156)

r? bootstrap
fmease added a commit to fmease/rust that referenced this pull request Jan 27, 2025
Add some tracing to core bootstrap logic

Follow-up to rust-lang#135391.

### Summary

Add some initial tracing logging to bootstrap, focused on the core logic (in this PR).

Also:

- Adjusted tracing-tree style to not use indent lines (I found that more distracting than helpful).
- Avoid glob-importing `tracing` items.
- Improve the rustc-dev-guide docs on bootstrap tracing.

### Example output

```bash
$ BOOTSTRAP_TRACING=bootstrap=TRACE ./x check src/bootstrap
```

![Example bootstrap tracing output](https://github.com/user-attachments/assets/0be39042-0822-44b6-9451-30427cfea156)

r? bootstrap
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 27, 2025
Rollup merge of rust-lang#136091 - jieyouxu:core-tracing, r=clubby789

Add some tracing to core bootstrap logic

Follow-up to rust-lang#135391.

### Summary

Add some initial tracing logging to bootstrap, focused on the core logic (in this PR).

Also:

- Adjusted tracing-tree style to not use indent lines (I found that more distracting than helpful).
- Avoid glob-importing `tracing` items.
- Improve the rustc-dev-guide docs on bootstrap tracing.

### Example output

```bash
$ BOOTSTRAP_TRACING=bootstrap=TRACE ./x check src/bootstrap
```

![Example bootstrap tracing output](https://github.com/user-attachments/assets/0be39042-0822-44b6-9451-30427cfea156)

r? bootstrap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-bootstrap-tracing Area: bootstrap tracing A-rustc-dev-guide Area: rustc-dev-guide 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.

8 participants