Re: fetching packs and storing them as packs

[Shawn Pearce <spearce@spearce.org>]

Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> 
> > Shawn Pearce <spearce@spearce.org> wrote:
> >> Why not just use create a new flag file?
> >> 
> >> Lets say that a pack X is NOT eligible to be repacked if
> >> "$GIT_DIR/objects/pack/pack-X.keep" exists.
> >
> > Here's the `git repack -a -d` portion of that.
> > Thoughts?
> 
> > +	args=--unpacked
> > +	active=
> > +	if test -d "$PACKDIR"
> > +	then
> > +		for p in `find "$PACKDIR" -type f -name '*.pack' -print`
> 
> This change to run 'find "$PACKDIR"' is fragile when your
> $GIT_OBJECT_DIRECTORY has $IFS in it; running "find ." after
> "cd" in a subprocess was done very much on purpose to avoid that
> issue.  Please don't break it.

I only broke it because you backed me into a corner with --unpacked=
:-)

The issue is --unpacked= uses the path of the pack name, which
includes $GIT_OBJECT_DIRECTORY, whatever that may be.  This makes it
impossible for the shell script to hand through a proper --unpacked=
line for the active packs without including $GIT_OBJECT_DIRECTORY
as part of the option.

I agree with you about the $IFS issue.  I'll redraft this patch
tonight such that $IFS doesn't get broken here but that's going
to take a small code patch over in revisions.c, which I'll also
do tonight.

> > +	if test "X$args" = X--unpacked
> > +	then
> > +		args='--unpacked --incremental'
> > +	fi
 
> I do not remember offhand what --incremental meant, but
> presumably this is for the very initial "repack" (PACKDIR did
> not exist or find loop did not find anything to repack) and the
> flag would not make a difference?  Care to explain?
 
I think there is a bug in pack-objects but I couldn't find it last
night.  Using --incremental worked around it.  :-)

According to the documentation:

  --unpacked tells pack-objects to only pack loose objects.

  --incremental tells pack-objects to skip any object that is
  already contained in a pack even if it appears in the input list.

What I really wanted here was to just use '--unpacked'; if there
are no active packs and the user asked for '-a' we actually just
want to pack the loose objects into a new active pack as we aren't
allowed to touch any of the existing packs (they are all kept or
there simply aren't any packs yet).

However on the git.git repository if I ran `git repack -a -d`
with every single object in a kept pack and no loose objects I kept
repacking the same 102 objects into a new active pack, even though
there were no loose objects to repack and no active packs.  Uh, yea.

Adding --incremental in this case kept it from repacking those
same 102 objects on every invocation.  Yea, its a bug.  I meant to
mention it in my email.

--