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.

Sign up for GitHub

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

Jump to bottom

Fix Bash runfiles_current_repository with RBE #16946

Closed
wants to merge 1 commit into from
Closed

Fix Bash runfiles_current_repository with RBE #16946

wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Dec 7, 2022 •
edited
Loading

During remote execution, absolute paths may not contain execroot segments.

Work towards #16933

@fmeum
Fix Bash runfiles_current_repository with RBE
33a0d5a 
During remote execution, absolute paths may not contain `execroot` segments.
@fmeum
Copy link
Collaborator Author

fmeum commented Dec 7, 2022

@Wyverald Could you review?

FYI @meteorcloudy

@fmeum fmeum marked this pull request as ready for review December 7, 2022 11:23
@fmeum fmeum mentioned this pull request Dec 7, 2022
Closed
@fmeum
Copy link
Collaborator Author

fmeum commented Dec 7, 2022

@Wyverald This doesn't include a test as I don't know how to set up RBE in bazel_module_test or how to set up Bzlmod in remote_integration_test. Do you think I should spend more time on getting either to work?

@sgowroji sgowroji added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Dec 7, 2022
Wyverald
Wyverald approved these changes Dec 7, 2022
View reviewed changes
Copy link
Member

@Wyverald Wyverald left a comment

Choose a reason for hiding this comment

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

LGTM -- this seems fine to me given the timeframe; we'll keep an eye on the downstream pipeline.

@Wyverald Wyverald added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Dec 7, 2022
@copybara-service copybara-service bot closed this in 497b7e4 Dec 8, 2022
@fmeum fmeum deleted the patch-6 branch December 8, 2022 16:06
@fmeum
Copy link
Collaborator Author

fmeum commented Dec 8, 2022

@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 Dec 8, 2022
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Dec 8, 2022
@Wyverald
Copy link
Member

Wyverald commented Dec 8, 2022

@bazel-io fork 6.0.0

@bazel-io bazel-io mentioned this pull request Dec 8, 2022
Closed
@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 Dec 8, 2022
Wyverald pushed a commit that referenced this pull request Dec 8, 2022
@fmeum @Wyverald
Fix Bash runfiles_current_repository with RBE
fb74cce 
During remote execution, absolute paths may not contain `execroot` segments.

Work towards #16933

Closes #16946.

PiperOrigin-RevId: 493876135
Change-Id: I5ceb8c6edbe0e58aae2066efe1230e622eabf9bf
@Wyverald Wyverald mentioned this pull request Dec 8, 2022
Merged
Wyverald added a commit that referenced this pull request Dec 8, 2022
@Wyverald @fmeum
Fix Bash runfiles_current_repository with RBE (#16963)
b95f85f 
During remote execution, absolute paths may not contain `execroot` segments.

Work towards #16933

Closes #16946.

PiperOrigin-RevId: 493876135
Change-Id: I5ceb8c6edbe0e58aae2066efe1230e622eabf9bf

Co-authored-by: Fabian Meumertzheim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wyverald Wyverald Wyverald approved these changes

Assignees
No one assigned
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fmeum @Wyverald @bazel-io @sgowroji