Re: [PATCH] pack-objects: re-validate data we copy from elsewhere.

From: Linus Torvalds <torvalds@osdl.org>
Date: Sep 3, 2006, 00:27 UTC
Message-ID: <Pine.LNX.4.64.0609021720500.27779@g5.osdl.org>
In-Reply-To: <7vodty1swz.fsf@assigned-by-dhcp.cox.net>

On Sat, 2 Sep 2006, Junio C Hamano wrote:

Show 12 quoted lines

> 
> repack without -a essentially boils down to:
> 
> 	rev-list --objects --all --unpacked |
>         pack-objects $new_pack
> 
> which picks up all live loose objects and create a new pack.
> 
> If rev-list had an extention that lets you say
> 
> 	rev-list --objects --all --unpacked=$active_pack |
> 	pack-objects $new_pack

I have to say, that adding another pack-objects-specific flag to rev-list doesn't necessarily strike me as very nice.

It might be much better to just teach "git pack-objects" to walk the object list by hand. The whole "git-rev-list | git-pack-objects" approach made more sense back when git-rev-list was the _only_ think listing objects, but now that we've librarized the whole object listing thing, maybe it's time to avoid the pipeline and just do it inside the packer logic.

Of course, having git-pack-objects be able to take input from stdin is still useful, but I'd rather start moving the obviously packing-specific flags out of git-rev-list, and into git-pack-objects instead.

[ That would also potentially make packing more efficient - right now the 
  "phase 1" of packing is literally to just figure out the type and the 
  size of the object, in order to sort the object list.

  So when you do a big repack, first "git-rev-list" ends up doing all this 
  work, and then "git-pack-object" ends up _redoing_ some of it.

  Especially for tree objects (which are one of the most common kinds), we 
  already opened the object once when we traversed it, so opening it again 
  just to look at its size is kind of sad.. ]

We also used to have a bug with repacking, where a git-rev-list that died with a failure would silently cause a "good" repack to happen because it had fed git-pack-objects an incomplete list of objects. Junio fixed that with a hack, but if we did all the object listing inside git-pack-objects, we'd not need that kind of things at all..

Hmm? Comments?

		Linus

-- 
VGER BF report: U 0.500053