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

CI: fix toolstate publishing #125145

Merged
merged 1 commit into from
May 19, 2024
Merged

CI: fix toolstate publishing #125145

merged 1 commit into from
May 19, 2024

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented May 15, 2024

Toolstate publishing after something broke was not working (discovered here). The toolstate env. vars should only be needed for the publishing step, so I moved them there.

The toolstate script is also being checked in mingw-check on PR and auto CI, but it doesn't really seem to do anything, and it shouldn't require the token.

@rustbot
Copy link
Collaborator

rustbot commented May 15, 2024

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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-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 May 15, 2024
@ChrisDenton
Copy link
Member

It's pretty impressive that toolstate has presumably been passing for years without a single failure

@Kobzol
Copy link
Contributor Author

Kobzol commented May 15, 2024

This was broken by my large CI restructuring that was finished ~2 weeks ago.

@ChrisDenton
Copy link
Member

Ah! That makes more sense.

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 19, 2024

📌 Commit 7b6a3d0 has been approved by Mark-Simulacrum

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 May 19, 2024
@bors
Copy link
Contributor

bors commented May 19, 2024

⌛ Testing commit 7b6a3d0 with merge 84b9b6d...

@bors
Copy link
Contributor

bors commented May 19, 2024

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 84b9b6d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 19, 2024
@bors bors merged commit 84b9b6d into rust-lang:master May 19, 2024
7 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 19, 2024
@Kobzol Kobzol deleted the ci-toolstate branch May 19, 2024 12:56
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (84b9b6d): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (primary -0.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.9% [-0.9%, -0.9%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.9% [-0.9%, -0.9%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 668.459s -> 669.294s (0.12%)
Artifact size: 316.25 MiB -> 316.25 MiB (0.00%)

@ehuss
Copy link
Contributor

ehuss commented Jun 5, 2024

@Kobzol Toolstate publishing seems to be broken since this PR. From https://github.com/rust-lang-nursery/rust-toolstate/commits/master/ there are no commits since before this PR landed. Is this something you can look into?

@Kobzol
Copy link
Contributor Author

Kobzol commented Jun 5, 2024

I am trying to make sense of toolstate publishing, and now I wonder if it has even working before. https://github.com/rust-lang-nursery/rust-toolstate/blob/master/_data/latest.json shouldn't this file be updated when anything changes? It seems like it hasn't been updated for two years. There are various comments in bootstrap code and the Python/Bash script that claim that latest.json should be updated on changes, but this does not seem to happen.

@ehuss
Copy link
Contributor

ehuss commented Jun 5, 2024

I believe the latest.json file is only modified when there is a change in status (going from passing to failing or vice versa). It's very likely there hasn't been a change in a while. The history file is what gets updated with every merge.

@ehuss
Copy link
Contributor

ehuss commented Jun 5, 2024

The nomicon status switched to failing in #124050 which should have updated the file.

@Kobzol
Copy link
Contributor Author

Kobzol commented Jun 5, 2024

It looks like this broke two years ago then. If you take a look at the history (https://github.com/rust-lang-nursery/rust-toolstate/commits/master/_data/latest.json), there were updates from time to time (every few days), but then it just stopped.

@ehuss
Copy link
Contributor

ehuss commented Jun 5, 2024

There were frequent updates because miri frequently broke. Then miri was switched to a subtree and removed from the toolstate system. That left just the books, which don't break very often.

@Kobzol
Copy link
Contributor Author

Kobzol commented Jun 5, 2024

I wonder if it's even worth it to keep this if it breaks once every two years 😆 Maybe we should just switch everything to subtrees. Anyway, it looks like the issue is with updating the history, not latest.json, I'll try to investigate more.

@ehuss
Copy link
Contributor

ehuss commented Jun 5, 2024

Anyway, it looks like the issue is with updating the history, not latest.json

Why does it seem that way? #124050 should have switched the nomicon from passing to failing in latest.json. The history should also be updated on every merge. It seems like all pushes to the toolstate repo aren't working.

@Kobzol
Copy link
Contributor Author

Kobzol commented Jun 5, 2024

Yeah, that's what I meant. This PR fixed (hopefully) updating of latest.json, but in the process broke updating of the history, which in turn broke updating of latest.json :) I'll send a PR with a fix soon.

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 6, 2024
CI: fix publishing of toolstate history

Hopefully fixes the upload of toolstate history that I broke in rust-lang#125145. The problem is that the toolstate environment variables need to be available not just when updating `latest.json` through the Bash/Python script, but also in the main CI workflow, where `history` is updated in bootstrap (the toolstate logic is distributed in two places :/ ).

The only tricky thing is how to pass the token secret to the CI job, as I need to enable it only for privileged (`auto`/`try`) builds. I assume that the secret is filled only on `rust-lang-ci`, not on `rust-lang`, so I chose the easiest way of just configuring the environment variable globally. We'll see if that works on PR CI.

r? `@ehuss`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 7, 2024
Rollup merge of rust-lang#126033 - Kobzol:fix-toolstate-history, r=ehuss

CI: fix publishing of toolstate history

Hopefully fixes the upload of toolstate history that I broke in rust-lang#125145. The problem is that the toolstate environment variables need to be available not just when updating `latest.json` through the Bash/Python script, but also in the main CI workflow, where `history` is updated in bootstrap (the toolstate logic is distributed in two places :/ ).

The only tricky thing is how to pass the token secret to the CI job, as I need to enable it only for privileged (`auto`/`try`) builds. I assume that the secret is filled only on `rust-lang-ci`, not on `rust-lang`, so I chose the easiest way of just configuring the environment variable globally. We'll see if that works on PR CI.

r? `@ehuss`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jun 7, 2024
CI: fix publishing of toolstate history

Hopefully fixes the upload of toolstate history that I broke in rust-lang/rust#125145. The problem is that the toolstate environment variables need to be available not just when updating `latest.json` through the Bash/Python script, but also in the main CI workflow, where `history` is updated in bootstrap (the toolstate logic is distributed in two places :/ ).

The only tricky thing is how to pass the token secret to the CI job, as I need to enable it only for privileged (`auto`/`try`) builds. I assume that the secret is filled only on `rust-lang-ci`, not on `rust-lang`, so I chose the easiest way of just configuring the environment variable globally. We'll see if that works on PR CI.

r? `@ehuss`
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Jun 28, 2024
CI: fix publishing of toolstate history

Hopefully fixes the upload of toolstate history that I broke in rust-lang/rust#125145. The problem is that the toolstate environment variables need to be available not just when updating `latest.json` through the Bash/Python script, but also in the main CI workflow, where `history` is updated in bootstrap (the toolstate logic is distributed in two places :/ ).

The only tricky thing is how to pass the token secret to the CI job, as I need to enable it only for privileged (`auto`/`try`) builds. I assume that the secret is filled only on `rust-lang-ci`, not on `rust-lang`, so I chose the easiest way of just configuring the environment variable globally. We'll see if that works on PR CI.

r? `@ehuss`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. 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.

7 participants