Conditionally preserve file permissions when archiving through pkg_tar (#951)
* preserve file permission through an option
* fix parsing and handling of new preserve_mode argument
* restore version
* add tests
* use an existing test file, as git does not preserve file perms
* delete test file
* cover windows testing
* record test subject file perms
* use pythonic elif
Co-authored-by: Chuck Grindel <chuck.grindel@gmail.com>
* buildifier
* trim trailing whitespaces
---------
Co-authored-by: rnascimento <rnascimento@salesforce.com>
Co-authored-by: Chuck Grindel <chuck.grindel@gmail.com>
diff --git a/pkg/private/tar/build_tar.py b/pkg/private/tar/build_tar.py
index 8976341..be74bc9 100644
--- a/pkg/private/tar/build_tar.py
+++ b/pkg/private/tar/build_tar.py
@@ -15,6 +15,7 @@
import argparse
import os
+import stat
import tarfile
import tempfile
@@ -43,7 +44,7 @@
pass
def __init__(self, output, directory, compression, compressor, create_parents,
- allow_dups_from_deps, default_mtime, compression_level):
+ allow_dups_from_deps, default_mtime, compression_level, preserve_mode):
# Directory prefix on all output paths
d = directory.strip('/')
self.directory = (d + '/') if d else None
@@ -54,6 +55,7 @@
self.create_parents = create_parents
self.allow_dups_from_deps = allow_dups_from_deps
self.compression_level = compression_level
+ self.preserve_mode = preserve_mode
def __enter__(self):
self.tarfile = tar_writer.TarFileWriter(
@@ -101,9 +103,13 @@
copied to `self.directory/destfile` in the layer.
"""
dest = self.normalize_path(destfile)
- # If mode is unspecified, derive the mode from the file's mode.
- if mode is None:
- mode = 0o755 if os.access(f, os.X_OK) else 0o644
+ # If preserve_mode is enabled, set mode by extracting the permission bits
+ # from the file's mode attribute. Note: the mode argument is ignored.
+ # Otherwise; if mode is unspecified, derive the mode from the file's mode.
+ if self.preserve_mode is True:
+ mode = stat.S_IMODE(os.stat(f).st_mode)
+ elif mode is None:
+ mode = 0o755 if os.access(f, os.X_OK) else 0o644
if ids is None:
ids = (0, 0)
if names is None:
@@ -403,6 +409,10 @@
action='store_true',
help='')
parser.add_argument(
+ '--preserve_mode', default='False',
+ action='store_true',
+ help='Preserve original file permissions in the archive. Mode argument is ignored.')
+ parser.add_argument(
'--compression_level', default=-1,
help='Specify the numeric compress level in gzip mode; may be 0-9 or -1 (default to 6).')
options = parser.parse_args()
@@ -461,7 +471,8 @@
default_mtime=default_mtime,
create_parents=options.create_parents,
allow_dups_from_deps=options.allow_dups_from_deps,
- compression_level = compression_level) as output:
+ compression_level = compression_level,
+ preserve_mode = options.preserve_mode) as output:
def file_attributes(filename):
if filename.startswith('/'):
diff --git a/pkg/private/tar/tar.bzl b/pkg/private/tar/tar.bzl
index 29539a6..0150192 100644
--- a/pkg/private/tar/tar.bzl
+++ b/pkg/private/tar/tar.bzl
@@ -181,6 +181,9 @@
if ctx.attr.allow_duplicates_from_deps:
args.add("--allow_dups_from_deps")
+ if ctx.attr.preserve_mode:
+ args.add("--preserve_mode")
+
inputs = depset(
direct = ctx.files.deps + files,
transitive = mapping_context.file_deps,
@@ -294,6 +297,10 @@
builds were accidentally doing it. Never explicitly set this to true for new code.
""",
),
+ "preserve_mode": attr.bool(
+ default = False,
+ doc = """If true, will add file to archive with preserved file permissions.""",
+ ),
"stamp": attr.int(
doc = """Enable file time stamping. Possible values:
<li>stamp = 1: Use the time of the build as the modification time of each file in the archive.
diff --git a/tests/tar/BUILD b/tests/tar/BUILD
index 5529b74..3164162 100644
--- a/tests/tar/BUILD
+++ b/tests/tar/BUILD
@@ -474,6 +474,8 @@
":test-tar-files_dict.tar",
":test-tar-long-filename",
":test-tar-mtime.tar",
+ ":test-tar-preserve_mode-False.tar",
+ ":test-tar-preserve_mode-True.tar",
":test-tar-repackaging-long-filename.tar",
":test-tar-strip_prefix-dot.tar",
":test-tar-strip_prefix-empty.tar",
@@ -801,3 +803,14 @@
6,
9,
]]
+
+[pkg_tar(
+ name = "test-tar-preserve_mode-%s" % state,
+ srcs = [
+ "//tests:testdata/hello.txt", # rw- r-- r--
+ ],
+ preserve_mode = state,
+) for state in [
+ True,
+ False,
+]]
diff --git a/tests/tar/pkg_tar_test.py b/tests/tar/pkg_tar_test.py
index 835b9e1..245bb79 100644
--- a/tests/tar/pkg_tar_test.py
+++ b/tests/tar/pkg_tar_test.py
@@ -306,5 +306,23 @@
file_size = os.stat(file_path).st_size
self.assertEqual(file_size, expected_size, 'size error for ' + file_name)
+ def test_preserve_mode(self):
+ if os.name == 'nt':
+ expected_mode = [
+ ('test-tar-preserve_mode-False.tar', "0o555"), # chmod 555 = r-x r-x r-x
+ ('test-tar-preserve_mode-True.tar', "0o666"), # chmod 666 = rw- rw- rw-
+ ]
+ else:
+ expected_mode = [
+ ('test-tar-preserve_mode-False.tar', "0o555"), # chmod 555 = r-x r-x r-x
+ ('test-tar-preserve_mode-True.tar', "0o644"), # chmod 644 = rw- r-- r--
+ ]
+ for file_name, expected_mode in expected_mode:
+ file_path = runfiles.Create().Rlocation('rules_pkg/tests/tar/' + file_name)
+ with tarfile.open(file_path, 'r') as f:
+ for member in f.getmembers():
+ self.assertEqual(member.name, "hello.txt", "unexpected file name for " + file_name)
+ self.assertEqual(member.mode, int(expected_mode, 0), 'file mode not preserved for ' + file_name)
+
if __name__ == '__main__':
unittest.main()