Re: [RFC] git-pack-refs --prune

From: Junio C Hamano <junkio@cox.net>
Date: Sep 22, 2006, 04:57 UTC
Message-ID: <7vzmcs5wh9.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <Pine.LNX.4.64.0609210815100.4388@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

Show 9 quoted lines

> On Thu, 21 Sep 2006, Junio C Hamano wrote:
>> 
>> Ok, so I did these and the result is a 4-patch series.
>
> Looks good to me. I would have combined 2/3, since the bulk of them is the 
> calling conversion change, and they both add a new argument to the same 
> function, so combining them would make just one patch that isn't even 
> noticeably larger than either of the two originals, but that's just a 
> small nitpick.

Actually I did these two as a single change and then later split them. Unfortunately cherry-picking only 3 without 2 would not have worked anyway in this particular case with git, but an ideal SCM would have done a better job. It's just taste and principle and splitting them into two would not have any practical advantage in the real world.

Side note to interested bystanders.

Patch 2 changes function signature of for_each_ref() and friends and adds one parameter, void *cbdata, to them and the called-back functions. Typical changes looked like these:

+typedef int each_ref_fn(const char *refname, const unsigned char *sha1, void *cb_data); -static int name_ref(const char *path, const unsigned char *sha1) +static int name_ref(const char *path, const unsigned char *sha1, void *cb_data) - for_each_ref(name_ref); + for_each_ref(name_ref, &tags_only);

Then patch 3 adds another parameter, int flags, to the same functions involved:

-typedef int each_ref_fn(const char *refname, const unsigned char *sha1, void *cb_data); +#define REF_ISSYMREF 01 +#define REF_ISPACKED 02 +typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int flags, void *cb_data); -static int name_ref(const char *path, const unsigned char *sha1, void *cb_data) +static int name_ref(const char *path, const unsigned char *sha1, int flags, void *cb_data)

An ideal SCM should help the user who wants to cherry pick patch 3 without patch 2, effectively applying a patch like this:

* Type definition of for_each_ref() callback function (with
  flags but without callback data):

+#define REF_ISSYMREF 01 +#define REF_ISPACKED 02 +typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int flags);

* An actual callback function:

-static int name_ref(const char *path, const unsigned char *sha1) +static int name_ref(const char *path, const unsigned char *sha1, int flags)

* The call-site of for_each_ref (note patch2 and patch2+patch3
  have the same effect):

- for_each_ref(name_ref); + for_each_ref(name_ref, &tags_only);