Use long executable path instead of argv[0] in all launchers#16916
Closed
fmeum wants to merge 3 commits intobazelbuild:masterfrom
Closed
Use long executable path instead of argv[0] in all launchers#16916fmeum wants to merge 3 commits intobazelbuild:masterfrom
argv[0] in all launchers#16916fmeum wants to merge 3 commits intobazelbuild:masterfrom
Conversation
`argv[0]` can differ from the path of the launcher executable and can contain 8.3 style filenames, which need to be resolved to long paths before path manipulation (e.g. appending ".runfiles") can succeed. The Python launcher handled this correctly, but other launchers didn't use the long executable path consistently and thus spuriously failed when Bazel emitted an 8.3 path.
7f80ff5 to
70e60d9
Compare
Collaborator
Author
|
@meteorcloudy Could you review this bug fix? I hit in a real world scenario with a Java tool that ended up falling back to the non-existent runfiles directory as it couldn't find the manifest. The error caused by that is indeed very confusing. I will flag this for cherry-picking into some Bazel 6 release. |
Collaborator
Author
|
@bazel-io flag |
meteorcloudy
requested changes
Dec 5, 2022
Member
meteorcloudy
left a comment
There was a problem hiding this comment.
Thanks a lot! Maybe we should add a simple test case in https://github.com/bazelbuild/bazel/blob/master/src/test/py/bazel/launcher_test.py#L615?
Member
|
@fmeum Since this is not a new regression in Bazel 6.0, I prefer to cherry pick it into 6.1 to not delay the release of 6.0 (tomorrow), WDYT? |
Collaborator
Author
|
Sounds good! |
Collaborator
Author
|
@meteorcloudy I added tests. In fact, I added one for invalid |
Member
|
Ah, thanks! That explains why we didn't catch this bug before. |
meteorcloudy
approved these changes
Dec 5, 2022
meteorcloudy
approved these changes
Dec 5, 2022
Member
|
@bazel-io fork 6.1.0 |
ShreeM01
added a commit
that referenced
this pull request
Jan 20, 2023
`argv[0]` can differ from the path of the launcher executable. The latter can contain 8.3 style filenames, which need to be resolved to long paths before path manipulation (e.g. appending ".runfiles") can succeed. The Python launcher handled this correctly, but other launchers didn't use the long executable path consistently. Closes #16916. PiperOrigin-RevId: 493615543 Change-Id: Ic8161890181c0110ecdf6893b9835e6f99d01097 Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
argv[0]can differ from the path of the launcher executable. The latter can contain 8.3 style filenames, which need to be resolved to long paths before path manipulation (e.g. appending ".runfiles") can succeed.The Python launcher handled this correctly, but other launchers didn't use the long executable path consistently.