Re: [PATCH v3] pkt-line: allow writing of LARGE_PACKET_MAX buffers
On Wed, Dec 10, 2014 at 2:34 AM, Jeff King p...@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 Note that in addition to cheating on the creation of the long ref, I had to tweak the fetch test a little to avoid writing the loose ref there, too. That makes the test a little weaker (it is not as end to end, checking that all parts of fetch can handle it), but it does check the thing we are changing here, that the protocol code can handle it. -- 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
Re: [PATCH v3] pkt-line: allow writing of LARGE_PACKET_MAX buffers
On Wed, Dec 10, 2014 at 3:36 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Dec 10, 2014 at 2:34 AM, Jeff King p...@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). -- 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
Re: [PATCH v3] pkt-line: allow writing of LARGE_PACKET_MAX buffers
On Wed, Dec 10, 2014 at 4:42 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Dec 10, 2014 at 3:36 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Dec 10, 2014 at 2:34 AM, Jeff King p...@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). -- 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
Re: [PATCH v3] pkt-line: allow writing of LARGE_PACKET_MAX buffers
On Wed, Dec 10, 2014 at 04:49:38AM -0500, Eric Sunshine wrote:
On Wed, Dec 10, 2014 at 4:42 AM, Eric Sunshine sunsh...@sunshineco.com
wrote:
On Wed, Dec 10, 2014 at 3:36 AM, Eric Sunshine sunsh...@sunshineco.com
wrote:
On Wed, Dec 10, 2014 at 2:34 AM, Jeff King p...@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
--
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
