Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch

[David Turner <dturner@twopensource.com>]

On Tue, 2015-05-12 at 13:22 -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Ahh, that would also give us "missing", so in that sense you are
> > being consistent.
> >
> > But I do not think that consistency is useful.  Showing just
> > "missing" instead is losing information and that is what bothers me.
> >
> > Showing "symlink 6 nosuch" to this "link points at a target that
> > would be in-tree but there is no such object in the tree" symbolic
> > link instead of "missing" would make it more useful, and I do not
> > offhand think of a downside, but maybe I am missing something.
> >
> > For a link that points outside, the code already gives
> >
> >     $ ln -s ../outside outlink
> >     $ git add outlink
> >     $ echo "$(git write-tree):outlink" |
> >       git cat-file --batch --follow-symlinks
> >
> > "symlink ../outside", so the script reading from the batch output
> > already has to be prepared to handle "symlink" and understand it as
> > saying "the link does not point an object that is inside the tree".
> 
> Having said all that, I think we can make readers' life easier by
> classifying various reasons why --follow-symlinks cannot pretend
> as if the in-tree pointee were given as its input and signal it
> differently:
> 
>  * A link points outside the original tree (or the index, once we
>    support "cat-file :RelNotes").
> 
>    I think your "symlink <size> LF <target> LF" already does this.
> 
>  * A link points (directly or indirectly) at itself.
> 
>    This would be your "loop <size> LF <target> LF", which I think is
>    good.
> 
>  * A link does not step outside the original tree, but the pointee
>    does not exist in the original tree.
> 
>    I think you currently say "<object name for HEAD:link> missing",
>    but I do not think it is useful.  That object does exist, but it
>    merely cannot be dereferenced.  Perhaps introducing a new one,
>    e.g. "dangling <size> LF <target> LF" similar to symlink/loop,
>    would help the reader better?
> 
> Are there other cases?  The only other case I think of is when the
> link resolves cleanly inside the tree, which you already handle.
> 
> Thanks.
On Tue, 2015-05-12 at 13:22 -0700, Junio C Hamano wrote:
Junio C Hamano <gitster@pobox.com> writes:
> 
> > Ahh, that would also give us "missing", so in that sense you are
> > being consistent.
> >
> > But I do not think that consistency is useful.  Showing just
> > "missing" instead is losing information and that is what bothers me.
> >
> > Showing "symlink 6 nosuch" to this "link points at a target that
> > would be in-tree but there is no such object in the tree" symbolic
> > link instead of "missing" would make it more useful, and I do not
> > offhand think of a downside, but maybe I am missing something.
> >
> > For a link that points outside, the code already gives
> >
> >     $ ln -s ../outside outlink
> >     $ git add outlink
> >     $ echo "$(git write-tree):outlink" |
> >       git cat-file --batch --follow-symlinks
> >
> > "symlink ../outside", so the script reading from the batch output
> > already has to be prepared to handle "symlink" and understand it as
> > saying "the link does not point an object that is inside the tree".
> 
> Having said all that, I think we can make readers' life easier by
> classifying various reasons why --follow-symlinks cannot pretend
> as if the in-tree pointee were given as its input and signal it
> differently:
> 
>  * A link points outside the original tree (or the index, once we
>    support "cat-file :RelNotes").
> 
>    I think your "symlink <size> LF <target> LF" already does this.
> 
>  * A link points (directly or indirectly) at itself.
> 
>    This would be your "loop <size> LF <target> LF", which I think is
>    good.
> 
>  * A link does not step outside the original tree, but the pointee
>    does not exist in the original tree.
> 
>    I think you currently say "<object name for HEAD:link> missing",
>    but I do not think it is useful.  That object does exist, but it
>    merely cannot be dereferenced.  Perhaps introducing a new one,
>    e.g. "dangling <size> LF <target> LF" similar to symlink/loop,
>    would help the reader better?
> 
> Are there other cases?  The only other case I think of is when the
> link resolves cleanly inside the tree, which you already handle.

While updating the tests, I noticed another two cases:
1. HEAD:broken-link/file

I am inclined to describe this as "dangling" as well.  (It is not useful
to tell users that "file" is the remaining bit to be resolved, because
due to chains of symlinks, users have no idea what file would be
relative to).  I think the filesystem returns ENOENT in the equivalent
case.

2. HEAD:link-to-file/file

This should be "notdir", I think, in that it is a distinct way of
failing that the user might care to know. The filesystem returns ENOTDIR
in the equivalent case.

In addition, I would like to have the format for the dangling, loop, and
notdir cases match the missing case.  In other words, "HEAD:link
missing", "HEAD:link dangling", etc.  Users already need to parse the
missing case, so we might as well make the others match.  It's true that
this parsing is impossible in the case of filenames containing newlines,
but git cat-file --batch cannot accept those filenames as input anyway
(presumably, a -0 flag would be needed, if anyone actually used such
filenames, and at that point we could make the -0 output unambiguously
parseable).