Skip to content

WIP: Update process spawning in platforms - #4885

Open
mwichmann wants to merge 1 commit into
SCons:masterfrom
mwichmann:maint/spawn
Open

WIP: Update process spawning in platforms#4885
mwichmann wants to merge 1 commit into
SCons:masterfrom
mwichmann:maint/spawn

Conversation

@mwichmann

@mwichmann mwichmann commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Note: this is not ready for inclusion in the currently pending release, it needs more proof. Pushing as a PR so can get a full CI run on it.

Edit: that's why we do these exercises, I guess. On the two Windows github runners that use Python 3.9, three tests fail (the same three), with an odd error:

Fatal Python error: _Py_HashRandomization_Init: failed to get random numbers to initialize Python

Exploring the mystery...

Assisted by AI for the win32 platform module, which is complicated by the fact that switching from spawnv* functions to subprocess means arguments have to be prepared differently; subprocess has a helper function that runs if an argument vector is passed, except it doesn't help our use case because we've already done some quoting work, and stuff would end up with excess quotes.

Default is now simplified functions that just use subprocess.run(). On posix, the escape function is now shlex.quote from stdlib. On win32, some funky dances with temporary files are eliminated, as subprocess is quite capable of capturing output.

In Subst.py, quote_spaces is generalized a bit, and a TODO is left in the comments - it doesn't look like CmdStringHolder.escape ever gets called with a quote_func argument, so the current way is a bit inefficient, but it's unlikely to be a compelling improvement so seems not worth the research to be sure "nothing breaks".

The change of escape function on posix means a couple of places in one test see a change: some arguments that don't need quoting are no longer quoted. Specifically, it's these two former expectations:

BF:    %(_python_)s myfail.py f03 f04 "f04" "f04.in"
BF:    %(_python_)s myfail.py f04 f05 "f05" "f05.in"

It's possible that the former slightly funky posix quoting was actually to match the win32 output, so tests like this can work with the same output on both flavors. For now, the test is modified to paste in a platform-specific variable to account for the current difference. Going back to the old quoting style is, in a sense, disjoint from the change to use subprocess.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt and RELEASE.txt (and read the README.rst).
  • I have updated the appropriate documentation

Default is now simplified functions that just use subprocess.run().
On posix, the escape function is now shlex.quote from stdlib.  On win32,
some funky dances with temporary files are eliminated, as subprocess is
quite capable of capturing output.

In Subst, quote_spaces is generalized a bit, and a TODO is left in the
comments - it doesn't look like CmdStringHolder.escape ever gets called
with a quote_func argument, so the current way is a bit inefficient,
but it's unlikely to be a compelling improvement so seems not worth the
research to be sure "nothing breaks".

The change of escape function on posix means a couple of places in one
test see a change: some arguments that don't need quoting are no longer
quoted. Specifically, it's these two former expectations:

BF:    %(_python_)s myfail.py f03 f04 "f04" "f04.in"
BF:    %(_python_)s myfail.py f04 f05 "f05" "f05.in"

It's possible that the former slightly funky posix quoting, was to match
the win32 output, so tests like this can work with the same output on
both flavors.  The test is modified to paste in a platform-specific
variable to account for the current difference. Going back to the old
quoting style is, in a sense, disjoint from the change to use subprocess.

Signed-off-by: Mats Wichmann <mats@linux.com>
@mwichmann mwichmann added maintenance Tasks to maintain internal SCons code/tools platform code labels Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Tasks to maintain internal SCons code/tools platform code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant