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

Test and fix root symlink edge case in runfiles library #17317

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Jan 25, 2023

Due to incomplete test coverage for the case of a root symlink named like a Bazel module, the Bash runfiles libraries behaved incorrectly in this case. This commit fixes that and adds proper test coverage for all libraries by adding repo mappings that conflict with the existing rlocation testcase for config.json.

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 25, 2023

@Wyverald

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 25, 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 25, 2023
@sgowroji sgowroji added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. awaiting-review PR is awaiting review from an assigned reviewer labels Jan 25, 2023
@ShreeM01
Copy link
Contributor

@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 26, 2023
Due to incomplete test coverage for the case of a root symlink named
like a Bazel module, the Bash runfiles libraries behaved incorrectly
in this case. This commit fixes that and adds proper test coverage for
all libraries.
@fmeum fmeum force-pushed the test-runfiles-edge-case branch from a5f8035 to c07e614 Compare January 30, 2023 15:53
@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 Jan 30, 2023
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jan 31, 2023
@fmeum fmeum deleted the test-runfiles-edge-case branch January 31, 2023 12:10
fmeum added a commit to fmeum/bazel that referenced this pull request Jan 31, 2023
Due to incomplete test coverage for the case of a root symlink named like a Bazel module, the Bash runfiles libraries behaved incorrectly in this case. This commit fixes that and adds proper test coverage for all libraries by adding repo mappings that conflict with the existing rlocation testcase for `config.json`.

Closes bazelbuild#17317.

PiperOrigin-RevId: 505873412
Change-Id: I72019d329b72cab9cf893deb714da8889d371617
ShreeM01 pushed a commit that referenced this pull request Jan 31, 2023
Due to incomplete test coverage for the case of a root symlink named like a Bazel module, the Bash runfiles libraries behaved incorrectly in this case. This commit fixes that and adds proper test coverage for all libraries by adding repo mappings that conflict with the existing rlocation testcase for `config.json`.

Closes #17317.

PiperOrigin-RevId: 505873412
Change-Id: I72019d329b72cab9cf893deb714da8889d371617
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
Due to incomplete test coverage for the case of a root symlink named like a Bazel module, the Bash runfiles libraries behaved incorrectly in this case. This commit fixes that and adds proper test coverage for all libraries by adding repo mappings that conflict with the existing rlocation testcase for `config.json`.

Closes #17317.

PiperOrigin-RevId: 505873412
Change-Id: I72019d329b72cab9cf893deb714da8889d371617
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants