Skip to content

Commit

Permalink
fix(tar): append slash to top-level directory mtree entries (#852)
Browse files Browse the repository at this point in the history
* fix(tar): append slash to top-level directory mtree entries

bsdtar's mtree format has a quirk wherein entries without "/" in their
first word are treated as "relative" entries, and "relative" directories
will cause tar to "change directory" into the declared directory entry.
If such a directory is followed by a "relative" entry, then the file
will be created within the directory, instead of at top-level as
expected. To mitigate, we append a slash to top-level directory entries.

Fixes #851.

* chore: golden files have BINDIR placeholder

---------

Co-authored-by: Alex Eagle <[email protected]>
  • Loading branch information
sin-ack and alexeagle authored Jul 2, 2024
1 parent 086624a commit cc956d8
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 0 deletions.
38 changes: 38 additions & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
load("@bazel_skylib//rules:write_file.bzl", "write_file")
load("@buildifier_prebuilt//:rules.bzl", "buildifier")
load("//lib:diff_test.bzl", "diff_test")
load("//lib:tar.bzl", "mtree_spec")
load("//lib:testing.bzl", "assert_contains")
load("//lib:utils.bzl", "is_bazel_7_or_greater")
load("//lib:write_source_files.bzl", "write_source_files")
Expand Down Expand Up @@ -136,3 +137,40 @@ bzl_library(
visibility = ["//visibility:public"],
deps = ["@bazel_gazelle//:deps"],
)

# Test case for mtree_spec: Ensure that multiple entries at the root directory are handled correctly (bug #851)
# See lib/tests/tar/BUILD.bazel for why this is here.
write_file(
name = "tar_test13_main",
out = "13project/__main__.py",
content = ["__main__.py"],
)

write_file(
name = "tar_test13_bin",
out = "13project_bin",
content = ["project_bin"],
)

mtree_spec(
name = "tar_test13_mtree_unsorted",
srcs = [
":tar_test13_bin",
":tar_test13_main",
],
)

# NOTE: On some systems, the mtree_spec output can have a different order.
# To make the test less brittle, we sort the mtree output and replace the BINDIR with a constant placeholder
genrule(
name = "tar_test13_mtree",
srcs = [":tar_test13_mtree_unsorted"],
outs = ["actual13.mtree"],
cmd = "sort $< | sed 's#$(BINDIR)#{BINDIR}#' >$@",
)

diff_test(
name = "tar_test13",
file1 = "tar_test13_mtree",
file2 = "//lib/tests/tar:expected13.mtree",
)
11 changes: 11 additions & 0 deletions lib/private/modify_mtree.awk
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@
} else if (index($1, strip_prefix) == 1) {
# this line starts with the strip_prefix
sub("^" strip_prefix "/", "");

# NOTE: The mtree format treats file paths without slashes as "relative" entries.
# If a relative entry is a directory, then it will "change directory" to that
# directory, and any subsequent "relative" entries will be created inside that
# directory. This causes issues when there is a top-level directory that is
# followed by a top-level file, as the file will be created inside the directory.
# To avoid this, we append a slash to the directory path to make it a "full" entry.
components = split($1, _, "/");
if ($0 ~ /type=dir/ && components == 1) {
$1 = $1 "/";
}
} else {
# this line declares some path under a parent directory, which will be discarded
next;
Expand Down
10 changes: 10 additions & 0 deletions lib/private/tar.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,16 @@ def _expand(file, expander, transform = to_repository_relative_path):
segments = path.split("/")
for i in range(1, len(segments)):
parent = "/".join(segments[:i])

# NOTE: The mtree format treats file paths without slashes as "relative" entries.
# If a relative entry is a directory, then it will "change directory" to that
# directory, and any subsequent "relative" entries will be created inside that
# directory. This causes issues when there is a top-level directory that is
# followed by a top-level file, as the file will be created inside the directory.
# To avoid this, we append a slash to the directory path to make it a "full" entry.
if i == 1:
parent += "/"

lines.append(_mtree_line(parent, "dir"))

lines.append(_mtree_line(_vis_encode(path), "file", content = _vis_encode(e.path)))
Expand Down
49 changes: 49 additions & 0 deletions lib/tests/tar/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -359,3 +359,52 @@ diff_test(
file1 = "modified2.mtree",
file2 = "expected2.mtree",
)

# Case 13: Ensure that multiple entries at the root directory are handled correctly (bug #851)
# NOTE: The mtree_spec part of this test is placed at the root BUILD.bazel because
# that's the only way to ensure that the mtree_spec generates single-component
# entries (which would trigger the bug).
exports_files(["expected13.mtree"])

# Case 14: Ensure mtree_mutate correctly handles prefix stripping for top-level directories (bug #851)

write_file(
name = "test14_main",
out = "14project/__main__.py",
content = ["__main__.py"],
)

write_file(
name = "test14_bin",
out = "14project_bin",
content = ["project_bin"],
)

mtree_spec(
name = "mtree14",
srcs = [
":test14_bin",
":test14_main",
],
)

mtree_mutate(
name = "strip_prefix14_unsorted",
mtree = "mtree14",
strip_prefix = "lib/tests/tar",
)

# NOTE: On some systems, the mtree_spec output can have a different order.
# To make the test less brittle, we sort the mtree output and replace the BINDIR with a constant placeholder
genrule(
name = "strip_prefix14",
srcs = [":strip_prefix14_unsorted"],
outs = ["actual14.mtree"],
cmd = "sort $< | sed 's#$(BINDIR)#{BINDIR}#' >$@",
)

diff_test(
name = "test14",
file1 = ":strip_prefix14",
file2 = "expected14.mtree",
)
3 changes: 3 additions & 0 deletions lib/tests/tar/expected13.mtree
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
13project/ uid=0 gid=0 time=1672560000 mode=0755 type=dir
13project/__main__.py uid=0 gid=0 time=1672560000 mode=0755 type=file content={BINDIR}/13project/__main__.py
13project_bin uid=0 gid=0 time=1672560000 mode=0755 type=file content={BINDIR}/13project_bin
3 changes: 3 additions & 0 deletions lib/tests/tar/expected14.mtree
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
14project/ uid=0 gid=0 time=1672560000 mode=0755 type=dir
14project/__main__.py uid=0 gid=0 time=1672560000 mode=0755 type=file content={BINDIR}/lib/tests/tar/14project/__main__.py
14project_bin uid=0 gid=0 time=1672560000 mode=0755 type=file content={BINDIR}/lib/tests/tar/14project_bin

0 comments on commit cc956d8

Please sign in to comment.