Subject: Re: [PATCH 0/6] "git repack -a -d" improvements

From: Kjetil Barvik <[email-masked]>
Date: Sat, 28 Feb 2009 13:29:12 +0100

* Junio C Hamano <[email-masked]> writes:
| [2/6] refactors public interface has_sha1_pack() that takes an optional
| "ignore_packed" list.  Most callers pass NULL, so it introduces a new
| function has_sha1_kept_pack() and migrate the minority caller to this
| interface while losing the argument from the original function and callers
| that currently pass NULL.
  [...]
| [4/6] identifies three places that use "ignore_packed" list to tell if a
| pack is on the list or not, and introduces a helper function to do so.
| The helper is conveniently called is_kept_pack(), even though at this
| stage the list does not necessarily mean a list of "unkept" packs yet.

  OK, patch 2/6 failed for me when I was doing 'git am' to import the
  patch-series, so sorry if do not see all bits of the patch correctly.

  Would it be an improvment to change the signature of the currently
  find_sha1_pack() function to:

    struct packed_git *
    find_pack_entry(const unsigned char *sha1, off_t *sha1_pack_offset,
                    struct packed_git *packs)

    - The currently existing 'struct pack_entry *e' parameter is only
      used to retrn the offset, so make it more clear.  The struct
      pack_entry can probably be deleted from the sha1_file.c file.

    - When the 'git repack -a -d' command is used, one has to compute
      the list of allowed pack-files to look into, and give this list to
      find_pack_entry().

    - The currently named find_sha1_pack() function can then be deleted.

    - For example, when this function is now used in sha1_object_info()
      it can be called like this:

          found_pack = find_pack_entry(sha1, &offset, packed_git);

  -- kjetil