Skip to content

fix: use exec to invoke the stage-2 bootstrap for non-zip case#2047

Merged
rickeylev merged 3 commits intobazel-contrib:mainfrom
chowder:bootstrap-fix
Jul 10, 2024
Merged

fix: use exec to invoke the stage-2 bootstrap for non-zip case#2047
rickeylev merged 3 commits intobazel-contrib:mainfrom
chowder:bootstrap-fix

Conversation

@chowder
Copy link
Contributor

@chowder chowder commented Jul 8, 2024

When the two-stage bootstrap is used, the parent shell process runs python as a child
process, which changes how signals are propagated. Specifically, if a signal is sent
directly to the parent (e.g. kill $parent), the child process (python) won't receive
it and it will appear to be ignored. This is because the parent process is busy waiting
for the child process.

To fix, invoke the python process using exec instead. Because the process is entirely
replaced, signals are sent directly to the replacement. This can't be used for zip files,
though, because they rely on a catching the exit signal to perform cleanup of the extracted
files.

Fixes #2043

@chowder chowder requested review from aignas and rickeylev as code owners July 8, 2024 17:17
@google-cla
Copy link

google-cla bot commented Jul 8, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@chowder chowder changed the title fix: use exec to invoke the stage-2 bootstrap in the common case fix: use exec to invoke the stage-2 bootstrap in the common case Jul 8, 2024
@rickeylev rickeylev changed the title fix: use exec to invoke the stage-2 bootstrap in the common case fix: use exec to invoke the stage-2 bootstrap for non-zip case Jul 8, 2024
Also remove a defunct comment since we're here
Copy link
Collaborator

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

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

LGTM.

#2045 adds the basics for testing this, so we can put a test in after that lands.

@rickeylev rickeylev enabled auto-merge July 10, 2024 02:28
@rickeylev rickeylev disabled auto-merge July 10, 2024 02:30
@rickeylev rickeylev enabled auto-merge July 10, 2024 02:33
@rickeylev rickeylev added this pull request to the merge queue Jul 10, 2024
Merged via the queue into bazel-contrib:main with commit e690975 Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(Linux) The stage-1 bootstrap script does not propagate certain signals to the stage-2 bootstrap script.

2 participants