Re: [PATCH] Additional git-archive tests

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 5, 2013, 19:52 UTC
Message-ID: <xmqqwqjj3wit.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CA9E9538-E39B-41CA-BB82-BDD8CF7A2E3F@mac.com>

Nick Townsend <nick.townsend@mac.com> writes:

Show 9 quoted lines

> Interplay between paths specified in three ways now tested:
> * After a : in the tree-ish,
> * As a pathspec in the command,
> * By virtue of the current working directory
>
> Note that these tests are based on the behaviours
> as found in 1.8.5. They may not be intentional.
> They were developed to regression test enhancements
> made to parse_treeish_arg() in archive.c

In other words, are all these new tests expected to pass?

My cursory read of parse_treeish_arg() in archive.c is:

 - It reads the given object with get_sha1(), checking if it is a
   commit-ish or tree-ish to decide if it wants to add the pax
   header to record the commit object name;

 - It parses the tree object;

 - If run from a subdirectory, attempts to grab the "prefix"
   (i.e. the path to the current subdirectory---in the tests you
   added, they are all "a/") out of that tree object (it errors out
   if it can't); and then

 - It archives the tree object.

So I do not think it is expected to accept tree object names with the HEAD:<path> style syntax, if the user expects a predictable and consistent result. The third step above attempts to make sure that you name a tree-ish that corresponds to the top-level of the project, i.e. with no <path>.

What seems to be supported are:

    cd a && git archive HEAD ;# archives HEAD:a tree
    cd a && git archive HEAD -- b ;# archives a/b/ part of HEAD:a as b/

Specifically, it appears that HEAD:./b, HEAD:b etc. are not designed to work, at least to me.

I am not saying that these should _not_ work, but it is unclear what it means to "work". For example, what should this do?

    cd a && git archive HEAD:./b $pathspec

The extended SHA-1 expression HEAD:./b in the subdirectory a/ is interpreted by get_sha1_with_context_1() to be the name of the tree object at path "a/b" in the commit HEAD. Further, you are giving a pathspec while in a subdirectory a/ to the command. What should that pathspec be relative to?

In a normal Git command, the pathspec always is relative to the current subdirectory, so, the way to learn about the tree object a/b/c in the HEAD while in subdirectory a/ would be:

    cd a && git ls-tree HEAD b/c

But what should the command line for archive to grab HEAD:a/b/c be? It feels wrong to say:

    cd a && git archive HEAD:./b b/c

and it also feels wrong to say

    cd a && git archive HEAD:./b c

No matter what we would do, we should behave consistently with this case:

    treeish=$(git rev-parse HEAD:a/b) &&
    cd a &&
    git archive $treeish -- $pathspec

so "take the pathspec relative to the tree when the treeish was given with '<treeish>:<path>' syntax, and otherwise treat it relative to the cwd" is not a workable solution.

Show 86 quoted lines

> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Nick Townsend <nick.townsend@mac.com>
> ---
>  t/t5004-archive-corner-cases.sh | 71 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
>
> diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
> index 67f3b54..a81a836 100755
> --- a/t/t5004-archive-corner-cases.sh
> +++ b/t/t5004-archive-corner-cases.sh
> @@ -113,4 +113,75 @@ test_expect_success 'archive empty subtree by direct pathspec' '
>  	check_dir extract sub
>  '
>  
> +test_expect_success 'setup - repository with subdirs' '
> +	mkdir -p a/b/c a/b/d &&
> +	echo af >a/af &&
> +	echo bf >a/b/bf &&
> +	echo cf >a/b/c/cf &&
> +	git add a &&
> +	git commit -m "commit 1" &&
> +	git tag -a -m "rev-1" rev-1
> +'
> +
> +test_expect_success 'archive subtree from root by treeish' '
> +	git archive --format=tar HEAD:a >atreeroot.tar &&
> +	make_dir extract &&
> +	"$TAR" xf atreeroot.tar -C extract &&
> +	check_dir extract af b b/bf b/c b/c/cf
> +'
> +
> +test_expect_success 'archive subtree from root with pathspec' '
> +	git archive --format=tar HEAD a >atreepath.tar &&
> +	make_dir extract &&
> +	"$TAR" xf atreepath.tar -C extract &&
> +	check_dir extract a a/af a/b a/b/bf a/b/c a/b/c/cf
> +'
> +
> +test_expect_success 'archive subtree from root by 2-level treeish' '
> +	git archive --format=tar HEAD:a/b >abtreeroot.tar &&
> +	make_dir extract &&
> +	"$TAR" xf abtreeroot.tar -C extract &&
> +	check_dir extract bf c c/cf
> +'
> +
> +test_expect_success 'archive subtree from subdir' '
> +	(
> +		cd a &&
> +		git archive --format=tar HEAD >../asubtree.tar
> +	) &&
> +	make_dir extract &&
> +	"$TAR" xf asubtree.tar -C extract &&
> +	check_dir extract af b b/bf b/c b/c/cf
> +'
> +
> +test_expect_success 'archive subtree from subdir with treeish' '
> +	(
> +		cd a &&
> +		git archive --format=tar HEAD:./b >../absubtree.tar
> +	) &&
> +	make_dir extract &&
> +	"$TAR" xf absubtree.tar -C extract &&
> +	check_dir extract bf c c/cf
> +'
> +
> +test_expect_success 'archive subtree from subdir with treeish and pathspec' '
> +	(
> +		cd a &&
> +		git archive --format=tar HEAD:./b c >../absubtree.tar
> +	) &&
> +	make_dir extract &&
> +	"$TAR" xf absubtree.tar -C extract &&
> +	check_dir extract c c/cf
> +'
> +
> +test_expect_success 'archive subtree from subdir with alt treeish' '
> +	(
> +		cd a &&
> +		git archive --format=tar HEAD:b >../abxsubtree.tar
> +	) &&
> +	make_dir extract &&
> +	"$TAR" xf abxsubtree.tar -C extract &&
> +	check_dir extract bf c c/cf
> +'
> +
>  test_done