From: Junio C Hamano <gitster@pobox•com>
To: Nick Townsend <nick.townsend@mac•com>
Cc: "Git List" <git@vger•kernel.org>,
"Eric Sunshine" <sunshine@sunshineco•com>,
"René Scharfe" <l.s.r@web•de>,
"Jens Lehmann" <Jens.Lehmann@web•de>,
"Heiko Voigt" <hvoigt@hvoigt•net>
Subject: Re: [PATCH] Additional git-archive tests
Date: Thu, 05 Dec 2013 11:52:58 -0800 [thread overview]
Message-ID: <xmqqwqjj3wit.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CA9E9538-E39B-41CA-BB82-BDD8CF7A2E3F@mac.com> (Nick Townsend's message of "Wed, 04 Dec 2013 18:49:39 -0800")
Nick Townsend <nick.townsend@mac•com> writes:
> 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.
> 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
next prev parent reply other threads:[~2013-12-05 19:53 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-03 0:10 [PATCH] Improvements to git-archive tests and add_submodule_odb() Nick Townsend
2013-12-03 0:14 ` Nick Townsend
2013-12-03 17:52 ` Junio C Hamano
2013-12-03 18:18 ` Heiko Voigt
2013-12-03 18:39 ` Re* " Junio C Hamano
2013-12-03 20:42 ` Heiko Voigt
2013-12-03 0:16 ` Nick Townsend
2013-12-03 9:26 ` Eric Sunshine
2013-12-03 17:54 ` Junio C Hamano
2013-12-05 2:49 ` [PATCH] Additional git-archive tests Nick Townsend
2013-12-05 19:52 ` Junio C Hamano [this message]
2013-12-10 5:26 ` Nick Townsend
2013-12-10 18:48 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqwqjj3wit.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=Jens.Lehmann@web$(echo .)de \
--cc=git@vger$(echo .)kernel.org \
--cc=hvoigt@hvoigt$(echo .)net \
--cc=l.s.r@web$(echo .)de \
--cc=nick.townsend@mac$(echo .)com \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox