From: Tom Scogland <scogland1@xxxxxxxx>
The documentation for archive describes the `--add-virtual-file` option
thusly:
The path of the file in the archive is built by concatenating the
value of the last `--prefix` moption (if any) before this
`--add-virtual-file` and <path>.
The `--add-file` documentation is similar:
The path of the file in the archive is built by concatenating the
value of the last --prefix option (if any) before this --add-file and
the basename of <file>.
Notably both explicitly state that they honor the last `--prefix` option
before the `--add` option in question. The implementation of
`--add-file` seems to have always honored prefix, but the implementation
of `--add-virtual-file` does not. Also note that `--add-virtual-file`
explicitly states it will use the full path given, while `--add-file`
uses the basename of the path it is given.
Modify archive.c to include the prefix in the path used by
`--add-virtual-file` and add checks into
the existing add-virtual-file test to verify:
* that `--prefix` is honored
* that leading path components are preserved
* that both work together and separately
Changes since v1:
- Revised the commit message style
- Added tests for basename/non-basename behavior
- Fixed archive.c to use full path for virtual and basename for add-file
Signed-off-by: Tom Scogland <scogland1@xxxxxxxx>
---
archive: make --add-virtual-file honor --prefix
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1719%2Ftrws%2Fhonor-prefix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1719/trws/honor-prefix-v2
Pull-Request: https://github.com/git/git/pull/1719
Range-diff vs v1:
1: 1b685d8ca1a ! 1: b9a0ef282a3 archive: make --add-virtual-file honor --prefix
@@ Metadata
## Commit message ##
archive: make --add-virtual-file honor --prefix
- The documentation for archive states:
+ The documentation for archive describes the `--add-virtual-file` option
+ thusly:
The path of the file in the archive is built by concatenating the
value of the last `--prefix` moption (if any) before this
`--add-virtual-file` and <path>.
- This matches the documentation for --add-file and the behavior works for
- that option, but --prefix is ignored for --add-virtual-file.
+ The `--add-file` documentation is similar:
- This commit modifies archive.c to include the prefix in the path and
- adds a check into the existing add-virtual-file test to ensure that it
- honors both the most recent prefix before the flag.
+ The path of the file in the archive is built by concatenating the
+ value of the last --prefix option (if any) before this --add-file and
+ the basename of <file>.
+
+ Notably both explicitly state that they honor the last `--prefix` option
+ before the `--add` option in question. The implementation of
+ `--add-file` seems to have always honored prefix, but the implementation
+ of `--add-virtual-file` does not. Also note that `--add-virtual-file`
+ explicitly states it will use the full path given, while `--add-file`
+ uses the basename of the path it is given.
+
+ Modify archive.c to include the prefix in the path used by
+ `--add-virtual-file` and add checks into
+ the existing add-virtual-file test to verify:
+
+ * that `--prefix` is honored
+ * that leading path components are preserved
+ * that both work together and separately
- In looking for others with this issue, I found message
- a143e25a70b44b82b4ee6fa3bb2bcda4@xxxxxxxxxxxxxxxxxxxx on the mailing
- list, where Stefan proposed a basically identical patch to archive.c
- back in February, so the main addition here is the test along with that
- patch.
+ Changes since v1:
+ - Revised the commit message style
+ - Added tests for basename/non-basename behavior
+ - Fixed archive.c to use full path for virtual and basename for add-file
Signed-off-by: Tom Scogland <scogland1@xxxxxxxx>
@@ archive.c: int write_archive_entries(struct archiver_args *args,
+ strbuf_reset(&path_in_archive);
+ if (info->base)
+ strbuf_addstr(&path_in_archive, info->base);
-+ strbuf_addstr(&path_in_archive, basename(path));
if (!info->content) {
- strbuf_reset(&path_in_archive);
- if (info->base)
- strbuf_addstr(&path_in_archive, info->base);
-- strbuf_addstr(&path_in_archive, basename(path));
+ strbuf_addstr(&path_in_archive, basename(path));
-
strbuf_reset(&content);
if (strbuf_read_file(&content, path, info->stat.st_size) < 0)
err = error_errno(_("cannot read '%s'"), path);
@@ archive.c: int write_archive_entries(struct archiver_args *args,
+ canon_mode(info->stat.st_mode),
content.buf, content.len);
} else {
++ strbuf_addstr(&path_in_archive, path);
err = write_entry(args, &fake_oid,
- path, strlen(path),
+ path_in_archive.buf, path_in_archive.len,
@@ t/t5003-archive-zip.sh: test_expect_success UNZIP 'git archive --format=zip --ad
--add-virtual-file=\""$PATHNAME"\": \
- --add-virtual-file=hello:world $EMPTY_TREE &&
+ --add-virtual-file=hello:world \
-+ --prefix=subdir/ --add-virtual-file=hello:world \
++ --add-virtual-file=with/dir/noprefix:withdirnopre \
++ --prefix=subdir/ --add-virtual-file=with/dirprefix:withdirprefix \
++ --prefix=subdir2/ --add-virtual-file=withoutdir:withoutdir \
+ --prefix= $EMPTY_TREE &&
test_when_finished "rm -rf tmp-unpack" &&
mkdir tmp-unpack && (
@@ t/t5003-archive-zip.sh: test_expect_success UNZIP 'git archive --format=zip --ad
test_path_is_file "$PATHNAME" &&
- test world = $(cat hello)
+ test world = $(cat hello) &&
-+ test_path_is_file subdir/hello &&
-+ test world = $(cat subdir/hello)
++ test_path_is_file with/dir/noprefix &&
++ test withdirnopre = $(cat with/dir/noprefix) &&
++ test_path_is_file subdir/with/dirprefix &&
++ test withdirprefix = $(cat subdir/with/dirprefix) &&
++ test_path_is_file subdir2/withoutdir &&
++ test withoutdir = $(cat subdir2/withoutdir)
)
'
archive.c | 10 +++++-----
t/t5003-archive-zip.sh | 14 ++++++++++++--
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/archive.c b/archive.c
index 5287fcdd8e0..64777a9870d 100644
--- a/archive.c
+++ b/archive.c
@@ -365,12 +365,11 @@ int write_archive_entries(struct archiver_args *args,
put_be64(fake_oid.hash, i + 1);
+ strbuf_reset(&path_in_archive);
+ if (info->base)
+ strbuf_addstr(&path_in_archive, info->base);
if (!info->content) {
- strbuf_reset(&path_in_archive);
- if (info->base)
- strbuf_addstr(&path_in_archive, info->base);
strbuf_addstr(&path_in_archive, basename(path));
-
strbuf_reset(&content);
if (strbuf_read_file(&content, path, info->stat.st_size) < 0)
err = error_errno(_("cannot read '%s'"), path);
@@ -380,8 +379,9 @@ int write_archive_entries(struct archiver_args *args,
canon_mode(info->stat.st_mode),
content.buf, content.len);
} else {
+ strbuf_addstr(&path_in_archive, path);
err = write_entry(args, &fake_oid,
- path, strlen(path),
+ path_in_archive.buf, path_in_archive.len,
canon_mode(info->stat.st_mode),
info->content, info->stat.st_size);
}
diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh
index 961c6aac256..0cf3aef8ace 100755
--- a/t/t5003-archive-zip.sh
+++ b/t/t5003-archive-zip.sh
@@ -218,14 +218,24 @@ test_expect_success UNZIP 'git archive --format=zip --add-virtual-file' '
fi &&
git archive --format=zip >with_file_with_content.zip \
--add-virtual-file=\""$PATHNAME"\": \
- --add-virtual-file=hello:world $EMPTY_TREE &&
+ --add-virtual-file=hello:world \
+ --add-virtual-file=with/dir/noprefix:withdirnopre \
+ --prefix=subdir/ --add-virtual-file=with/dirprefix:withdirprefix \
+ --prefix=subdir2/ --add-virtual-file=withoutdir:withoutdir \
+ --prefix= $EMPTY_TREE &&
test_when_finished "rm -rf tmp-unpack" &&
mkdir tmp-unpack && (
cd tmp-unpack &&
"$GIT_UNZIP" ../with_file_with_content.zip &&
test_path_is_file hello &&
test_path_is_file "$PATHNAME" &&
- test world = $(cat hello)
+ test world = $(cat hello) &&
+ test_path_is_file with/dir/noprefix &&
+ test withdirnopre = $(cat with/dir/noprefix) &&
+ test_path_is_file subdir/with/dirprefix &&
+ test withdirprefix = $(cat subdir/with/dirprefix) &&
+ test_path_is_file subdir2/withoutdir &&
+ test withoutdir = $(cat subdir2/withoutdir)
)
'
base-commit: 786a3e4b8d754d2b14b1208b98eeb0a554ef19a8
--
gitgitgadget
