Re: [PATCH v2 15/16] index-pack: use nr_objects_final as sha1_table size
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 9, 2013, 18:34 UTC
- Message-ID
- <xmqq61u94zew.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <alpine.LFD.2.03.1309091047510.20709@syhkavp.arg>
Nicolas Pitre <nico@fluxnic.net> writes:
Show 32 quoted lines
> 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.
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.