On Mon, 9 Sep 2013, Junio C Hamano wrote:

> Nicolas Pitre <n...@fluxnic.net> writes:
> > On Mon, 9 Sep 2013, Nguyễn Thái Ngọc Duy wrote:
> >
> >> nr_objects in the next patch is used to reflect the number of actual
> >> objects in the stream, which may be smaller than the number recorded
> >> in pack header.
> >
> > This highlights an issue that has been nagging me for a while.
> >
> > We decided to send the final number of objects in the thin pack header 
> > for two reasons:
> >
> > 1) it allows to properly size the SHA1 table upfront which already 
> >    contains entries for the omitted objects;
> >
> > 2) the whole pack doesn't have to be re-summed again after being 
> >    completed on the receiving end since we don't alter the header.
> >
> > However this means that the progress meter will now be wrong and that's 
> > terrible !  Users *will* complain that the meter doesn't reach 100% and 
> > they'll protest for being denied the remaining objects during the 
> > transfer !
> >
> > Joking aside, we should think about doing something about it.  I was 
> > wondering if some kind of prefix to the pack stream could be inserted 
> > onto the wire when sending a pack v4.  Something like:
> >
> > 'T', 'H', 'I', 'N', <actual_number_of_sent_objects_in_network_order>
> >
> > This 8-byte prefix would simply be discarded by index-pack after being 
> > parsed.
> >
> > What do you think?
> I do not think it is _too_ bad if the meter jumped from 92% to 100%
> when we finish reading from the other end ;-), as long as we can
> reliably tell that we read the right thing.

Sure.  but eventually people will complain about this.  So while we're 
about to introduce a new pack format anyway, better think of this little 
cosmetic detail now when it can be included in the pack v4 capability 

> Which brings me to a tangent.  Do we have a means to make sure that
> the data received over the wire is bit-for-bit correct as a whole
> when it is a thin pack stream?  When it is a non-thin pack stream,
> we have the checksum at the end added by sha1close() which
> index-pack.c::parse_pack_objects() can (and does) verify.

The trailing checksum is still there.  Nothing has changed in that 


Reply via email to