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(builtin): fix a bug where the launcher produces incorrect runfiles path on windows #3562

Merged
merged 1 commit into from
Sep 28, 2022

Conversation

kormide
Copy link
Collaborator

@kormide kormide commented Sep 27, 2022

No description provided.

@kormide kormide force-pushed the launcher-path-normalize branch 3 times, most recently from 31a3ec0 to 44673f7 Compare September 27, 2022 23:34
@kormide
Copy link
Collaborator Author

kormide commented Sep 27, 2022

This is causing more issues. I think I'll need a pair.

Copy link
Collaborator

@gregmagolan gregmagolan left a comment

Choose a reason for hiding this comment

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

🌮

@gregmagolan
Copy link
Collaborator

lgtm; looks like you just have to update an env test

@kormide
Copy link
Collaborator Author

kormide commented Sep 28, 2022

It's not setting BAZEL_PATCH_ROOTS correctly, which I don't really understand,

@gregmagolan
Copy link
Collaborator

gregmagolan commented Sep 28, 2022

It's not setting BAZEL_PATCH_ROOTS correctly, which I don't really understand,

TL;DR is that RUNFILES_DIR is now equal to RUNFILES on Windows with the changes so the exception is no longer needed in that test. I'll fix the test to remove the windows exception

@gregmagolan gregmagolan changed the title fix(builtin): fix a bug where the launcher produces incorrect runfiles fix(builtin): fix a bug where the launcher produces incorrect runfiles path on windows Sep 28, 2022
@gregmagolan gregmagolan merged commit b02128b into stable Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants