Uh oh!
There was an error while loading. Please reload this page.
feat: support raw symlinks from declare_symlink() - #930
Conversation
cgrindel
left a comment
There was a problem hiding this comment.
Can we add a test for this?
rickeylev
commented
Mar 16, 2025
Ok, it's been awhile since I could get time to come back to this! I've added a basic test. PTAL. |
rickeylev
commented
Mar 19, 2025
OK, PTAL. I fixed the buildifier presubmit failure. The other failure looks unrelated -- I get the same error at clean head. It's specific to the rolling version of bazel and something about workspace support. |
rickeylev
commented
Mar 19, 2025
Actually, I partially take that back -- The LTS windows test is failing. Some quick searches indicates windows symlink support should work fine with the values the test is using (relative symlinks that point nowhere). Diagnosing and fixing this will be a bit hard, but I'll see what I can do. If it comes to it, can we simply mark this test to skip windows? |
meteorcloudy
commented
Mar 19, 2025
https://bazel.build/configure/windows#symlink |
rickeylev
commented
Mar 19, 2025
I tried adding So far, all I can tell is that, when tar building occurs, and python calls I can't tell which of these cases is happening:
I won't have direct access to a windows machine until this evening, so I'll have to wait until then to check some of these ideas. But overall, this isn't sounding promising. I don't think we can work around (1) or (2). A work around for (3) and (4) might be possible by using e.g. ctypes and dropping into lower-level APIs. |
rickeylev
commented
Mar 20, 2025
After testing on my personal windows machine, I'm thinking its (1: Bazel writing an absolute path). I can use |
Aha, here we go. known issue in Bazel specific to windows: bazelbuild/bazel#14224 |
meteorcloudy
commented
Mar 20, 2025
OK, I guess let's disable the test on Windows for now |
rickeylev
commented
Apr 1, 2025
Tests fixed. PTAL |
rickeylev
commented
Apr 10, 2025
bump -- this has been sitting for awhile; is there an ETA on when someone can review? |
cgrindel
commented
Apr 10, 2025
In the future, if you hit the "Re-request review" button, that will let the reviewer who has left comments look at the PR again. Looking now |
rickeylev
commented
Apr 10, 2025
Oh, i had not noticed that thingy before. Done, for at least my own practice :). Thanks! |
Using the
ctx.actions.declare_symlink()API, rules can create explicit symlinks to arbitrarypaths, which Bazel will keep as symlinks. The packaging logic treats these
as regular files, so tries to read through them and store the content they point to. This
is incorrect for two reasons: they are supposed to be left as symlinks and may be dangling
symlinks from the perspective of packaging.
To fix, introduce a new type of file entry:
raw_symlink. These entries mean thesrcisa symlink whose stored path should be stored into the destination verbatim, also as a symlink.
Such artifacts are identified using
File.is_symlink, a Bazel 8+ API.Work towards #929