Skip to content

Fix Bash runfiles calling repository lookup for directly run scripts#19810

Closed
fmeum wants to merge 2 commits intobazelbuild:masterfrom
fmeum:runfiles_lib_direct_run
Closed

Fix Bash runfiles calling repository lookup for directly run scripts#19810
fmeum wants to merge 2 commits intobazelbuild:masterfrom
fmeum:runfiles_lib_direct_run

Conversation

@fmeum
Copy link
Copy Markdown
Collaborator

@fmeum fmeum commented Oct 13, 2023

When an sh_binary is run directly from bazel-bin, its path won't be contained in the manifest and will not match a bazel-out regex. Instead, make the path absolute and then match after the bazel-bin to extract the binaries repository name.

Also improve the error message when runfiles_current_repository can find neither the runfiles manifest nor the runfiles directory.

Work towards fixing #19796 (comment)

@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Oct 13, 2023
@fmeum fmeum force-pushed the runfiles_lib_direct_run branch from 874fbbe to 5e1423c Compare October 13, 2023 17:38
When an `sh_binary` is run directly from `bazel-bin`, its path won't be
contained in the manifest and will not match a `bazel-out` regex.
Instead, make the path absolute and then match after the `bazel-bin` to
extract the binaries repository name.
@fmeum fmeum force-pushed the runfiles_lib_direct_run branch from 5e1423c to 5ce7217 Compare October 13, 2023 18:03
@meteorcloudy meteorcloudy 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 Oct 13, 2023
@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Oct 13, 2023

@meteorcloudy This doesn't fix the full problem. When invoking the script directly, it also fails to find RUNFILES_DIR and RUNFILES_MANIFEST_FILE and I still don't know why. But it does fix an actual problem that can be verified in tests.

@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Oct 13, 2023

@meteorcloudy Looks like an existing test reproduces the problem. I will look into fixing it.

@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Oct 14, 2023

@meteorcloudy Failure is fixed, I simply forgot the .exe suffix on Windows

@sgowroji sgowroji added the team-Configurability platforms, toolchains, cquery, select(), config transitions label Oct 16, 2023
Copy link
Copy Markdown
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

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

I see, thanks!

@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Oct 17, 2023
@fmeum fmeum deleted the runfiles_lib_direct_run branch October 17, 2023 21:55
@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Oct 17, 2023

@bazel-io flag

@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Oct 17, 2023

(for 7.0.0)

@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 Oct 17, 2023
@iancha1992
Copy link
Copy Markdown
Member

@bazel-io fork 7.0.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 Oct 17, 2023
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Oct 17, 2023
When an `sh_binary` is run directly from `bazel-bin`, its path won't be contained in the manifest and will not match a `bazel-out` regex. Instead, make the path absolute and then match after the `bazel-bin` to extract the binaries repository name.

Also improve the error message when `runfiles_current_repository` can find neither the runfiles manifest nor the runfiles directory.

Work towards fixing bazelbuild#19796 (comment)

Closes bazelbuild#19810.

PiperOrigin-RevId: 574208158
Change-Id: I45d6b503e8f34e13177d57ca64c202640306cfb8
meteorcloudy pushed a commit that referenced this pull request Oct 18, 2023
…scripts (#19857)

When an `sh_binary` is run directly from `bazel-bin`, its path won't be
contained in the manifest and will not match a `bazel-out` regex.
Instead, make the path absolute and then match after the `bazel-bin` to
extract the binaries repository name.

Also improve the error message when `runfiles_current_repository` can
find neither the runfiles manifest nor the runfiles directory.

Work towards fixing
#19796 (comment)

Closes #19810.

Commit
58ce112

PiperOrigin-RevId: 574208158
Change-Id: I45d6b503e8f34e13177d57ca64c202640306cfb8

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Configurability platforms, toolchains, cquery, select(), config transitions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants