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

do not panic on failure to acquire jobserver token #109694

Merged
merged 1 commit into from
Mar 29, 2023

Conversation

belovdv
Copy link
Contributor

@belovdv belovdv commented Mar 28, 2023

Purpose: remove panic.

Rust fails to acquire token if an error in build system occurs - environment variable contains incorrect jobserver-auth. It isn't ice so compiler shouldn't panic on such error.

Related issue: #46981

@rustbot
Copy link
Collaborator

rustbot commented Mar 28, 2023

r? @WaffleLapkin

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 28, 2023
@WaffleLapkin
Copy link
Member

r? compiler

@rustbot rustbot assigned petrochenkov and unassigned WaffleLapkin Mar 28, 2023
@petrochenkov
Copy link
Contributor

r? @Zoxc

@rustbot
Copy link
Collaborator

rustbot commented Mar 28, 2023

Failed to set assignee to Zoxc: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

@petrochenkov
Copy link
Contributor

r? @bjorn3

@rustbot rustbot assigned bjorn3 and unassigned petrochenkov Mar 28, 2023
@petrochenkov
Copy link
Contributor

It isn't ice so compiler shouldn't panic on such error.

What @belovdv is trying to say is that it's a build environment configuration issue, not an internal compiler error.
With rust-lang/jobserver-rs#52 we'll be able to diagnose such build environment issues better soon.

@Zoxc
Copy link
Contributor

Zoxc commented Mar 28, 2023

This change seems fine, but the parallel compiler does also rely a bit on this operation to succeed. An incorrect jobserver can also cause deadlocks for it.

@belovdv belovdv force-pushed the fix-panic-jobserver-token branch from 6a4c830 to e1fdb21 Compare March 28, 2023 13:37
@belovdv belovdv force-pushed the fix-panic-jobserver-token branch from e1fdb21 to be6a09f Compare March 28, 2023 14:23
@bjorn3
Copy link
Member

bjorn3 commented Mar 28, 2023

r=me when CI passes

@bjorn3
Copy link
Member

bjorn3 commented Mar 28, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Mar 28, 2023

📌 Commit be6a09f has been approved by bjorn3

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

Rollup of 6 pull requests

Successful merges:

 - rust-lang#109149 (Improve error message when writer is forgotten in write and writeln macro)
 - rust-lang#109367 (Streamline fast rejection)
 - rust-lang#109548 (AnnotationColumn struct to fix hard tab column numbers in errors)
 - rust-lang#109694 (do not panic on failure to acquire jobserver token)
 - rust-lang#109705 (new solver: check for intercrate mode when accessing the cache)
 - rust-lang#109708 (Specialization involving RPITITs is broken so ignore the diagnostic differences)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 09c1398 into rust-lang:master Mar 29, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 29, 2023
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. T-compiler Relevant to the compiler 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