bug in git-archive?

bug in git-archive?

[Nick Williams]

Hello everyone

git-archive only archives the current working dir (and sub dirs) even 
when no paths are specified. For example, if I do

git archive --format=tar --prefix=git-1.5.0.2/ HEAD > ~/test/test.tar

from with in the Documentation dir, then I only get part of the tree.

Is this the intended behavior?

The reason I ask is that from my (mis)reading of the man page I expect 
to get all of the tree unless paths are specified.

thanks

NJW

Re: bug in git-archive?

[Johannes Schindelin]

Hi,

On Sat, 3 Mar 2007, Nick Williams wrote:

> git-archive only archives the current working dir (and sub dirs) even 
> when no paths are specified. For example, if I do
> 
> git archive --format=tar --prefix=git-1.5.0.2/ HEAD > ~/test/test.tar
> 
> > from with in the Documentation dir, then I only get part of the tree.
> 
> Is this the intended behavior?

It is. So, the documentation is wrong.

Ciao,
Dscho

[PATCH] git-archive: document CWD effect

[René Scharfe]

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---

Nick Williams schrieb:
> git-archive only archives the current working dir (and sub dirs) even
> when no paths are specified. For example, if I do
> 
> git archive --format=tar --prefix=git-1.5.0.2/ HEAD > ~/test/test.tar
> 
> from with in the Documentation dir, then I only get part of the tree.
> 
> Is this the intended behavior?
> 
> The reason I ask is that from my (mis)reading of the man page I expect
> to get all of the tree unless paths are specified.

Sorry about the late reply.  Would these two additional manpage lines
clear things up for you?

Thanks,
René


diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt
index 493474b..b688330 100644
--- a/Documentation/git-archive.txt
+++ b/Documentation/git-archive.txt
@@ -17,6 +17,9 @@ Creates an archive of the specified format containing the tree
 structure for the named tree.  If <prefix> is specified it is
 prepended to the filenames in the archive.
 
+Only the files and directories in the current working directory are
+included in archives created locally (i.e. without --remote).
+
 'git-archive' behaves differently when given a tree ID versus when
 given a commit ID or tag ID.  In the first case the current time is
 used as modification time of each file in the archive.  In the latter

Re: [PATCH] git-archive: document CWD effect

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Nick Williams schrieb:
>> git-archive only archives the current working dir (and sub dirs) even
>> when no paths are specified. For example, if I do
>> 
>> git archive --format=tar --prefix=git-1.5.0.2/ HEAD > ~/test/test.tar
>> 
>> from with in the Documentation dir, then I only get part of the tree.
>> 
>> Is this the intended behavior?
>> 
>> The reason I ask is that from my (mis)reading of the man page I expect
>> to get all of the tree unless paths are specified.
>
> Sorry about the late reply.  Would these two additional manpage lines
> clear things up for you?

While the updated description reflects what the command does
more accurately, I am not sure if it is a desired behaviour.
For one thing, --format=tar (by the way, maybe we would want to
make this the default when none is specified?) adds the comment
that is readable by get-tar-commit-id that claims the tarball
contains the named commit, giving a false impression that it is
the whole thing.  Since people who _really_ want a subtree can
just say "git archive --format=tar HEAD:Documentation", I
suspect we may be better off not doing "current directory only"
by default.  This changes the behaviour, but (1) it affects only
people who run from a subdirectory, (2) it is counterintuitive
that your location in the working tree matters when you say "I
want a tarball of that commit", and (3) it is an undocumented
behaviour anyway.

So my suggestions are:

 (1) When no pathspec is given, archive the whole tree, even
     when you are in a subdirectory.

 (2) When a pathspec is given, produce a partial tarball limited
     to the named spec like we do now, but do not say it is a
     tarball of the named commit (i.e. get-tar-commit-id would
     say empty).

An alternative to (2) would be to say "$commit:Documentation"
instead, but that has a little issue of what to do when more
than one pathspecs are given.

Re: [PATCH] git-archive: document CWD effect

[René Scharfe]

Junio C Hamano schrieb:
> While the updated description reflects what the command does
> more accurately, I am not sure if it is a desired behaviour.
> For one thing, --format=tar (by the way, maybe we would want to
> make this the default when none is specified?) adds the comment
> that is readable by get-tar-commit-id that claims the tarball
> contains the named commit, giving a false impression that it is
> the whole thing.

It marks the archive as being _created_ from this specific commit, not
necessarily as containing all of it.  Perhaps this should be noted in
the documentation..

Making '--format=tar' the default is a good idea.  I doubt we'll see the
addition of a new archive format -- that deserves to be the default one
instead of tar -- soon.

> Since people who _really_ want a subtree can
> just say "git archive --format=tar HEAD:Documentation", I
> suspect we may be better off not doing "current directory only"
> by default.  This changes the behaviour, but (1) it affects only
> people who run from a subdirectory, (2) it is counterintuitive
> that your location in the working tree matters when you say "I
> want a tarball of that commit", and (3) it is an undocumented
> behaviour anyway.

I agree with (1) and (3), meaning that we are free to change the
behaviour.  I don't agree with (2), though.  I'd find it strange if
changing the working directory wouldn't change the archive contents.

We should keep consistency with the rest of git here.  Since git-archive
is just a fancy git-ls-tree, I think we should mirror its behaviour with
respect to the working directory.  (Which is what the current code does.
 Modulo bugs, of course.)

René

Re: [PATCH] git-archive: document CWD effect

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Junio C Hamano schrieb:
> ...
>> Since people who _really_ want a subtree can
>> just say "git archive --format=tar HEAD:Documentation", I
>> suspect we may be better off not doing "current directory only"
>> by default.  This changes the behaviour, but (1) it affects only
>> people who run from a subdirectory, (2) it is counterintuitive
>> that your location in the working tree matters when you say "I
>> want a tarball of that commit", and (3) it is an undocumented
>> behaviour anyway.
>
> I agree with (1) and (3), meaning that we are free to change the
> behaviour.  I don't agree with (2), though.  I'd find it strange if
> changing the working directory wouldn't change the archive contents.
>
> We should keep consistency with the rest of git here.  Since git-archive
> is just a fancy git-ls-tree, I think we should mirror its behaviour with
> respect to the working directory.  (Which is what the current code does.
> Modulo bugs, of course.)

I am tempted to say "fair enough", but I would wait for others
to come up with objections I did not think of.

Re: [PATCH] git-archive: document CWD effect

[Andy Parkins]

On Monday 2007, April 09, René Scharfe wrote:

> I agree with (1) and (3), meaning that we are free to change the
> behaviour.  I don't agree with (2), though.  I'd find it strange if
> changing the working directory wouldn't change the archive contents.
>
> We should keep consistency with the rest of git here.  Since
> git-archive is just a fancy git-ls-tree, I think we should mirror its
> behaviour with respect to the working directory.  (Which is what the
> current code does. Modulo bugs, of course.)

I don't agree with the supposition that git-archive is a fancy 
git-ls-tree.  If it were, then you'd be right.  It's not though.  It's 
more like a git-read-tree or git-checkout-index; those both don't care 
where you are in the working tree.

Argument 1)
git-archive should have nothing to do with a working tree in fact; it's 
perfectly reasonable to expect that it would work in a bare repository 
in fact - that's almost the definition of a command that shouldn't be 
working directory aware.

Argument 2)
Consider the --remote option.  What "working path" should be relevant 
when "--remote" is passed?  For consistency, git-archive should always 
refer to the repository root.

Argument 3)
git-archive is similar to other VCS's "export" command; and for those 
the export command in it's default form will work without a local 
checkout and they export from the repository root.

Argument 4)
What if the repository has multiple root commits, similar to git's html 
and todo branches.  Now, use git-archive and reference one of those 
commits.  The working directory you're in now has no relevance at all 
to the commit your targeting - it need not even exist.  The same 
problem exists of course if you are now in a directory that didn't 
exist in the past.



Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

Re: [PATCH] git-archive: document CWD effect

[René Scharfe]

Andy Parkins schrieb:
> On Monday 2007, April 09, René Scharfe wrote:
> 
>> I agree with (1) and (3), meaning that we are free to change the
>> behaviour.  I don't agree with (2), though.  I'd find it strange if
>> changing the working directory wouldn't change the archive contents.
>>
>> We should keep consistency with the rest of git here.  Since
>> git-archive is just a fancy git-ls-tree, I think we should mirror its
>> behaviour with respect to the working directory.  (Which is what the
>> current code does. Modulo bugs, of course.)
> 
> I don't agree with the supposition that git-archive is a fancy 
> git-ls-tree.  If it were, then you'd be right.  It's not though.  It's 
> more like a git-read-tree or git-checkout-index; those both don't care 
> where you are in the working tree.
>
> Argument 1)
> git-archive should have nothing to do with a working tree in fact; it's 
> perfectly reasonable to expect that it would work in a bare repository 
> in fact - that's almost the definition of a command that shouldn't be 
> working directory aware.

It works in bare repositories, as does git-ls-tree.  There the question
of what to do in subdirectories doesn't even come up, because there are
none. :)

But just because it works in situations where there are no
subdirectories that doesn't mean that it has to ignore them in other
situations.

> Argument 2)
> Consider the --remote option.  What "working path" should be relevant 
> when "--remote" is passed?  For consistency, git-archive should always 
> refer to the repository root.

'git-archive --remote' passes the options to the remote system, ignoring
the local working directory.  What matters in this case is the working
directory on the remote end, which is the repository's root.

But just because there is currently no way (that I know of) to change
the working directory on the remote end that doesn't mean the command
needs to ignore the working directory when operating locally, where it
can be changed easily.

> Argument 3)
> git-archive is similar to other VCS's "export" command; and for those 
> the export command in it's default form will work without a local 
> checkout and they export from the repository root.

OK, I haven't seen other tools, so I can't really comment on them.
git-archive _does_ work without a checkout, though.

> Argument 4)
> What if the repository has multiple root commits, similar to git's html 
> and todo branches.  Now, use git-archive and reference one of those 
> commits.  The working directory you're in now has no relevance at all 
> to the commit your targeting - it need not even exist.  The same 
> problem exists of course if you are now in a directory that didn't 
> exist in the past.

Yes, that's true; git-archive will tell you that your current working
directory is untracked in that case.  (git-ls-tree in that case lists:
nothing.)  But if you want to create a full archive you need to be in
the repository's root directory anyway, so this is no new issue.

What it comes down to is the decision between consistency with third
party export tools or with its (implementation-wise) brother
git-ls-tree, between the convenience of not needing to care where in the
repository you are or the convenience of being able to let the working
directory determine the file set that ends up in the archive.

I'd rather keep it consistent from a technical, implementation point of
view and not care too much about other export tools, and I like my
working directory to influence which files I'm working with.  Working
directory sensitivity is just another input method, like parameters and
environment variables.  I still see no benefit in removing it.  But
perhaps I'm in a rut, unable to see the difficulties with this way to work.

Re(not getting it ;-)né

Re: [PATCH] git-archive: document CWD effect

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> But just because there is currently no way (that I know of) to change
> the working directory on the remote end that doesn't mean the command
> needs to ignore the working directory when operating locally, where it
> can be changed easily.

I do not think it has much to do with "working directory".  The
remote one would most often be bare, and especially the public
ones are so.  I think the issue is if I ask a tarball of this
commit without giving any specific parameters, what should
happen.

And I think most people find it natural if you give them the
whole tree no matter where they start from.

You could:

        $ git archive HEAD -- t/howto

to have a narrowed tarball, and even if you make:

	$ cd t/howto
	$ git archive HEAD

to produce the whole tree, the user can still do:

	$ cd t/howto
        $ git archive HEAD -- .

to get the narrowed tree if the command understands the prefix
it receives from git_setup_directory().

So while I understand when you say this is "another input
method", letting people in a deep directory to abbreviate their
cwd with "." would equally a good input method, and probably
better than the current implementation, which leaves people
without a single way to say "I want the whole tree" without
cd'ing up.

Re: [PATCH] git-archive: document CWD effect

[René Scharfe]

Junio C Hamano schrieb:
> And I think most people find it natural if you give them the whole
> tree no matter where they start from.

Well, I can't argue against that, as I don't have any numbers.  But I
can understand this expectation a bit ("archive HEAD!" -- "here is an
ear" might be confusing ;).

> You could:
> 
> $ git archive HEAD -- t/howto
> 
> to have a narrowed tarball, and even if you make:
> 
> $ cd t/howto $ git archive HEAD
> 
> to produce the whole tree, the user can still do:
> 
> $ cd t/howto $ git archive HEAD -- .
> 
> to get the narrowed tree if the command understands the prefix it
> receives from git_setup_directory().
> 
> So while I understand when you say this is "another input method",
> letting people in a deep directory to abbreviate their cwd with "."
> would equally a good input method, and probably better than the
> current implementation, which leaves people without a single way to
> say "I want the whole tree" without cd'ing up.

Hmm, switching the feature's default mode around instead of cutting it
off completely might be a good idea.  Something like this?  Unlike the
current implementation, it always writes the full path from the repo
root for each file.

René


diff --git a/builtin-archive.c b/builtin-archive.c
index 7f4e409..efa8679 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -128,18 +128,6 @@ void parse_treeish_arg(const char **argv, struct archiver_args *ar_args,
 	if (tree == NULL)
 		die("not a tree object");
 
-	if (prefix) {
-		unsigned char tree_sha1[20];
-		unsigned int mode;
-		int err;
-
-		err = get_tree_entry(tree->object.sha1, prefix,
-				     tree_sha1, &mode);
-		if (err || !S_ISDIR(mode))
-			die("current working directory is untracked");
-
-		tree = parse_tree_indirect(tree_sha1);
-	}
 	ar_args->tree = tree;
 	ar_args->commit_sha1 = commit_sha1;
 	ar_args->time = archive_time;
@@ -241,6 +229,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 	struct archiver ar;
 	int tree_idx;
 	const char *remote = NULL;
+	const char **pathspec;
 
 	remote = extract_remote_arg(&argc, argv);
 	if (remote)
@@ -255,7 +244,11 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 
 	argv += tree_idx;
 	parse_treeish_arg(argv, &ar.args, prefix);
-	parse_pathspec_arg(argv + 1, &ar.args);
+
+	pathspec = argv + 1;
+	if (*pathspec && prefix)
+		pathspec = get_pathspec(prefix, pathspec);
+	parse_pathspec_arg(pathspec, &ar.args);
 
 	return ar.write_archive(&ar.args);
 }