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

Set PATH_PREFIX=./ instead of failing to bazel run a program with a perl_binary data dep #65

Conversation

lalten
Copy link
Contributor

@lalten lalten commented Aug 18, 2024

I had the issue that the binary wrapper fails when it is a data dependency of some other program which is run under bazel run (it does work under bazel test!)
The issue is that during bazel run RUNFILES_DIR may not be set and the MANIFEST file may not exist at ../.. even if you did not cd somewhere else.
The correct thing to do for the wrapper in this case is to accept the interpreter short path and main short path as-is from the starlark side. We can just set PATH_PREFIX=./ for the same effect.

I added a test that is red without the binary_wrapper change and becomes green when the change is applied. The trick here is to use env -i to remove the test envs from the sh_test so it behaves more like a bazel run and not bazel test.

Note that I don't think the binary_wrapper should explicitly fail anyways. Rather it should just let things play out. Other things will fail in any case if paths are not set up correctly. And if things won't fail -- well then the wrapper shouldn't make them fail either :)

@lalten lalten requested a review from skeletonkey as a code owner August 18, 2024 12:37
@lalten lalten changed the title Set PATH_PREFIX=./ instead of failing Set PATH_PREFIX=./ instead of failing to bazel run a program with a perl_binary data dep Aug 18, 2024
@lalten lalten force-pushed the fix-path-prefix-for-bazel-run-data-dep branch from 0b29c4d to dd80f71 Compare August 18, 2024 12:39
lalten added a commit to lalten/bazel-central-registry that referenced this pull request Aug 18, 2024
Wyverald pushed a commit to bazelbuild/bazel-central-registry that referenced this pull request Aug 19, 2024
This is latest rules_perl plus
* bazel-contrib/rules_perl#62
* bazel-contrib/rules_perl#65
* bazel-contrib/rules_perl#66

Obviously it would be nice to get a review / merge this upstream. If
that still happens we can add a new (0.2.4 ?) release without patches in
the BCR.

The patch enables external rules_perl Bazel repos to work under Bzlmod.
This is needed for https://github.com/lalten/rules_cpan to work out of
the box

needs the `unstable source url` label
@skeletonkey skeletonkey merged commit 427babc into bazel-contrib:main Sep 6, 2024
1 of 2 checks passed
@lalten lalten deleted the fix-path-prefix-for-bazel-run-data-dep branch September 6, 2024 05:20
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.

2 participants