Re: [PATCH 5/8] get-repack --max-pack-size: write_object() takes 'limit' arg

Junio C Hamano [off-list ref] writes:

Dana How [off-list ref] writes:

quoted

Accept new 'limit' argument and check against it
before each group of writes.  Update delta usability rules
for possibility of delta base being in a previously-
written pack. Inline sha1write_compressed() so we know
the exact size of the written data when it needs to be compressed.

Signed-off-by: Dana How <redacted>

My first reaction of open-coding sha1write_compressed() was
"Ugh", but as you are removing the only user of that function,
maybe this is not as bad as it looks.

Having said that, I suspect that for other possible users of
that function we might have later, it would be a better
interface to add an optional 'limit' and 'prelude' to
sha1write_compressed().  The function would write prelude
followed by the compressed payload, only if they fit the limit.

Then your write_object() would prepare the header (depending on
the type, the object header, ofs-delta header or ref-delta
header) in header[] but would not cal sha1write() itself.
Instead it would send header[] in as prelude, *buf as the
payload, with an appropriate limit.

On Sun, 8 Apr 2007, Junio C Hamano wrote:

Junio C Hamano [off-list ref] writes:

quoted

Dana How [off-list ref] writes:

quoted

Accept new 'limit' argument and check against it
before each group of writes.  Update delta usability rules
for possibility of delta base being in a previously-
written pack. Inline sha1write_compressed() so we know
the exact size of the written data when it needs to be compressed.

Signed-off-by: Dana How <redacted>

My first reaction of open-coding sha1write_compressed() was
"Ugh", but as you are removing the only user of that function,
maybe this is not as bad as it looks.

Having said that, I suspect that for other possible users of
that function we might have later, it would be a better
interface to add an optional 'limit' and 'prelude' to
sha1write_compressed().  The function would write prelude
followed by the compressed payload, only if they fit the limit.

I'd wait for those possible future users to show up before doing such 
thing though.


Nicolas