Re: [PATCH v3 00/23] Move exported packfile funcs to its own file

Jonathan Tan [off-list ref] writes:

On Sat, 19 Aug 2017 23:40:33 -0700
Junio C Hamano [off-list ref] wrote:

quoted

Junio C Hamano [off-list ref] writes:

quoted

I have to say that this was a painful topic to integrate.

As you may know, the mk/use-size-t-in-zlib topic is being retracted
and getting rerolled as a larger size_t series, most of which still
needs help in reviewing.

The jt/sha1-file-cleanup topic is the only one among the other four
that are still not in 'next', and I think that topic, as well as the
other three, are all good and serve as a good base to build on top.
So I first rebuilt your patches on top of these four topics.  This
took some time but it wasn't all that painful.

... but it turns out that I screwed it up in at least one place,
making Linux32 build fail (Thanks Lars and folks who pushed hard to
arrange Travis to build all my pushes to 'pu').  I'm pushing out my
second attempt.  Let's see how it goes.

Thanks.

It seems like a later pushout late Sunday night that had the second
attempt made it pass on Linux32 ;-)  Whew.

quoted

 (1) make sure that the topics this depends on are sound by
     re-reading them once again, and merge them quickly down to
     'master';

I took a look and they look sound.

 - rs/find-pack-entry-bisection resolves an issue first introduced in
   commit 1f68855 ("[PATCH] Teach read_sha1_file() and friends about
   packed git object store.", 2005-06-27), which already had that issue.
 - jk/drop-sha1-entry-pos is some code deletion.
 - rs/unpack-entry-leakfix ensures that delta_stack is freed. This
   function does not (for example) expose the destination of delta_stack
   to its caller, so it is correct that delta_stack should be freed
   unless it points to the local buffer, just like in the success case.
 - jt/sha1-file-cleanup (my patches) still looks OK to me.

In your latest "What's cooking" (Aug 2017, #04; Fri, 18), you mentioned
that the first 3 will be merged to master, and the 4th will be merged to
next.

Yup, thanks for double checking.  I'll be merging them down
soon-ish.

Thanks.