Re: [PATCH v3] pkt-line: allow writing of LARGE_PACKET_MAX buffers

[Jeff King <peff@peff.net>]

On Wed, Dec 10, 2014 at 04:49:38AM -0500, Eric Sunshine wrote:

> On Wed, Dec 10, 2014 at 4:42 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Wed, Dec 10, 2014 at 3:36 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> >> On Wed, Dec 10, 2014 at 2:34 AM, Jeff King <peff@peff.net> wrote:
> >>> Below is a another iteration on the patch. The actual code changes are
> >>> the same as the strbuf one, but the tests take care to avoid assuming
> >>> the filesystem can handle such a long path. Testing on Windows and OS X
> >>> is appreciated.
> >>
> >> All three new tests fail on OS X. Thus far brief examination of the
> >> first failing tests shows that 'expect' and 'actual' differ:
> >>
> >> expect:
> >>     long
> >>     master
> >>
> >> actual:
> >>     master
> >
> > The failure manifests as soon as the refname hits length 1024, at
> > which point for-each-ref stops reporting it. MAX_PATH on OS X is 1024,
> > so some part of the machinery invoked by for-each-ref likely is
> > rejecting refnames longer than that (even when coming from
> > packed-refs).
> 
> Clarification: for-each-ref ignores the ref when the full line read
> from packed-refs hits length 1024 (not when the refname itself hits
> length 1024).

Yes, the problem is in read_packed_refs:

    char refline[PATH_MAX];
    ...
    while (fgets(refline, sizeof(refline), f)) {
        ...
    }

This could be trivially converted to strbuf_getwholeline, but I am not
sure what else would break, or whether such a system would actually be
_usable_ with such long refs (e.g., would it break the first time you

Using fgets like this does shear lines, though. The next fgets call will
see the second half of the line. I think we are saved from doing
anything stupid by parse_ref_line, but it is mostly luck. So perhaps for
that reason the trivial conversion to strbuf is worth it, even if it
doesn't help any practical cases.

-Peff