-
-
Notifications
You must be signed in to change notification settings - Fork 98
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
[FR]: tar rule should support runfiles with symlinks #603
Comments
I think it's okay for this to slip to 2.1 as it's not a breaking change. |
I'm on the fence about shipping 2.0 final so long as you can't replace rules_pkg in our js_image_layer example. @thesayyn WDYT? |
we can land that after 2.0. it's non-breaking change. |
IIUC, We'll probably want to wait for bazelbuild/bazel#19944 to be available in Bazel 7.0 and direct users to upgrade so that we can handle the cases correctly under bzlmod. |
Hi! Is there a plan for implementing this? In the absence of this feature, is the recommended way to use a custom binary like |
If you don't care about symlinks tar rule should work for you already, js_image_layer is special as symlinks needs to be preserved in the resulting layers. |
IIUC, the |
It sounds like https://github.com/aspect-build/bazel-lib/blob/main/lib/private/tar.bzl#L227 is missing a bit for symlinks |
I quickly hacked this bookingcom@HEAD, I'm going to test it in our use case, what you think? Does it look like something we could merge? |
Ooh thanks! @thesayyn could you TAL? |
FYI I tested it for our case and made a few improvements:
|
Ah unfortunately, this does not work if something is reported as symlink but not added to the This is why we need a js_image_layer rule that has its own tar generator. https://github.com/aspect-build/rules_js/blob/98081d0fb66b3619f5a191ea29765012849d13f7/js/private/image/index.ts#L84-L104 |
awk has a |
@thesayyn I see, so we would need something like a genrule or some kind of action that can look into the generated manifest correct? I need to verify if the runfiles_manifest contains our symlinks at all. Maybe we need a small program that can do the thing, is there a policy against that kind of thing in this repo? I'm thinking a small go or python program that can compute it |
We have a bunch of go programs in this repo already. That's an option if there isn't a simpler solution. |
I have a WIP here; #967 which i will try to look into more coming days. awk has a |
@thesayyn, I continued working on #967 because of the related issue in bazelbuild/rules_python#2489. However, I’ve run into a problem: the genrule doesn’t have access to the binary’s runfiles, so using system(readlink) isn’t feasible. Do you have any suggestions or alternative approaches for handling this? |
Hmm, not really, we have to include binary in srcs to be able to resolve symlinks. |
I ended with this implementation, but I am not sure if this is a reasonable direction to take. This could be potentially implemented using If this approach makes sense, I can continue to work on it. |
What is the current behavior?
We had to do some stuff to make js_image_layer work with rules_pkg.
Describe the feature
Should have feature parity with those, and be able to make clean bazelbuild/examples directories showing container builds for JS, Python, Java, etc with no userland starlark code.
The text was updated successfully, but these errors were encountered: