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

Store maximum queue length #7829

Merged
merged 1 commit into from
Jan 26, 2020

Conversation

Mark-Simulacrum
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum commented Jan 25, 2020

Previously, the queue length was constantly decreasing as we built crates, which
meant that we were incorrectly displaying the progress bar. In debug builds,
this even led to panics (due to underflow on subtraction).

Not sure if we can add a test case for this. I have made the panic unconditional on release/debug though by explicitly checking that current is less than the maximum for the progress bar.

Fixes #7731 (comment).

Previously, the queue length was constantly decreasing as we built crates, which
meant that we were incorrectly displaying the progress bar. In debug builds,
this even led to panics (due to underflow on subtraction).
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 25, 2020
@ehuss
Copy link
Contributor

ehuss commented Jan 26, 2020

Thanks for the speedy fix. Yea, it's kinda hard to test progress bar stuff. We should probably fix that one of these days.

@bors r+

@bors
Copy link
Contributor

bors commented Jan 26, 2020

📌 Commit dc6d219 has been approved by ehuss

@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 26, 2020
@bors
Copy link
Contributor

bors commented Jan 26, 2020

⌛ Testing commit dc6d219 with merge 2a0f0c8...

bors added a commit that referenced this pull request Jan 26, 2020
Store maximum queue length

Previously, the queue length was constantly decreasing as we built crates, which
meant that we were incorrectly displaying the progress bar. In debug builds,
this even led to panics (due to underflow on subtraction).

Not sure if we can add a test case for this. I have made the panic unconditional on release/debug though by explicitly checking that current is less than the maximum for the progress bar.

Fixes #7731 (comment).
@bors
Copy link
Contributor

bors commented Jan 26, 2020

☀️ Test successful - checks-azure
Approved by: ehuss
Pushing 2a0f0c8 to master...

@bors bors merged commit dc6d219 into rust-lang:master Jan 26, 2020
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jan 27, 2020
Update cargo

2 commits in b68b0978ab8012f871c80736fb910d14b89c4498..9d32b7b01409024b165545c568b1525d86e2b7cb
2020-01-24 18:26:23 +0000 to 2020-01-26 18:27:29 +0000
- Polish code to clarify meaning (rust-lang/cargo#7836)
- Store maximum queue length (rust-lang/cargo#7829)
bors added a commit that referenced this pull request Jan 31, 2020
[beta] Revert scalable jobserver.

This reverts #7731, #7829, and #7836.

This should prevent #7840 on beta. I feel more comfortable reverting this than merging #7844, since the impact is still unknown.
@ehuss ehuss added this to the 1.42.0 milestone Feb 6, 2022
@Mark-Simulacrum Mark-Simulacrum deleted the fix-progress-panics branch July 10, 2022 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants