From: Dana How <danahow@gmail.com>
Date: Tue, 01 May 2007 06:15:09 GMT
Subject: Re: [PATCH 8/8] git-repack --max-pack-size: add option parsing to enable feature
Message-ID: <56b7f5510704302315v3d985dcfodb48fcd0bf49853f@mail.gmail.com>
In-Reply-To: <20070501054528.GC5942@spearce.org>

On 4/30/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> Dana How <danahow@gmail.com> wrote:
> > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> ...
> > +             if (!prefixcmp(arg, "--max-pack-size=")) {
> > +                     char *end;
> > +                     pack_size_limit = strtoul(arg+16, &end, 0) * 1024 * 1024;
> > +                     if (!arg[16] || *end)
> > +                             usage(pack_usage);
> > +                     no_reuse_delta = 1;
>
> Wow, are you serious?  This entire change is about making repack
> automatically split large projects into multiple packfiles.  If
> that happens are you intending that the caller will mark all of
> those packfiles with .keep files immediately after repacking them?
> If you want users to create .keep files, can git-repack.sh do that
> for them when more than one packfile was output?
>
> Because otherwise a "quick" git-gc will not be quick because the
> reuse delta feature (which is a massive performance improvement for
> repack/gc) will always be disabled.  But odds are a future repack
> of the same project will generally keep things that are in the
> same packfile already in the same packfile again, so delta reuse is
> actually possible for most objects.  I think you should find a way
> to make this change work without needing to force no_reuse_delta
> just because this limit was added.
In a previous version of this patchset I explained that I had turned
on no_reuse_delta because I hadn't yet verified all the combinations
in the reuse codepath(s).  I guess I'll be reinspecting them sooner
rather than later.

> > diff --git a/git-repack.sh b/git-repack.sh
> ...
> > +names=$(git-pack-objects --non-empty --all --reflog $args </dev/null "$PACKTMP") ||
> >       exit 1
> > -if [ -z "$name" ]; then
> > +if [ -z "$names" ]; then
> >       echo Nothing new to pack.
> > -else
> > +fi
> > +for name in $names ; do
>
> I think this particular change needs to either preceed the prior
> commit, or be part of it.  If someone tries to bisect this patch
> series with a large enough project that multiple packfiles are being
> produced then you run into some ugly problems in this repack script.
Sorry, you lost me.  git is being bisected, or a project managed by git?
My intent was that the pack splitting wouldn't be available until
_all_ patches were applied (active).  Bisecting git _within_ this patchset
would still be useful -- it could be used to isolate where some of
this new code broke some pre-existing feature.

Thanks,
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell