Re: [PATCH 4/8] git-repack --max-pack-size: add fixup_header_footer()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 1 May 2007, Shawn O. Pearce wrote:

> Dana How <danahow@xxxxxxxxx> wrote:
> > On 4/30/07, Shawn O. Pearce <spearce@xxxxxxxxxxx> wrote:
> > >Why not
> > >refactor both to use the same implementation and stuff it away in
> > >say pack-check.c (for lack of a better place), or start a new file
> > >(pack-write.c)?
> > Actually I didn't just copy it, I tried to rewrite it for my use
> > as well as the fast-import.c use (note there is a 3rd copy
> > in some *index*.c file which I didn't try to merge in yet).
> > However I didn't yet put it in a new file or change fast-import.c
> > to call it since I wanted to change as little as possible.
> ...
> > I agree with all your arguments.  I had several reasons
> > to avoid extra rearrangements/refactorings:
> > (a) First patch to git, not previously known to me;
> > (b) I prefer to separate new functionality from "clean-up" work;
> 
> A really good reason.  ;-)
> 
> But I'd still rather see it done right the first time, then done
> partially (copied) and wait for someone to clean it up later.
> Sometimes that cleanup doesn't happen.

Well I intended to do more cleanups in the pack code eventually.  That 
included the index writing and pack header fixing.  But I was expecting 
for the pack splitting changes to go in first as it is likely to impose 
some requirements of its own. It is then easier to have a proper 
interface common to all users after everything is in place.

> > (d) Apparently you and Nicolas Pitre have a lot of pending changes
> >    affecting the packing code.
> 
> Yes, but Nico's work has also destroyed in pack v4 topic.  Nico has
> promised to start working on porting some of that work over, but I
> don't know if he has been able to start doing so yet.

It shouldn't be too hard to port the existing code.  Most of it is new 
code that hooks into the existing code in a limited way.

> I personally have been too busy this past month and a half to really 
> work on packv4, but I'm hoping to circle back to it before the end of 
> May (if Nico doesn't beat me to it!).

While the current pack v4 branch is certainly valuable, I consider it 
more as a proof of concept and a test bench.  In practice it isn't 
really efficient and it won't be able to show its full potential until 
core code like tree walking is better abstracted seamless processing of 
parallel tree representations.

But we're getting there slowly.  My work on index v2 will make the pack 
v4 changes much smaller in that area.  My progress display rework was a 
direct "huh!" reaction to the extra progress reporting the current pack 
v4 code added.  And the general pack-objects.c refactoring was made with 
a look on better and easier pack v4 integration in the future.

So in my mind pack v4 already started to make its way in the main code 
in subtle ways.  ;-)  But since I may do Git work only when I'm bored 
progress doesn't happen as fast as one would have expected.

> > I'd have no problem submitting a follow-on patch later containing
> > some clean-up work if you & NP clear it, so I know I won't have
> > problems from (d).  Note I had to completely rewrite this patch
> > when NP submitted some of his pending stuff.
> 
> Yea, hazard of working in this part of the code when Nico is
> also active.  My own sliding mmap stuff was written twice too,
> for the same reason - Nico doing much needed improvements right in
> the same spot as I was working, at the same time.

Well well.  OK I'm used to be considered as the bad guy anyway.  ;-)


Nicolas
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]