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.

All directories, files and symlinks created by an action are owned by the user. Files provided as input are readonly and are therefore owned by root so that actions don't try to make them writable to edit them. Because directories are always mutable and symlink targets can only be modified by replacing them, directories and symlinks provided as inputs can safely be owned by the user.

This commit changes the owner of user provided symlinks from root to the user. This solves the use case of hardlinking symlinks, which requires the user to be the owner of the symlink.

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 3s.

[EdSchouten]

Thanks for working on this!

[moroten]

I'm trying to get going on this again. Have I got it correct that:

• Owned by root
  • Input file
• Owned by user
  • Input directory
  • Input symlink
  • Action created file
  • Action created directory
  • Action created symlink

Is the rational that files are read only but directories and symlinks cannot be modified anyway, they are simply replaced?

[EdSchouten]

Yes, exactly. Directories are always mutable, and symlink targets can only be modified by replacing them.

[EdSchouten]

Thanks for working on this!

[EdSchouten]

This is starting to look great, Fredrik. Only one tiny request for change, and this should be good to land.

[EdSchouten]

LGTM! Will merge once CI is happy.

[BoleynSu]

@moroten @EdSchouten This is nil in the case of Native.

[EdSchouten]

Thanks! Fixed in 31166ae.

[BoleynSu]

Thanks for the quick fix!