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

Rollback #14510 because it causes remote test execution to fail #17147

Closed
wants to merge 1 commit into from

Conversation

shs96c
Copy link
Contributor

@shs96c shs96c commented Jan 5, 2023

When the parent process is run as a different user to the test itself (as can happen in some remote build environments, such as buildbarn) using kill -0 $PPID fails, causing the test to be prematurely killed.

This PR rolls back the (nicer, cleaner, not always working) fix and replaces it with the (less nice, less clean, more often working) check that was there before.

Tested with a local build against buildbarn.

…fail

When the parent process is run as a different user to the test
itself (as can happen in some remote build environments, such
as buildbarn) using `kill -0 $PPID` fails, causing the test to
be prematurely killed.

This PR rolls back the (nicer, cleaner, not always working)
fix and replaces it with the (less nice, less clean, more often
working) check that was there before.

Tested with a local build against buildbarn.
@shs96c
Copy link
Contributor Author

shs96c commented Jan 5, 2023

One optimisation we might be able to do is to check for the presence of /proc/$PPID/exe as a quick test to see whether the parent process is still running. If that's what we want, I suggest doing so in a follow-up PR.

@shs96c
Copy link
Contributor Author

shs96c commented Jan 5, 2023

Not being able to run tests remotely is preventing my organisation from being able to update to Bazel 6, so once this lands, could it please be cherry-picked into the next release?

@fmeum
Copy link
Collaborator

fmeum commented Jan 5, 2023

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jan 5, 2023
@larsrc-google larsrc-google added the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jan 9, 2023
@copybara-service copybara-service bot closed this in f090433 Jan 9, 2023
@sgowroji sgowroji added team-Local-Exec Issues and PRs for the Execution (Local) team and removed awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally labels Jan 9, 2023
@meteorcloudy
Copy link
Member

@bazel-io fork 6.1.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jan 13, 2023
@ShreeM01 ShreeM01 added potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone and removed potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone labels Jan 13, 2023
ShreeM01 added a commit that referenced this pull request Jan 25, 2023
When the parent process is run as a different user to the test itself (as can happen in some remote build environments, such as buildbarn) using `kill -0 $PPID` fails, causing the test to be prematurely killed.

This PR rolls back the (nicer, cleaner, not always working) fix and replaces it with the (less nice, less clean, more often working) check that was there before.

Tested with a local build against buildbarn.

Closes #17147.

PiperOrigin-RevId: 500691976
Change-Id: Ife55db5c1ed88d40ba502257ce9b679b04eeb179

Co-authored-by: Simon Mavi Stewart <simon_stewart@apple.com>
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
When the parent process is run as a different user to the test itself (as can happen in some remote build environments, such as buildbarn) using `kill -0 $PPID` fails, causing the test to be prematurely killed.

This PR rolls back the (nicer, cleaner, not always working) fix and replaces it with the (less nice, less clean, more often working) check that was there before.

Tested with a local build against buildbarn.

Closes #17147.

PiperOrigin-RevId: 500691976
Change-Id: Ife55db5c1ed88d40ba502257ce9b679b04eeb179
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Local-Exec Issues and PRs for the Execution (Local) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants