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

bump jobserver to respect --jobserver-auth=fifo:PATH #11767

Merged
merged 1 commit into from
Feb 28, 2023

Conversation

weihanglo
Copy link
Member

@weihanglo weihanglo commented Feb 25, 2023

What does this PR try to resolve?

tl;dr: GNU Make 4.4 switches to use named pipes instead of anonymous simple pipes. Cargo needs to respect that to prevent clients from breaking.

See rust-lang/jobserver-rs#49

How should we test and review this PR?

  1. Get a GNU Make 4.4 binary.
  2. Change lines like this pointing to your binary.
    For example, on macOS I changed to gmake to invoke GNU Make from homebrew.
  3. Run cargo t --test testsuite jobserver::

If you want to test this patch live, you can also execute cargo build from make -j2 to test the behaviour.
(I usually run procs -w rustc to observe the amount of rustc invocations.)

# Makefile
all:
	+./target/debug/cargo build --release
clean:
	cargo clean --release

Additional information

Fallback workaround: add --jobserver-style=pipe if you are using GNU Make 4.4 and this PR has yet got landed.

Some distros that already got GNU Make 4.4:

@rustbot
Copy link
Collaborator

rustbot commented Feb 25, 2023

r? @ehuss

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 25, 2023
@weihanglo weihanglo marked this pull request as ready for review February 28, 2023 16:22
@weihanglo weihanglo changed the title bump jobserver to respects --jobserver-style=fifo' bump jobserver to respect --jobserver-auth=fifo:PATH Feb 28, 2023
@weihanglo
Copy link
Member Author

BTW, should we update rustc and other crates as well in the ecosystem, i.e. cc?

@ehuss
Copy link
Contributor

ehuss commented Feb 28, 2023

BTW, should we update rustc and other crates as well in the ecosystem, i.e. cc?

When this merges in rust-lang/rust, it should implicitly update rustc.

As for users of cc, bumping its requirement would only be helpful for users who explicitly update cc without updating any other dependencies (such as cargo update -p cc), right? I'm not sure how common that would be. Of course it shouldn't hurt, so if you want to send a PR to update it, that sounds good.

This also only really matters when cargo is being run under a relatively recent make, correct?

Is this ready to merge?

@weihanglo
Copy link
Member Author

Yep. It should behave as before for any GNU Make older than v4.4.
And yes, it is ready to merge.

@ehuss
Copy link
Contributor

ehuss commented Feb 28, 2023

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Feb 28, 2023

📌 Commit b9bfda5 has been approved by ehuss

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 Feb 28, 2023
@bors
Copy link
Contributor

bors commented Feb 28, 2023

⌛ Testing commit b9bfda5 with merge 9880b40...

@bors
Copy link
Contributor

bors commented Feb 28, 2023

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 9880b40 to master...

@bors bors merged commit 9880b40 into rust-lang:master Feb 28, 2023
weihanglo added a commit to weihanglo/rust that referenced this pull request Feb 28, 2023
10 commits in 9d5b32f503fc099c4064298465add14d4bce11e6..9880b408a3af50c08fab3dbf4aa2a972df71e951
2023-02-22 23:04:16 +0000 to 2023-02-28 19:39:39 +0000

- bump jobserver to respect `--jobserver-auth=fifo:PATH` (rust-lang/cargo#11767)
- Addition of support for -F as an alias for --features (rust-lang/cargo#11774)
- Added documentation for the configuration discovery of `cargo install` to the man pages (rust-lang/cargo#11763)
- Fix Cargo removing the sparse+ prefix from sparse URLs in .crates.toml (rust-lang/cargo#11756)
- Fix warning with tempfile (rust-lang/cargo#11771)
- Error message for transitive artifact dependencies with targets the package doesn't directly interact with (rust-lang/cargo#11643)
- Fix tests with nondeterministic ordering (rust-lang/cargo#11766)
- Make some blocking tests non-blocking (rust-lang/cargo#11650)
- Suggest cargo add when installing library crate (rust-lang/cargo#11410)
- chore: bump is-terminal to 0.4.4 (rust-lang/cargo#11759)
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 1, 2023
Update cargo

10 commits in 9d5b32f503fc099c4064298465add14d4bce11e6..9880b408a3af50c08fab3dbf4aa2a972df71e951 2023-02-22 23:04:16 +0000 to 2023-02-28 19:39:39 +0000

- bump jobserver to respect `--jobserver-auth=fifo:PATH` (rust-lang/cargo#11767)
- Addition of support for -F as an alias for --features (rust-lang/cargo#11774)
- Added documentation for the configuration discovery of `cargo install` to the man pages (rust-lang/cargo#11763)
- Fix Cargo removing the sparse+ prefix from sparse URLs in .crates.toml (rust-lang/cargo#11756)
- Fix warning with tempfile (rust-lang/cargo#11771)
- Error message for transitive artifact dependencies with targets the package doesn't directly interact with (rust-lang/cargo#11643)
- Fix tests with nondeterministic ordering (rust-lang/cargo#11766)
- Make some blocking tests non-blocking (rust-lang/cargo#11650)
- Suggest cargo add when installing library crate (rust-lang/cargo#11410)
- chore: bump is-terminal to 0.4.4 (rust-lang/cargo#11759)

r? `@ghost`
@weihanglo weihanglo deleted the jobserver-style-fifo branch March 1, 2023 14:47
@ehuss ehuss added this to the 1.69.0 milestone Mar 9, 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants