Skip to content

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 4, 2021

On reflection on the issue in #79540 (comment), I think the bug was actually using the compiler/ filter, not using --author=bors. 9a1d617 has no CI artifacts because it was merged as part of a rollup:

$ curl -I https://ci-artifacts.rust-lang.org/rustc-builds/96e843ce6ae42e0aa519ba45e148269de347fd84/rust-std-nightly-x86_64-unknown-linux-gnu.tar.xz
HTTP/2 404

So 9a1d617 is the correct commit to download, and that's what --author=bors does:

$ git log --author=bors 4aec8a5
commit 9a1d617

Ideally it would look for "the most recent bors commit not followed by a change to compiler/", which would exclude things like documentation changes and avoid redownloading more than necessary, but

  • Redownloading isn't the end of the world,
  • That metric is hard to implement, and
  • Documentation-only or library-only changes are very rare anyway since they're usually rolled up with changes to the compiler.

Helps with #81930.

r? @Mark-Simulacrum

Unverified

This user has not yet uploaded their public signing key.
On reflection on the issue in rust-lang#79540 (comment),  I think the bug was actually using the `compiler/` filter, not using `--author=bors`. rust-lang@9a1d617 has no CI artifacts because it was merged as part of a rollup:
```
$ curl -I https://ci-artifacts.rust-lang.org/rustc-builds/96e843ce6ae42e0aa519ba45e148269de347fd84/rust-std-nightly-x86_64-unknown-linux-gnu.tar.xz
HTTP/2 404
```
So 9a1d617 is the correct commit to download, and that's what `--author=bors` does:

$ git log --author=bors 4aec8a5
commit 9a1d617

Ideally it would look for "the most recent bors commit not followed by a change to `compiler/`", which would exclude things like documentation changes and avoid redownloading more than necessary, but
- Redownloading isn't the end of the world,
- That metric is hard to implement, and
- Documentation-only or library-only changes are very rare anyway since they're usually rolled up with changes to the compiler.
@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-contributor-roadblock Area: Makes things more difficult for new or seasoned contributors to Rust labels Mar 4, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 4, 2021
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 4, 2021

📌 Commit a705a58 has been approved by Mark-Simulacrum

@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 4, 2021
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Removing intermediate container 785ef9e9d3bc
 ---> 0fb7965f0b50
Step 5/10 : RUN npm install es-check -g
 ---> Running in 233e6a72fcdf
/node-v14.4.0-linux-x64/bin/es-check -> /node-v14.4.0-linux-x64/lib/node_modules/es-check/index.js

> [email protected] postinstall /node-v14.4.0-linux-x64/lib/node_modules/es-check/node_modules/spawn-sync
> node postinstall
+ [email protected]
added 95 packages from 44 contributors in 3.627s
Removing intermediate container 233e6a72fcdf
 ---> 8d0968f7bb21
---
Successfully built a6bec2649292
Successfully tagged rust-ci:latest
Built container sha256:a6bec2649292265266d7066b61ad8e9afaa2facb34a79c5d9b18c6e4cd420e1f
Uploading finished image to https://ci-caches.rust-lang.org/docker/9f2a38e35a8211f9c2c342213b6dabbf1ce1e957c3f9f4a6874af054aa93d446d1c6f8252277cb4118d76ddf7862eec7c972b385df9acf97d8b518b20c0181e6
upload failed: - to s3://rust-lang-ci-sccache2/docker/9f2a38e35a8211f9c2c342213b6dabbf1ce1e957c3f9f4a6874af054aa93d446d1c6f8252277cb4118d76ddf7862eec7c972b385df9acf97d8b518b20c0181e6 Unable to locate credentials
[CI_JOB_NAME=mingw-check]
---
configure: rust.channel         := nightly
configure: rust.debug-assertions := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
configure: 
---
Cloning into 'rust-toolstate'...
<Nothing changed>
+ es-check es5 ../src/librustdoc/html/static/main.js ../src/librustdoc/html/static/settings.js ../src/librustdoc/html/static/source-script.js ../src/librustdoc/html/static/storage.js

Cannot read property 'includes' of undefined

@jyn514
Copy link
Member Author

jyn514 commented Mar 4, 2021

Seems spurious? @GuillaumeGomez do you recognize this error?

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 4, 2021
…acrum

Fix commit detected when using `download-rustc`

On reflection on the issue in rust-lang#79540 (comment), I think the bug was actually using the `compiler/` filter, not using `--author=bors`. rust-lang@9a1d617 has no CI artifacts because it was merged as part of a rollup:
```
$ curl -I https://ci-artifacts.rust-lang.org/rustc-builds/96e843ce6ae42e0aa519ba45e148269de347fd84/rust-std-nightly-x86_64-unknown-linux-gnu.tar.xz
HTTP/2 404
```
So 9a1d617 is the correct commit to download, and that's what `--author=bors` does:

$ git log --author=bors 4aec8a5
commit 9a1d617

Ideally it would look for "the most recent bors commit not followed by a change to `compiler/`", which would exclude things like documentation changes and avoid redownloading more than necessary, but
- Redownloading isn't the end of the world,
- That metric is hard to implement, and
- Documentation-only or library-only changes are very rare anyway since they're usually rolled up with changes to the compiler.

Helps with rust-lang#81930.

r? `@Mark-Simulacrum`
@GuillaumeGomez
Copy link
Member

That's definitely spurious.

@bors: retry

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 4, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#80527 (Make rustdoc lints a tool lint instead of built-in)
 - rust-lang#82310 (Load rustdoc's JS search index on-demand.)
 - rust-lang#82315 (Improve page load performance in rustdoc)
 - rust-lang#82564 (Revert `Vec::spare_capacity_mut` impl to prevent pointers invalidation)
 - rust-lang#82697 (Fix stabilization version of move_ref_pattern)
 - rust-lang#82717 (Account for macros when suggesting adding lifetime)
 - rust-lang#82740 (Fix commit detected when using `download-rustc`)
 - rust-lang#82744 (Pass `CrateNum` by value instead of by reference)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 17121f2 into rust-lang:master Mar 4, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 4, 2021
@jyn514 jyn514 deleted the proper-history branch March 4, 2021 15:17
@jyn514 jyn514 mentioned this pull request Mar 21, 2021
13 tasks
@jyn514 jyn514 added the A-download-rustc Area: The `rust.download-rustc` build option. label Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new or seasoned contributors to Rust A-download-rustc Area: The `rust.download-rustc` build option. 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.

None yet

7 participants