[PATCH v2] archive: make --add-virtual-file honor --prefix

From: Tom Scogland <scogland1@llnl.gov>

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@llnl.gov>
---
    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@atlas-elektronik.com 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@llnl.gov>
      
     @@ 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(-)