Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/.bazelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Define workspace status command for stamp_mtime attribute in pkg_tar rule
test --workspace_status_command=./tests/workspace_status.sh
19 changes: 18 additions & 1 deletion pkg/archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ def __init__(self,
compression='',
root_directory='.',
default_mtime=None,
preserve_tar_mtimes=True):
preserve_tar_mtimes=True,
workspace_status_file=None):
"""TarFileWriter wraps tarfile.open().

Args:
Expand Down Expand Up @@ -155,6 +156,22 @@ def __init__(self,
self.default_mtime = 0
elif default_mtime == 'portable':
self.default_mtime = PORTABLE_MTIME
elif workspace_status_file is not None:
default_mtime_strip = default_mtime.strip()
if not default_mtime_strip.startswith("{") or not default_mtime_strip.endswith("}"):
raise self.Error('Workspace status file provided, but mtime does not contain a valid key.')
default_mtime_key = default_mtime_strip[1:-1]
key_found = False
with open(workspace_status_file, 'r') as f:
for line in f:
key, value = line.strip().split(' ', 1)
if key == default_mtime_key:
key_found = True
self.default_mtime = int(value)
break
if not key_found:
raise self.Error('Key "{}" was not found in workspace status file. '.format(default_mtime_key) +
'Please check if the volatile workspace status file contains this key.')
else:
self.default_mtime = int(default_mtime)

Expand Down
11 changes: 8 additions & 3 deletions pkg/build_tar.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,21 @@ class DebError(Exception):
pass

def __init__(self, output, directory, compression, root_directory,
default_mtime):
default_mtime, workspace_status_file=None):
self.directory = directory
self.output = output
self.compression = compression
self.root_directory = root_directory
self.default_mtime = default_mtime
self.workspace_status_file = workspace_status_file

def __enter__(self):
self.tarfile = archive.TarFileWriter(
self.output,
self.compression,
self.root_directory,
default_mtime=self.default_mtime)
default_mtime=self.default_mtime,
workspace_status_file=self.workspace_status_file)
return self

def __exit__(self, t, v, traceback):
Expand Down Expand Up @@ -256,6 +258,9 @@ def main():
'path/to/file=root.root.')
parser.add_argument('--root_directory', default='./',
help='Default root directory is named "."')
parser.add_argument('--workspace_status_file',
help='Workspace status file with volatile keys/values '
'for use with "mtime" containing the corresponding key to use.')
options = parser.parse_args()

# Parse modes arguments
Expand Down Expand Up @@ -298,7 +303,7 @@ def main():
# Add objects to the tar file
with TarFile(
options.output, helpers.GetFlagValue(options.directory),
options.compression, options.root_directory, options.mtime) as output:
options.compression, options.root_directory, options.mtime, options.workspace_status_file) as output:

def file_attributes(filename):
if filename.startswith('/'):
Expand Down
16 changes: 16 additions & 0 deletions pkg/docs/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,22 @@ Creates a tar file from a list of inputs.
</p>
</td>
</tr>
<tr>
<td><code>stamp_mtime</code></td>
<td>
<code>String, optional</code>
<p>
Stamp variable from workspace status file.
The key's name must not start with <code>STABLE_</code>
because only "volatile" keys are read.
</p>
<p>
<code>
stamp_mtime = "{STAMP_MTIME}",

@aiuto aiuto Jan 30, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this using a different name than BUILD_TIMESTAMP?
But really, this should be a bool. Either stamp the time found in BUILD_TIMESTAMP or use the portable time.

</code>
</p>
</td>
</tr>
</tbody>
</table>

Expand Down
19 changes: 18 additions & 1 deletion pkg/pkg.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,28 @@ def _pkg_tar_impl(ctx):
"--owner=" + ctx.attr.owner,
"--owner_name=" + ctx.attr.ownername,
]
stamp_inputs = []
if ctx.attr.mtime != _DEFAULT_MTIME:
if ctx.attr.portable_mtime:
fail("You may not set both mtime and portable_mtime")
if ctx.attr.stamp_mtime:
fail("You may not set both mtime and stamp_mtime")
args.append("--mtime=%d" % ctx.attr.mtime)
if ctx.attr.portable_mtime:
if ctx.attr.stamp_mtime:
fail("You may not set both portable_mtime and stamp_mtime")
args.append("--mtime=portable")
if ctx.attr.stamp_mtime:
# Only ctx.version_file for file volatile-status.txt is needed
# because change of key/value should not invalid existing build
stamp_inputs += [ctx.version_file]
stamp_mtime_strip = ctx.attr.stamp_mtime.strip()
if not stamp_mtime_strip.startswith("{") or not stamp_mtime_strip.endswith("}"):
fail("You set stamp_mtime, but this doesn't contain a valid key variable")
Comment on lines +108 to +110

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just use BUILD_STAMP. We don't need the flexibility to let the user set an arbitrary variable out of the volatile status file. This adds complexity that no one asked for.

# Add mtime with stamped mtime variable
args.append("--mtime=%s" % stamp_mtime_strip)
# Add volatile-status.txt file as argument
args.append("--workspace_status_file=%s" % ctx.version_file.path)

# Add runfiles if requested
file_inputs = []
Expand Down Expand Up @@ -162,7 +178,7 @@ def _pkg_tar_impl(ctx):
ctx.actions.run(
mnemonic = "PackageTar",
progress_message = "Writing: %s" % output_file.path,
inputs = file_inputs + ctx.files.deps + files,
inputs = file_inputs + ctx.files.deps + files + stamp_inputs,
executable = ctx.executable.build_tar,
arguments = ["@" + arg_file.path],
outputs = [output_file],
Expand Down Expand Up @@ -322,6 +338,7 @@ pkg_tar_impl = rule(
"modes": attr.string_dict(),
"mtime": attr.int(default = _DEFAULT_MTIME),
"portable_mtime": attr.bool(default = True),
"stamp_mtime": attr.string(),
"owner": attr.string(default = "0.0"),
"ownername": attr.string(default = "."),
"owners": attr.string_dict(),
Expand Down
10 changes: 10 additions & 0 deletions pkg/tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,15 @@ pkg_tar(
portable_mtime = False,
)

pkg_tar(
name = "test-tar-stamp_mtime",
srcs = [
":etc/nsswitch.conf",
],
portable_mtime = False,
stamp_mtime = "{STAMP_MTIME}",
)

py_test(
name = "pkg_tar_test",
size = "medium",
Expand All @@ -411,6 +420,7 @@ py_test(
":test-tar-empty_files.tar",
":test-tar-files_dict.tar",
":test-tar-mtime.tar",
":test-tar-stamp_mtime.tar",
":test-tar-strip_prefix-dot.tar",
":test-tar-strip_prefix-empty.tar",
":test-tar-strip_prefix-etc.tar",
Expand Down
5 changes: 5 additions & 0 deletions pkg/tests/archive_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,11 @@ def testPreserveTarMtimesFalse(self):
for output_file in f.tar:
self.assertEqual(output_file.mtime, 0)

def testStampMtime(self):
with archive.TarFileWriter(self.tempfile, default_mtime="{STAMP_MTIME}",
workspace_status_file="./tests/testdata/test_volatile-status.txt") as f:
self.assertEqual(f.default_mtime, 946684741)

def testAddingDirectoriesForFile(self):
with archive.TarFileWriter(self.tempfile) as f:
f.add_file("d/f")
Expand Down
8 changes: 8 additions & 0 deletions pkg/tests/pkg_tar_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,14 @@ def test_mtime(self):
]
self.assertTarFileContent('test-tar-mtime.tar', content)

def test_stamp_mtime(self):
# Note strange mtime. It is specified in the testdata/workspace_status.sh file.
content = [
{'name': '.', 'mtime': 946684741},
{'name': './nsswitch.conf', 'mtime': 946684741},
]
self.assertTarFileContent('test-tar-stamp_mtime.tar', content)

def test_basic(self):
# Check the set of 'test-tar-basic-*' smoke test.
content = [
Expand Down
1 change: 1 addition & 0 deletions pkg/tests/testdata/test_volatile-status.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
STAMP_MTIME 946684741
5 changes: 5 additions & 0 deletions pkg/tests/workspace_status.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env bash
stamp_mtime=946684741 # 1999-12-31, 23:58:01
cat << EOF
STAMP_MTIME ${stamp_mtime}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be BUILD_TIMESTAMP

EOF