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

rustc: document the jobserver #121564

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

ojeda
Copy link
Contributor

@ojeda ojeda commented Feb 24, 2024

Explicitly document that the jobserver may be used by rustc, as well as recommend the + indicator for integration of rustc into GNU Make.

In particular, show the warning to increase the chances that this document is found when searching for solutions online.

In addition, add a note about the issue with GNU Make 4.3 since it is important that users realize they should do this even if they do not expect parallelism from rustc.

Finally, show how to workaround the issue of $(shell ...) calls in recursive Make (which e.g. was needed for the Linux kernel).

The GNU Make 4.4 case under --jobserver-style=pipe is not added since it got fixed after Rust 1.76.0 already (i.e. rustc will not warn if it finds the negative file descriptors).

From: #120515
Cc: @petrochenkov @belovdv @weihanglo @bjorn3


v2: To be able to use tab characters for the Make examples, add <!-- ignore-tidy-{check} --> support to tidy.
v3: Added "Integration with build systems" section to hold the GNU Make one. Added "by clearing the MAKEFLAGS variable". Added "aforementioned" so that it is clear we are talking about the warning above.
v4: Added CMake subsection. Added a note that rustc may be affected by other flags, e.g. CARGO_MAKEFLAGS.

@rustbot
Copy link
Collaborator

rustbot commented Feb 24, 2024

r? @ehuss

rustbot has assigned @ehuss.
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 the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 24, 2024
@rust-log-analyzer

This comment has been minimized.

To be used to skip the `tab` check in `jobserver.md`.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 24, 2024
@petrochenkov petrochenkov self-assigned this Feb 25, 2024

Internally, `rustc` may take advantage of parallelism. `rustc` will coordinate
with the build system calling it if a [GNU Make jobserver] is passed in the
`MAKEFLAGS` environment variable.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be good to mention what will happen if a jobserver cannot be found? Maybe something like:

Suggested change
`MAKEFLAGS` environment variable.
`MAKEFLAGS` environment variable. If a jobserver is not defined, then `rustc` will use a fixed limit on the maximum number of threads.

Copy link
Contributor Author

@ojeda ojeda Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not sure: I considered doing that, but does rustc want to commit to anything here? (i.e. it is always going to be the same?).

In addition, if we mention it, should we say what "fixed limit" means? Should we mention/link to the parallel compiler pages, -Zthreads, -Ccodegen-units, etc.?

src/doc/rustc/src/jobserver.md Outdated Show resolved Hide resolved
Comment on lines +7 to +16
Starting with Rust 1.76.0, `rustc` will warn if a jobserver appears to be
available but is not accessible, e.g.:

```console
$ echo 'fn main() {}' | MAKEFLAGS=--jobserver-auth=3,4 rustc -
warning: failed to connect to jobserver from environment variable `MAKEFLAGS="--jobserver-auth=3,4"`: cannot open file descriptor 3 from the jobserver environment variable value: Bad file descriptor (os error 9)
|
= note: the build environment is likely misconfigured
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seemed a little unusual to call out near the beginning of the chapter. Perhaps a different way to flow this would be to move this down below after the paragraph that says "if the + indicator is removed", and show that as an illustration of what happens when + is removed.

Copy link
Contributor Author

@ojeda ojeda Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I put it at the top is that the warning applies to all build systems, i.e. the intention was that we would have subsections for other build systems too if needed, but it isn't clear since we only have one, as you mention (and indeed I did have to do a bit of a dance to make the text work without having to repeat the warning block, which is what I originally had, but it felt too heavy).

On the other hand, it may be true that other build systems do not have this issue (i.e. either they don't use a jobserver, or if they do, they enable it "properly", so perhaps nobody else will actually hit that warning, but it is hard to say). Still, it seemed like conceptually it was something outside GNU Make in particular.

Perhaps it could help adding a subsection called "Integrations" or similar with a small text like "These are recommendations for integration of rustc with different build systems." and then make the GNU Make one a subsubsection of that.

I also thought about adding a trivial one about Cargo, saying something like "Cargo already handles it".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a new version with what I meant here so that it is simpler to see.

ojeda added a commit to Rust-for-Linux/linux that referenced this pull request Feb 29, 2024
`rustc` (like Cargo) may take advantage of the jobserver at any time
(e.g. for backend parallelism, or eventually frontend too). In the kernel,
we call `rustc` with `-Ccodegen-units=1` (and `-Zthreads` is 1 so far),
so we do not expect parallelism. However, in the upcoming Rust 1.76.0, a
warning is emitted by `rustc` [1] when it cannot connect to the jobserver
it was passed (in many cases, but not all: compiling and `--print sysroot`
do, but `--version` does not). And given GNU Make always passes
the jobserver in the environment variable (even when a line is deemed
non-recursive), `rustc` will end up complaining about it (in particular
in Make 4.3 where there is only the simple pipe jobserver style).

One solution is to remove the jobserver from `MAKEFLAGS`. However, we
can mark the lines with calls to `rustc` (and Cargo) as recursive, which
looks simpler. This is being documented as a recommendation in `rustc`
[2] and allows us to be ready for the time we may use parallelism inside
`rustc` (potentially now, if a user passes `-Zthreads`). Thus do so.

Similarly, do the same for `rustdoc` and `cargo` calls.

Finally, there is one case that the solution does not cover, which is the
`$(shell ...)` call we have. Thus, for that one, set an empty `MAKEFLAGS`
environment variable.

Link: rust-lang/rust#120515 [1]
Acked-by: Masahiro Yamada <masahiroy@kernel.org>
Link: rust-lang/rust#121564 [2]
Link: https://lore.kernel.org/r/20240217002638.57373-1-ojeda@kernel.org
[ Reworded to add link to PR documenting the recommendation. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Internally, `rustc` may take advantage of parallelism. `rustc` will coordinate
with the build system calling it if a [GNU Make jobserver] is passed in the
`MAKEFLAGS` environment variable.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may use other env vars too, like CARGO_MAKEFLAGS, not sure whether we should document the exhaustive list here.
If something goes wrong, then rust will print the specific env var that was used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could mention that other flags could also have an effect and/or link to the Cargo docs, since they are publicly documented anyway. Though it wouldn't hurt to document all env vars in rustc somewhere -- as a user, I would appreciate it, even if they are documented as "do not use".

I have added the mention in v4.


```make
x:
+@echo 'fn main() {}' | rustc -
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+ works in make itself, but not in some wrappers around it, like cmake.
The "universal workaround" for such cases is to use $(MAKE) (literally) somewhere on the command line, then make will pass the jobserver even without +.

I haven't use the $(shell ...) construction mentioned below, but perhaps this method will work for it as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I use $(MAKE) in the example below (in the recursive Make case), but the GNU Make manual mentions + in the jobserver page as well as in the errors one (and I agree it is cleaner for this purpose than $(MAKE)), so I think it is best to keep + here. After all, we are in the section about GNU Make here, not other build systems.

But mentioning how to workaround it in CMake sounds like a good idea (even if it is just for their Makefile generator). After all, the intention of the document is to describe the recommendations for different build systems eventually, not just GNU Make.

I looked a bit into it, and I see issues like https://gitlab.kitware.com/cmake/cmake/-/issues/17781. However, I also found JOB_SERVER_AWARE from 3.28, documented in https://cmake.org/cmake/help/latest/command/ that actually adds a + in the generated Makefiles.

So I added that new CMake section in v4. Please take a look!

`make -j2`, then the aforementioned warning will trigger.

For calls to `rustc` inside `$(shell ...)` inside a recursive Make, one can
disable the jobserver manually by clearing the `MAKEFLAGS` variable, e.g.:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps worth emphasizing that even if removing the env var is a possible workaround, is not the recommended solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not aware of a solution for that case, so I am not sure what we could recommend instead, which is why I don't mention anything. Though perhaps we could recommend disabling all parallel features in rustc, though then it means we should probably document how to do so and maintain the list, unless there is an easy way to do so globally.

@petrochenkov petrochenkov removed their assignment Mar 1, 2024
Explicitly document that the jobserver may be used by `rustc` and show
the warning to increase the chances that this document is found when
searching for solutions online.

In particular, add a section about the interaction with build systems,
which is intended to contain recommendations on how to integrate `rustc`
with different built systems.

For GNU Make, recommend using the `+` indicator. In addition, add a
note about the issue with GNU Make 4.3 since it is important that users
realize they should do this even if they do not expect parallelism from
`rustc`.  Finally, show how to workaround the issue of `$(shell ...)`
calls in recursive Make (which e.g. was needed for the Linux kernel).

The GNU Make 4.4 case under `--jobserver-style=pipe` is not added since
it got fixed after Rust 1.76.0 already (i.e. `rustc` will not warn if
it finds the negative file descriptors).

For CMake, recommend using `JOB_SERVER_AWARE` and show a workaround using
`$(MAKE)` for earlier versions (when using the Makefile generator).

From: rust-lang#120515
Cc: @petrochenkov @belovdv @weihanglo @bjorn3
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
bertschingert pushed a commit to bertschingert/bcachefs that referenced this pull request Mar 9, 2024
`rustc` (like Cargo) may take advantage of the jobserver at any time
(e.g. for backend parallelism, or eventually frontend too). In the kernel,
we call `rustc` with `-Ccodegen-units=1` (and `-Zthreads` is 1 so far),
so we do not expect parallelism. However, in the upcoming Rust 1.76.0, a
warning is emitted by `rustc` [1] when it cannot connect to the jobserver
it was passed (in many cases, but not all: compiling and `--print sysroot`
do, but `--version` does not). And given GNU Make always passes
the jobserver in the environment variable (even when a line is deemed
non-recursive), `rustc` will end up complaining about it (in particular
in Make 4.3 where there is only the simple pipe jobserver style).

One solution is to remove the jobserver from `MAKEFLAGS`. However, we
can mark the lines with calls to `rustc` (and Cargo) as recursive, which
looks simpler. This is being documented as a recommendation in `rustc`
[2] and allows us to be ready for the time we may use parallelism inside
`rustc` (potentially now, if a user passes `-Zthreads`). Thus do so.

Similarly, do the same for `rustdoc` and `cargo` calls.

Finally, there is one case that the solution does not cover, which is the
`$(shell ...)` call we have. Thus, for that one, set an empty `MAKEFLAGS`
environment variable.

Link: rust-lang/rust#120515 [1]
Acked-by: Masahiro Yamada <masahiroy@kernel.org>
Link: rust-lang/rust#121564 [2]
Link: https://lore.kernel.org/r/20240217002638.57373-1-ojeda@kernel.org
[ Reworded to add link to PR documenting the recommendation. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
bertschingert pushed a commit to bertschingert/bcachefs that referenced this pull request Mar 9, 2024
`rustc` (like Cargo) may take advantage of the jobserver at any time
(e.g. for backend parallelism, or eventually frontend too). In the kernel,
we call `rustc` with `-Ccodegen-units=1` (and `-Zthreads` is 1 so far),
so we do not expect parallelism. However, in the upcoming Rust 1.76.0, a
warning is emitted by `rustc` [1] when it cannot connect to the jobserver
it was passed (in many cases, but not all: compiling and `--print sysroot`
do, but `--version` does not). And given GNU Make always passes
the jobserver in the environment variable (even when a line is deemed
non-recursive), `rustc` will end up complaining about it (in particular
in Make 4.3 where there is only the simple pipe jobserver style).

One solution is to remove the jobserver from `MAKEFLAGS`. However, we
can mark the lines with calls to `rustc` (and Cargo) as recursive, which
looks simpler. This is being documented as a recommendation in `rustc`
[2] and allows us to be ready for the time we may use parallelism inside
`rustc` (potentially now, if a user passes `-Zthreads`). Thus do so.

Similarly, do the same for `rustdoc` and `cargo` calls.

Finally, there is one case that the solution does not cover, which is the
`$(shell ...)` call we have. Thus, for that one, set an empty `MAKEFLAGS`
environment variable.

Link: rust-lang/rust#120515 [1]
Acked-by: Masahiro Yamada <masahiroy@kernel.org>
Link: rust-lang/rust#121564 [2]
Link: https://lore.kernel.org/r/20240217002638.57373-1-ojeda@kernel.org
[ Reworded to add link to PR documenting the recommendation. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
bertschingert pushed a commit to bertschingert/bcachefs that referenced this pull request Mar 12, 2024
`rustc` (like Cargo) may take advantage of the jobserver at any time
(e.g. for backend parallelism, or eventually frontend too). In the kernel,
we call `rustc` with `-Ccodegen-units=1` (and `-Zthreads` is 1 so far),
so we do not expect parallelism. However, in the upcoming Rust 1.76.0, a
warning is emitted by `rustc` [1] when it cannot connect to the jobserver
it was passed (in many cases, but not all: compiling and `--print sysroot`
do, but `--version` does not). And given GNU Make always passes
the jobserver in the environment variable (even when a line is deemed
non-recursive), `rustc` will end up complaining about it (in particular
in Make 4.3 where there is only the simple pipe jobserver style).

One solution is to remove the jobserver from `MAKEFLAGS`. However, we
can mark the lines with calls to `rustc` (and Cargo) as recursive, which
looks simpler. This is being documented as a recommendation in `rustc`
[2] and allows us to be ready for the time we may use parallelism inside
`rustc` (potentially now, if a user passes `-Zthreads`). Thus do so.

Similarly, do the same for `rustdoc` and `cargo` calls.

Finally, there is one case that the solution does not cover, which is the
`$(shell ...)` call we have. Thus, for that one, set an empty `MAKEFLAGS`
environment variable.

Link: rust-lang/rust#120515 [1]
Acked-by: Masahiro Yamada <masahiroy@kernel.org>
Link: rust-lang/rust#121564 [2]
Link: https://lore.kernel.org/r/20240217002638.57373-1-ojeda@kernel.org
[ Reworded to add link to PR documenting the recommendation. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
jannau pushed a commit to jannau/linux that referenced this pull request Mar 25, 2024
`rustc` (like Cargo) may take advantage of the jobserver at any time
(e.g. for backend parallelism, or eventually frontend too). In the kernel,
we call `rustc` with `-Ccodegen-units=1` (and `-Zthreads` is 1 so far),
so we do not expect parallelism. However, in the upcoming Rust 1.76.0, a
warning is emitted by `rustc` [1] when it cannot connect to the jobserver
it was passed (in many cases, but not all: compiling and `--print sysroot`
do, but `--version` does not). And given GNU Make always passes
the jobserver in the environment variable (even when a line is deemed
non-recursive), `rustc` will end up complaining about it (in particular
in Make 4.3 where there is only the simple pipe jobserver style).

One solution is to remove the jobserver from `MAKEFLAGS`. However, we
can mark the lines with calls to `rustc` (and Cargo) as recursive, which
looks simpler. This is being documented as a recommendation in `rustc`
[2] and allows us to be ready for the time we may use parallelism inside
`rustc` (potentially now, if a user passes `-Zthreads`). Thus do so.

Similarly, do the same for `rustdoc` and `cargo` calls.

Finally, there is one case that the solution does not cover, which is the
`$(shell ...)` call we have. Thus, for that one, set an empty `MAKEFLAGS`
environment variable.

Link: rust-lang/rust#120515 [1]
Acked-by: Masahiro Yamada <masahiroy@kernel.org>
Link: rust-lang/rust#121564 [2]
Link: https://lore.kernel.org/r/20240217002638.57373-1-ojeda@kernel.org
[ Reworded to add link to PR documenting the recommendation. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
happy-thw pushed a commit to happy-thw/rust-for-linux that referenced this pull request Apr 2, 2024
`rustc` (like Cargo) may take advantage of the jobserver at any time
(e.g. for backend parallelism, or eventually frontend too). In the kernel,
we call `rustc` with `-Ccodegen-units=1` (and `-Zthreads` is 1 so far),
so we do not expect parallelism. However, in the upcoming Rust 1.76.0, a
warning is emitted by `rustc` [1] when it cannot connect to the jobserver
it was passed (in many cases, but not all: compiling and `--print sysroot`
do, but `--version` does not). And given GNU Make always passes
the jobserver in the environment variable (even when a line is deemed
non-recursive), `rustc` will end up complaining about it (in particular
in Make 4.3 where there is only the simple pipe jobserver style).

One solution is to remove the jobserver from `MAKEFLAGS`. However, we
can mark the lines with calls to `rustc` (and Cargo) as recursive, which
looks simpler. This is being documented as a recommendation in `rustc`
[2] and allows us to be ready for the time we may use parallelism inside
`rustc` (potentially now, if a user passes `-Zthreads`). Thus do so.

Similarly, do the same for `rustdoc` and `cargo` calls.

Finally, there is one case that the solution does not cover, which is the
`$(shell ...)` call we have. Thus, for that one, set an empty `MAKEFLAGS`
environment variable.

Link: rust-lang/rust#120515 [1]
Acked-by: Masahiro Yamada <masahiroy@kernel.org>
Link: rust-lang/rust#121564 [2]
Link: https://lore.kernel.org/r/20240217002638.57373-1-ojeda@kernel.org
[ Reworded to add link to PR documenting the recommendation. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
jannau pushed a commit to jannau/linux that referenced this pull request Apr 8, 2024
`rustc` (like Cargo) may take advantage of the jobserver at any time
(e.g. for backend parallelism, or eventually frontend too). In the kernel,
we call `rustc` with `-Ccodegen-units=1` (and `-Zthreads` is 1 so far),
so we do not expect parallelism. However, in the upcoming Rust 1.76.0, a
warning is emitted by `rustc` [1] when it cannot connect to the jobserver
it was passed (in many cases, but not all: compiling and `--print sysroot`
do, but `--version` does not). And given GNU Make always passes
the jobserver in the environment variable (even when a line is deemed
non-recursive), `rustc` will end up complaining about it (in particular
in Make 4.3 where there is only the simple pipe jobserver style).

One solution is to remove the jobserver from `MAKEFLAGS`. However, we
can mark the lines with calls to `rustc` (and Cargo) as recursive, which
looks simpler. This is being documented as a recommendation in `rustc`
[2] and allows us to be ready for the time we may use parallelism inside
`rustc` (potentially now, if a user passes `-Zthreads`). Thus do so.

Similarly, do the same for `rustdoc` and `cargo` calls.

Finally, there is one case that the solution does not cover, which is the
`$(shell ...)` call we have. Thus, for that one, set an empty `MAKEFLAGS`
environment variable.

Link: rust-lang/rust#120515 [1]
Acked-by: Masahiro Yamada <masahiroy@kernel.org>
Link: rust-lang/rust#121564 [2]
Link: https://lore.kernel.org/r/20240217002638.57373-1-ojeda@kernel.org
[ Reworded to add link to PR documenting the recommendation. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
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 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants