From: "Shawn O. Pearce" <spearce@spearce•org>
To: Nicolas Pitre <nico@cam•org>
Cc: Junio C Hamano <junkio@cox•net>, Dana How <danahow@gmail•com>,
Git Mailing List <git@vger•kernel.org>
Subject: Re: [PATCH 4/8] git-repack --max-pack-size: add fixup_header_footer()
Date: Mon, 9 Apr 2007 13:38:58 -0400 [thread overview]
Message-ID: <20070409173858.GT5436@spearce.org> (raw)
In-Reply-To: <alpine.LFD.0.98.0704082012220.28181@xanadu.home>
Nicolas Pitre <nico@cam•org> wrote:
> On Sun, 8 Apr 2007, Junio C Hamano wrote:
>
> > Dana How <danahow@gmail•com> writes:
> >
> > > +/*
> > > + * Move this, the version in fast-import.c,
> > > + * and index_pack.c:readjust_pack_header_and_sha1 into sha1_file.c ?
> > > + */
> > > +static void fixup_header_footer(int pack_fd, unsigned char *pack_file_sha1,
> > > + char *pack_name, uint32_t object_count)
> > > +{
> >
> > Indeed that is a very good point.
> >
> > I admit I did not notice we already had the duplication between
> > fast-import.c and index-pack.c
> >
> > Shawn, Nico, what do you think? Wouldn't it be better to
> > refactor them first, independent of Dana's series?
>
> Probably, yes. But probably not in sha1_file.c though. This file is
> getting a bit large already, and it deals with pack reading only not
> pack writing.
>
> I think another file with common pack writing functions could be
> created. Pack index writing is another item that is currently
> duplicated in pack-objects and index-pack for example.
I agree entirely. And I'd like to see that refactoring occur
before this series, or as part of it. At least for the nr_objects
correction routine. To be honest we should have done that when
fast-import.c and index-pack.c both needed that logic, but we didn't.
I don't remember whose version showed up first in Junio's tree
(I think it was index-pack.c) but the other one (probably me with
fast-import.c) should have done the refactoring then.
We already have *waaay* too many functions that know packfile
structure. I'd like to see that decline, but unfortunately a number
of them are using rather specialized data structures so it makes
things somewhat difficult.
For example, writing objects to a packfile: we have 3
implementations. fast-import.c doesn't use sha1write_compressed
because that was a waste of time to compute the SHA_CTX when we
know we have to go back and fixup nr_objects. It also doesn't use
it because fast-import.c's pack-splitting logic is based on the
final object size, not the starting offset. It does the deflate
itself, decides if the end of the object will overflow, and if so,
jumps to a new packfile.
--
Shawn.
next prev parent reply other threads:[~2007-04-09 17:39 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-04-08 23:22 [PATCH 4/8] git-repack --max-pack-size: add fixup_header_footer() Dana How
2007-04-09 0:04 ` Junio C Hamano
2007-04-09 0:18 ` Nicolas Pitre
2007-04-09 17:38 ` Shawn O. Pearce [this message]
2007-04-09 18:30 ` Nicolas Pitre
2007-04-09 18:40 ` Shawn O. Pearce
2007-04-09 19:11 ` Dana How
2007-04-09 19:33 ` Nicolas Pitre
2007-04-09 21:38 ` Dana How
2007-04-09 23:22 ` Nicolas Pitre
-- strict thread matches above, loose matches on Subject: below --
2007-04-30 23:21 Dana How
2007-05-01 5:06 ` Shawn O. Pearce
2007-05-01 5:41 ` Dana How
2007-05-01 6:03 ` Shawn O. Pearce
2007-05-01 7:32 ` Johannes Schindelin
2007-05-01 17:48 ` Nicolas Pitre
2007-05-01 17:58 ` Dana How
2007-05-01 18:39 ` Nicolas Pitre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20070409173858.GT5436@spearce.org \
--to=spearce@spearce$(echo .)org \
--cc=danahow@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=junkio@cox$(echo .)net \
--cc=nico@cam$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox