Support --jobserver-auth=fifo:PATH#49
Conversation
8608a22 to
d93ffd9
Compare
|
Thanks for working on this! I'm looking forward to the follow-up PR, any chance to (draft) PR that already? |
|
Haven't yet prepared the PR. However, making I'd like to hear what you expect :) |
alexcrichton
left a comment
There was a problem hiding this comment.
Thanks! Would it be possible to add some tests for this on CI?
GNU `make` 4.4, released in October 2022[^1]. The jobserver defaults to use named pipes (via `mkfifo(3)`) on supported platforms by introducing a new IPC style `--jobserver-auth=fifo:PATH`, which `PATH` is the path of fifo[^2]. This commit makes sure that the new style `--jobserver-auth=fifo:PATH` can be forwarded to inherited processes correctly. The support of creating a new client with named pipe will come as a follow-up pull request. [^1]: https://lists.gnu.org/archive/html/info-gnu/2022-10/msg00008.html [^2]: https://www.gnu.org/software/make/manual/html_node/POSIX-Jobserver.html
d93ffd9 to
0dc0684
Compare
|
Hi Alex. I amend the first commit to include your suggestions. Also add a new commit to test GNU Make 4.4 in CI, it compiles make from source as a temporary workaround. |
0dc0684 to
270fcd5
Compare
|
Thanks for the quick review, @alexcrichton! Sorry for bombarding your inbox again. If you've got time, it would be helpful if we have a release for this update. No hurry and take your time :) |
|
Sure thing, done! If y'all would like I'm happy to transfer ownership of this repo as well, but it's quite low traffic so I also don't mind continuing to shepherd it. |
| format!("{},{}", self.read.as_raw_fd(), self.write.as_raw_fd()) | ||
| match self { | ||
| Client::Pipe { read, write } => format!("{},{}", read.as_raw_fd(), write.as_raw_fd()), | ||
| Client::Fifo { path, .. } => format!("fifo:{}", path.to_str().unwrap()), |
There was a problem hiding this comment.
I think it's a bad idea to use the new fifo: syntax by default since this is a new syntax and not every program supports this.
There was a problem hiding this comment.
Correct. That's why we only support this syntax when inheriting from the environment, and it's really in a fifo style.
There was a problem hiding this comment.
Correct. That's why we only support this syntax when inheriting from the environment, and it's really in a fifo style.
Yeah this makes sense, though IMO it would be better to keep using the old syntax.
I do appreciate this new syntax since it means we no longer have to register a callback to Command::pre_exec, which blocks the use of vfork.
GNU
make4.4, released in October 20221. The jobserver defaultsto use named pipes (via
mkfifo(3)) on supported platforms byintroducing a new IPC style
--jobserver-auth=fifo:PATH, whichPATHis the path of fifo2.
This commit makes sure that the new style
--jobserver-auth=fifo:PATHcan be forwarded to inherited processes correctly.
The support of creating a new client with named pipe will come as a
follow-up pull request.
Footnotes
https://lists.gnu.org/archive/html/info-gnu/2022-10/msg00008.html ↩
https://www.gnu.org/software/make/manual/html_node/POSIX-Jobserver.html ↩