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

Fix Bash rlocation failure with stricter Bash options #16755

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Nov 12, 2022

When run in the context of the bazel-skylib unit test setup, the new repo mapping logic in the Bash runfiles library failed due to a non-matching grep in a pipe. Fix this by using a wrapper around grep throughout that does not exit with exit code 1 if there is no match.

Fixes bazelbuild/bazel-skylib#411 (comment)

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 12, 2022

@c-parsons as you set up the unit test framework in bazel-skylib: I haven't been able to reproduce the failure of $(grep ... | ...) that I see over in bazel-skylib's unit tests when the grep doesn't match anything here in runfiles_test.sh. Even with set -o errexit, set -e, set -o pipefail` I don't get this behavior. Do you know how I could enable it so that this fix has test coverage?

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 12, 2022

@shs96c Could you verify that this fixes the issue you are seeing?

When run in the context of the bazel-skylib unit test setup, the new
repo mapping logic in the Bash runfiles library failed due to a
non-matching grep in a pipe. Fix this by using a wrapper around grep
throughout that does not exit with exit code 1 if there is no match.
@shs96c
Copy link
Contributor

shs96c commented Nov 13, 2022

That does appear to fix the problem.

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 15, 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 Nov 15, 2022
@Wyverald
Copy link
Member

@bazel-io fork 6.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 Nov 15, 2022
Wyverald pushed a commit that referenced this pull request Nov 15, 2022
When run in the context of the bazel-skylib unit test setup, the new repo mapping logic in the Bash runfiles library failed due to a non-matching grep in a pipe. Fix this by using a wrapper around grep throughout that does not exit with exit code 1 if there is no match.

Fixes bazelbuild/bazel-skylib#411 (comment)

Closes #16755.

PiperOrigin-RevId: 488749744
Change-Id: I087b03d9e95ba27a409c551bdc27d0027919a0fe
ShreeM01 pushed a commit that referenced this pull request Nov 15, 2022
When run in the context of the bazel-skylib unit test setup, the new repo mapping logic in the Bash runfiles library failed due to a non-matching grep in a pipe. Fix this by using a wrapper around grep throughout that does not exit with exit code 1 if there is no match.

Fixes bazelbuild/bazel-skylib#411 (comment)

Closes #16755.

PiperOrigin-RevId: 488749744
Change-Id: I087b03d9e95ba27a409c551bdc27d0027919a0fe

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
@fmeum fmeum deleted the stricter-bash-runfiles branch November 16, 2022 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants