-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix spawn not used on linux #25206
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
fix spawn not used on linux #25206
Conversation
|
||
const useProcessAuxSpawn = declared(posix_spawn) and not defined(useFork) and | ||
not defined(useClone) and not defined(linux) | ||
not (defined(useClone) and defined(linux)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not (defined(useClone) and defined(linux)) | |
not defined(useClone) |
var dataCopy = data | ||
|
||
when defined(useClone): | ||
when defined(useClone) and defined(linux): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when defined(useClone) and defined(linux): | |
when defined(useClone): |
Assumption here is that useClone implies Linux because it's a linux specific call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can lead to a bug in the current implementation, e.g. when useFork
and useClone
are defined, but we're not on linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the top of osproc.nim
there is this:
when defined(linux) and defined(useClone):
import std/linux
So I assumed it can happen that useClone
is defined, but not linux
. If this shouldn't happen, maybe an assertion should be added here, and useClone & linux
check reduced to useClone
?
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
Subj, among other things slows down the compilation of large projects on linux significantly. (cherry picked from commit 440b55a)
Subj, among other things slows down the compilation of large projects on linux significantly.