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

avoid depending on child process execution when the target OS does not support it #10750

Closed
andrewrk opened this issue Feb 1, 2022 · 6 comments
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Feb 1, 2022

There are a few places in the Zig compiler where we invoke a child process, but none of them are strictly necessary. This issue is to detect when the OS does not support child process execution and produce a build of the compiler that does not attempt to use child processes.

For now the detection logic can be simply:

const can_spawn = switch (builtin.os.tag) {
    .wasi => false,
    else => true,
};

Here is a complete list of everywhere Zig might want to spawn a child process:

  • zig test - instead print a message about not being able to spawn child processes and exit success.
  • zig run - instead print "unable to spawn child processes" and exit failure.
  • for clang we can invoke clang_main directly instead of as a child process
  • for lld we can invoke the lld library functions instead of as child process
  • same thing for zig ar, zig dlltool, etc.
  • for detecting native libc, just return error.LibcNotDetected or whatever the appropriate error is.

This is a prerequisite for #10716.

@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. frontend Tokenization, parsing, AstGen, Sema, and Liveness. labels Feb 1, 2022
@andrewrk andrewrk added this to the 0.10.0 milestone Feb 1, 2022
@marler8997
Copy link
Contributor

I'd be curious to see how significant the performance cost is for sub processes on platforms that support it. With this in place we could easily find out!

@zigazeljko
Copy link
Contributor

Doesn't zig build also spawn child processes?

@fogti
Copy link
Contributor

fogti commented Feb 1, 2022

Why should zig test exit with success?

@marler8997
Copy link
Contributor

Doesn't zig build also spawn child processes?

Oh yes. To compile anything zig build will spawn a subprocess like zig build-exe ....

@matu3ba
Copy link
Contributor

matu3ba commented Feb 2, 2022

This should probably be first implemented before #1356 with potentially panicking subprocess being started from the subprocess that supervises test results.

topolarity added a commit to topolarity/zig that referenced this issue Feb 3, 2022
In accordance with the requesting issue (ziglang#10750):
- `zig test` skips any tests that it cannot spawn, returning success
- `zig run` and `zig build` exit with failure, reporting the command the cannot run
- `zig clang`, `zig ar`, etc. already punt directly to the appropriate clang/lld main(), even without this change

This comes with a number of limitations for no-spawn targets:
- `exec()` and related Builder functions error at run-time
- Libc Detection is not supported
topolarity added a commit to topolarity/zig that referenced this issue Feb 3, 2022
In accordance with the requesting issue (ziglang#10750):
- `zig test` skips any tests that it cannot spawn, returning success
- `zig run` and `zig build` exit with failure, reporting the command the cannot be run
- `zig clang`, `zig ar`, etc. already punt directly to the appropriate clang/lld main(), even before this change
- Native `libc` Detection is not supported

Additionally, `exec()` and related Builder functions error at run-time, reporting the command that cannot be run
topolarity added a commit to topolarity/zig that referenced this issue Feb 5, 2022
In accordance with the requesting issue (ziglang#10750):
- `zig test` skips any tests that it cannot spawn, returning success
- `zig run` and `zig build` exit with failure, reporting the command the cannot be run
- `zig clang`, `zig ar`, etc. already punt directly to the appropriate clang/lld main(), even before this change
- Native `libc` Detection is not supported

Additionally, `exec()` and related Builder functions error at run-time, reporting the command that cannot be run
topolarity added a commit to topolarity/zig that referenced this issue Feb 5, 2022
In accordance with the requesting issue (ziglang#10750):
- `zig test` skips any tests that it cannot spawn, returning success
- `zig run` and `zig build` exit with failure, reporting the command the cannot be run
- `zig clang`, `zig ar`, etc. already punt directly to the appropriate clang/lld main(), even before this change
- Native `libc` Detection is not supported

Additionally, `exec()` and related Builder functions error at run-time, reporting the command that cannot be run
topolarity added a commit to topolarity/zig that referenced this issue Feb 5, 2022
In accordance with the requesting issue (ziglang#10750):
- `zig test` skips any tests that it cannot spawn, returning success
- `zig run` and `zig build` exit with failure, reporting the command the cannot be run
- `zig clang`, `zig ar`, etc. already punt directly to the appropriate clang/lld main(), even before this change
- Native `libc` Detection is not supported

Additionally, `exec()` and related Builder functions error at run-time, reporting the command that cannot be run
andrewrk pushed a commit to topolarity/zig that referenced this issue Feb 7, 2022
In accordance with the requesting issue (ziglang#10750):
- `zig test` skips any tests that it cannot spawn, returning success
- `zig run` and `zig build` exit with failure, reporting the command the cannot be run
- `zig clang`, `zig ar`, etc. already punt directly to the appropriate clang/lld main(), even before this change
- Native `libc` Detection is not supported

Additionally, `exec()` and related Builder functions error at run-time, reporting the command that cannot be run
@andrewrk
Copy link
Member Author

andrewrk commented Feb 7, 2022

Landed in 2113538

@andrewrk andrewrk closed this as completed Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Projects
None yet
Development

No branches or pull requests

5 participants