Auto-detecting executable bit in `pkg_tar` can't be used in conjunction with `pkg_files`

[flode]

The build_tar.py has logic to auto-detect the executable bit of the source files and also apply that in the final tar file:

        # If mode is unspecified, derive the mode from the file's mode.
        if mode is None:
          f_mode = 0o755 if os.access(content_path, os.X_OK) else 0o644
        else:
          f_mode = mode

https://github.com/bazelbuild/rules_pkg/blob/main/pkg/private/tar/build_tar.py#L283

This logic cannot be reached however when using pkg_tar in conjunction with pkg_files, as the _pkg_files_impl doesn't allow None to pass through, it overwrites it with a 0644 default.

    # The least surprising default mode is that of a normal file (0644)
    out_attributes.setdefault("mode", "0644")

https://github.com/bazelbuild/rules_pkg/blob/main/pkg/mappings.bzl#L256

Is that default there on purpose? There is another default for mode in pkg_tar of 0555. So the default in pkg_files make the default of pkg_tar not usable.

The initial problem could be solved by simply removing that default in _pkg_files_impl, then this code would be using the auto-detection of the executable bit:

    pkg_files(
        name ="files",
        srcs = [src],
    )
   
    pkg_tar(
        name = package,
        srcs = ["files"],
        mode = None,
    )

For the case where neither pkg_files nor pkg_tar specify a mode, the files are created with the default mode of tar (555).

Would that make sense? I didn't look at the other packaging rules yet how they would be impacted.

[aiuto]

We have a long-standing philosophy in Bazel that decoupled file paths from filesystem attributes (the rwx bits). That is a reasonable thing in the context of remote build and immutable artifact stores. It does, however, always seem strange when you build on a single machine. The end result is that rules_pkg needs to have an explicit mode for everything rather than look at the file system.

I don't want to remove the default in pkg_files, because that will surely break existing users. Nor can pkg_files look at the file system, because it is a Bazel rule, and that is not available. I'm not going to have time to think about a big design until after BazelCon, but my first thoughts are

• being able to say pkg_files(name=..., srcs=..., use_filesystem_attributes=True) would be useful.
• but we also need a command line flag which enable the use_filesystem_attributes capability

The flag is to limit the possibility of a target relying on native filesystem attributes failing when the source moves to a different environment, like a read-only network store.