From: Junio C Hamano <hidden> Date: 2016-06-15 22:53:14
Jeff King [off-list ref] writes:
quoted
Now, instead of "", we use a pathspec prefix of NULL. If no path
arguments were provided, get_pathspec() will return NULL, and we won't
try to verify the existence of any paths in the tree.
Yeah, this looks like the right thing to do. The get_pathspec code
treats a NULL prefix specially as "no prefix", and I think that is what
we are trying to say here (i.e., we are interpreting pathspecs from the
root).
Yes, that sounds sane.
... However, prefix_pathspec does a lot of magic parsing;
it's unclear to me whether this is all in support of properly
adding the prefix, or if its side effects are important.
These "magic" are for things like :(root)/path that will explicitly
refuse the prefix when run from a subdirectory.
In the longer term, get_pathspec() should be converted to directly
deal with "struct pathspec", but we are not there yet. There are
too many code left-over, even after Duy's last pathspec related
topic, that still look at ->raw field of "struct pathspec" and they
all need to be taught to work with the unified pathspec matching
machinery. This is one of the lots of unfinished loose ends, which
might be a good GSoC project but may be a bit too large for a
student to bite and chew.
From: Jeff King <hidden> Date: 2016-06-15 22:53:14
On Wed, Mar 07, 2012 at 10:38:07PM -0800, Junio C Hamano wrote:
quoted
... However, prefix_pathspec does a lot of magic parsing;
it's unclear to me whether this is all in support of properly
adding the prefix, or if its side effects are important.
These "magic" are for things like :(root)/path that will explicitly
refuse the prefix when run from a subdirectory.
Yeah, that was my impression. In that case, I would think we could get
rid of the get_pathspec call entirely, as it is purely about fixing-up
prefixes, and we know that we have none.
-Peff
From: René Scharfe <hidden> Date: 2016-06-15 22:53:15
Am 08.03.2012 08:15, schrieb Jeff King:
On Wed, Mar 07, 2012 at 10:38:07PM -0800, Junio C Hamano wrote:
quoted
quoted
... However, prefix_pathspec does a lot of magic parsing;
it's unclear to me whether this is all in support of properly
adding the prefix, or if its side effects are important.
These "magic" are for things like :(root)/path that will explicitly
refuse the prefix when run from a subdirectory.
Yeah, that was my impression. In that case, I would think we could get
rid of the get_pathspec call entirely, as it is purely about fixing-up
prefixes, and we know that we have none.
Yes, I think you're right. Not sure why I didn't do that in ebfbdb34,
when that empty prefix was introduced instead.
René
On Wed, Mar 07, 2012 at 10:38:07PM -0800, Junio C Hamano wrote:
quoted
quoted
... However, prefix_pathspec does a lot of magic parsing;
it's unclear to me whether this is all in support of properly
adding the prefix, or if its side effects are important.
These "magic" are for things like :(root)/path that will explicitly
refuse the prefix when run from a subdirectory.
Yeah, that was my impression. In that case, I would think we could get
rid of the get_pathspec call entirely, as it is purely about fixing-up
prefixes, and we know that we have none.
Let me see if I've got this right: We're currently passing in ""/NULL
to get_pathspec() because we handle the prefix beforehand in
parse_treeish_args(). Once we get the tree object, every path is
relative to it, so we don't need to continue using a prefix.
Wouldn't it be better to continue using get_pathspec(), passing it the
real prefix, and looking up tree entries relative to the top-level
tree? The way it works now, you get weird behavior like this:
$ cd xdiff
$ git archive -v --format=tar HEAD ../t/t5000-tar-tree.sh > /dev/null
fatal: '../t/t5000-tar-tree.sh' is outside repository
$ git archive -v --format=tar HEAD .. > /dev/null
fatal: '..' is outside repository
From: René Scharfe <hidden> Date: 2016-06-15 22:53:15
Am 09.03.2012 01:06, schrieb Brodie Rao:
2012/3/7 Jeff King[off-list ref]:
quoted
On Wed, Mar 07, 2012 at 10:38:07PM -0800, Junio C Hamano wrote:
quoted
quoted
... However, prefix_pathspec does a lot of magic parsing;
it's unclear to me whether this is all in support of properly
adding the prefix, or if its side effects are important.
These "magic" are for things like :(root)/path that will explicitly
refuse the prefix when run from a subdirectory.
Yeah, that was my impression. In that case, I would think we could get
rid of the get_pathspec call entirely, as it is purely about fixing-up
prefixes, and we know that we have none.
Let me see if I've got this right: We're currently passing in ""/NULL
to get_pathspec() because we handle the prefix beforehand in
parse_treeish_args(). Once we get the tree object, every path is
relative to it, so we don't need to continue using a prefix.
Wouldn't it be better to continue using get_pathspec(), passing it the
real prefix, and looking up tree entries relative to the top-level
tree? The way it works now, you get weird behavior like this:
$ cd xdiff
$ git archive -v --format=tar HEAD ../t/t5000-tar-tree.sh> /dev/null
fatal: '../t/t5000-tar-tree.sh' is outside repository
$ git archive -v --format=tar HEAD ..> /dev/null
fatal: '..' is outside repository
With get_pathspec() gone you'd get this instead:
fatal: path not found: ..
The message could be improved by mentioning the subdirectory and perhaps
the tree, something like this:
fatal: path not found in subdir 'xdiff' of 'HEAD': ..
However, you seem to expect such an invocation to succeed. What should
go into the created archive in that case and which pathes would be recorded?
René