Set ownership on symlinks created by build actions during their execution

[moroten]

In commit 410ea5c, Set ownership on files created by build actions during their execution, files got correct ownership associated with them. This commit adds the same feature to symlinks.

One use case is to allow hardlinking symlinks, which requires the owner of the symlink to be correct.

The following test was performed with Bazel:

--- rules.bzl ---
def _impl(ctx):
    file = ctx.actions.declare_file("file")
    symlink = ctx.actions.declare_symlink("symlink")
    args = ctx.actions.args()
    args.add_all([file, symlink])
    ctx.actions.run_shell(
        outputs = [file, symlink],
        command = """
exec 2>&1
set +ex
touch $1
ln -s file $2
ln $2 $2.hardlink
ls -la $(dirname $1)
""",
        arguments = [args],
    )
    return [
        DefaultInfo(files = depset([file, symlink])),
        OutputGroupInfo(
            file = depset([file]),
        ),
    ]

hardlink_a_symlink_rule = rule(
    implementation = _impl,
)

--- BUILD.bazel ---
load(":rules.bzl", "hardlink_a_symlink_rule")

hardlink_a_symlink_rule(name = "hardlink_a_symlink")

filegroup(
    name = "hardlink_a_symlink_file",
    srcs = [":hardlink_a_symlink"],
    output_group = "file",
)
genrule(
    name = "hardlink_input_symlink",
    srcs = [":hardlink_a_symlink", ":hardlink_a_symlink_file"],
    outs = ["symlink.another_hardlink"],
    cmd = """
exec 2>&1
set +ex
cd $$(dirname $(location :hardlink_a_symlink_file))
ln symlink symlink.another_hardlink
ls -la
""",
)
    
--- Output ---
$ bazel build :hardlink_input_symlink
INFO: From Action file:
total 0
drwxrwxrwx    1 fredrik fredrik 0 Feb  5 21:12 .
drwxrwxrwx    1 fredrik fredrik 0 Feb  5 21:12 ..
-rw-rw-rw-    1 fredrik fredrik 0 Feb  5 21:12 file
lrwxrwxrwx 9999 fredrik fredrik 4 Jan  1  2000 symlink -> file
lrwxrwxrwx 9999 fredrik fredrik 4 Jan  1  2000 symlink.hardlink -> file
INFO: From Executing genrule //:hardlink_input_symlink:
total 0
drwxrwxrwx    1 fredrik fredrik 0 Feb  5 21:12 .
drwxrwxrwx    1 fredrik fredrik 0 Feb  5 21:12 ..
-r-xr-xr-x 9999 root    root    0 Jan  1  2000 file
lrwxrwxrwx 9999 fredrik fredrik 4 Jan  1  2000 symlink -> file
lrwxrwxrwx 9999 fredrik fredrik 4 Jan  1  2000 symlink.another_hardlink -> file

References:
https://sourceforge.net/p/fuse/mailman/message/35004606/
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=800179c9b8a1e796e441674776d11cd4c05d61d7
torvalds/linux@800179c

[moroten]

Shouldn't files provided as inputs have the ownership set in the runner configuration? I looks like only files created by the action has the ownership of the user.

[aspect-workflows]

Test

All tests were cache hits

19 tests (100.0%) were fully cached saving 2s.

[EdSchouten]

Thanks for working on this!

[EdSchouten]

If we are going to give the top level directory a SymlinkFactory that differs from the one that's used per build directory, what are your thoughts on giving it an instance that always returns an error? There should be no need for anyone to ever place symlinks in there. This does require changing SymlinkFactory.LookupSymlink() to return an error.

[EdSchouten]

Given that defaultAttributesSetter and handleAllocator are both already provided to NewVirtualBuildDirectory(), I think it's cleaner if we move construction of symlinkFactory into NewVirtualBuildDirectory(). This is also a bit more consistent with the creation of FileAllocator, which already lives inside virtualBuildDirectory.

[EdSchouten]

While there, can we rename this to baseSymlinkFactory? That's more consistent with the constructor function. I guess I forgot to rename this when I added the SymlinkFactory interface. It used to be the case that symlinks were constructed directly, without using a factory type.