Skip to content

Conversation

Kobzol
Copy link
Member

@Kobzol Kobzol commented Mar 15, 2025

Some CI jobs (x64 Linux, ARM64 Linux and x64 MSVC) use the opt-dist tool to build an optimized toolchain using PGO and BOLT. When performing a default try build for x64 Linux, in most cases we want to run perf. on that artifact. To reduce the latency of this common use-case, opt-dist skips building several components not needed for perf., and it also skips running post-optimization tests, when it detects that the job is executed as a try job (not a merge/auto job).

This is useful, but it also means that if you want to run the tests, you had to go to jobs.yml and manually comment this environment variable, create a WIP commit, do a try build, and then remove the WIP commit, which is annoying (in the similar way that modifying what gets run in try builds was annoying before we had the try-job annotations).

I thought that we could introduce some additional PR description marker like try-job-run-tests, but it's hard to discover that such things exist.

Instead, I think that there's a much simpler heuristic for determining whether DIST_TRY_BUILD should be used (that I implemented in this PR):

  • If you do just @bors try, without any custom try jobs selected, DIST_TRY_BUILD will be activated, to finish the build as fast as possible.
  • If you specify any custom try jobs, you are most likely doing experiments and you want to see if tests pass and everything builds as it should. The DIST_TRY_BUILD variable will thus not be set in this case.

In this way, if you want to run dist tests, you can just add the try-job: dist-x86_64-linux line to the PR description, and you don't need to create any WIP commits.

r? @marcoieni

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Mar 15, 2025
@jieyouxu
Copy link
Member

Could you also leave a backlink somewhere, from maybe citool README or rustc-dev-guide's try job docs about this distinction?

@rustbot
Copy link
Collaborator

rustbot commented Mar 16, 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

@rustbot rustbot added the A-rustc-dev-guide Area: rustc-dev-guide label Mar 16, 2025
@Kobzol
Copy link
Member Author

Kobzol commented Mar 16, 2025

Good idea, done.

@marcoieni
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 17, 2025

📌 Commit b2fda93 has been approved by marcoieni

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 Mar 17, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 18, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138384 (Move `hir::Item::ident` into `hir::ItemKind`.)
 - rust-lang#138508 (Clarify "owned data" in E0515.md)
 - rust-lang#138531 (Store test diffs in job summaries and improve analysis formatting)
 - rust-lang#138533 (Only use `DIST_TRY_BUILD` for try jobs that were not selected explicitly)
 - rust-lang#138556 (Fix ICE: attempted to remap an already remapped filename)
 - rust-lang#138608 (rustc_target: Add target feature constraints for LoongArch)
 - rust-lang#138619 (Flatten `if`s in `rustc_codegen_ssa`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 18, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138384 (Move `hir::Item::ident` into `hir::ItemKind`.)
 - rust-lang#138508 (Clarify "owned data" in E0515.md)
 - rust-lang#138531 (Store test diffs in job summaries and improve analysis formatting)
 - rust-lang#138533 (Only use `DIST_TRY_BUILD` for try jobs that were not selected explicitly)
 - rust-lang#138556 (Fix ICE: attempted to remap an already remapped filename)
 - rust-lang#138608 (rustc_target: Add target feature constraints for LoongArch)
 - rust-lang#138619 (Flatten `if`s in `rustc_codegen_ssa`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c19ce9d into rust-lang:master Mar 18, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 18, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 18, 2025
Rollup merge of rust-lang#138533 - Kobzol:try-job-auto-tests, r=marcoieni

Only use `DIST_TRY_BUILD` for try jobs that were not selected explicitly

Some CI jobs (x64 Linux, ARM64 Linux and x64 MSVC) use the `opt-dist` tool to build an optimized toolchain using PGO and BOLT. When performing a default try build for x64 Linux, in most cases we want to run perf. on that artifact. To reduce the latency of this common use-case, `opt-dist` skips building several components not needed for perf., and it also skips running post-optimization tests, when it detects that the job is executed as a try job (not a merge/auto job).

This is useful, but it also means that if you *want* to run the tests, you had to go to `jobs.yml` and manually comment this environment variable, create a WIP commit, do a try build, and then remove the WIP commit, which is annoying (in the similar way that modifying what gets run in try builds was annoying before we had the `try-job` annotations).

I thought that we could introduce some additional PR description marker like `try-job-run-tests`, but it's hard to discover that such things exist.

Instead, I think that there's a much simpler heuristic for determining whether `DIST_TRY_BUILD` should be used (that I implemented in this PR):
- If you do just ``@bors` try`, without any custom try jobs selected, `DIST_TRY_BUILD` will be activated, to finish the build as fast as possible.
- If you specify any custom try jobs, you are most likely doing experiments and you want to see if tests pass and everything builds as it should. The `DIST_TRY_BUILD` variable will thus *not* be set in this case.

In this way, if you want to run dist tests, you can just add the `try-job: dist-x86_64-linux` line to the PR description, and you don't need to create any WIP commits.

r? `@marcoieni`
@Kobzol Kobzol deleted the try-job-auto-tests branch March 18, 2025 09:12
flip1995 pushed a commit to flip1995/rust that referenced this pull request Mar 20, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138384 (Move `hir::Item::ident` into `hir::ItemKind`.)
 - rust-lang#138508 (Clarify "owned data" in E0515.md)
 - rust-lang#138531 (Store test diffs in job summaries and improve analysis formatting)
 - rust-lang#138533 (Only use `DIST_TRY_BUILD` for try jobs that were not selected explicitly)
 - rust-lang#138556 (Fix ICE: attempted to remap an already remapped filename)
 - rust-lang#138608 (rustc_target: Add target feature constraints for LoongArch)
 - rust-lang#138619 (Flatten `if`s in `rustc_codegen_ssa`)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants