On 5 Dec 2013, at 11:52, Junio C Hamano <gits...@pobox.com> wrote: > Nick Townsend <nick.towns...@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? > Sorry about that - the first four tests do pass with 1.8.5, the last three tests do not currently pass. I believe they should pass and with my reworked git-archive patch (similar to the previous one)they do. (though I removed that update from this set pending the discussion). Currently 5 and 6 fail with the message: ‘fatal: current working directory is untracked’ and number 7 fails with: ‘fatal: Not a valid object name'
> 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>. > That’s not quite right - paths do work from the root directory - see tests. It was this very useful capability that I sought to add and generalized the code to do. > 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. > Clearly when you specify them today they don’t work, but I believe they should work. > 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 > I think we can clear this up by documenting the cases and choosing sensible behaviour _for git-archive_ ie. what people might expect. Here is a suggestion: a. The pathspec is used as a selector to include things in the archive. it is applied after the cwd and treeish selection. b. The current working directory (if present) prefixes a path in the object if one is present. c. The path in the object (if present) is prefixed by the cwd (if present) and used to select items for archiving. However the composite path so created *is not present* in the archive - ie. the leading components are stripped. (This is very useful behaviour for creating archives without leading paths) d. As currently the —prefix option (not the prefix from setup_git_directory) is prepended to all entries in the archive. These correspond to the use cases that I have for git archive - extracting all or part of a multiple submodule tree into a tgz file, with or without leading directories. > 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 It’s clear to me that if you are in a subdirectory, that is an implicit prefix on the ./b so you retrieve HEAD:a/b which then archives everything in b without the leading a/b. The pathspec is wrong (including the b) and this archive is empty. > > and it also feels wrong to say > > cd a && git archive HEAD:./b c > That looks fine to me given the rules suggested above. Also git-parse manages to return the correct thing in this case, so I’d expect this to work. > 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 > Well this will fail both in the existing case (‘fatal: current working directory is untracked’) and with the scheme proposed above (‘fatal: invalid object name $treeish:a/‘) > 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. I’m not seeing that - that’s not quite the same as the algorithm above is it? > >> Helped-by: Eric Sunshine <sunsh...@sunshineco.com> >> Signed-off-by: Nick Townsend <nick.towns...@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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
