Re: [RFC] [PATCH 0/5] Implement 'prior' commit object links (and

From: Junio C Hamano <junkio@cox.net>
Date: Apr 29, 2006, 19:30 UTC
Message-ID: <7vlkto6uym.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <e309vq$m2r$1@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> writes:

Show 5 quoted lines

> IF (and that is big if) git commit header will be extended to have some
> extra "link" (enforcing connectivity) headers, like proposed "bind" for
> subprojects, "prev" for pu-like union branches, "merge-base" for merges,
> there would be repeated work on enforcing connectivity. Hence generic
> "link" header (formerly "related") proposal.

The "link <sha1> <type> <meta>" header extension was done primarily for that reason this way. I carried it in my "pu" branch for a few days but Linus convinced me privately that it was a bad idea, so it is not merged in "pu" anymore. Just to make it easy for people to view what we are discussing, I pushed the branch head to jc/bind-2 topic branch, but the code will _not_ be merged.

The code in commit.c to recognize and link the releated objects pointed by the "link" header to the commit looked like below (see 11bbee26 commit on that branch):

+       optr = &item->links;
+       while (!memcmp(bufptr, "link ", 5)) {
+               struct object *object;
+
+               if (!get_sha1_hex(bufptr + 5, parent) &&
+                   bufptr[45] == ' ' &&
+                   (object = lookup_unknown_object(parent)) != NULL) {
+                       struct object_list *l = xmalloc(sizeof(*l));
+                       l->item = object;
+                       l->next = *optr;
+                       l->name = NULL;
+                       *optr = l;
+                       optr = &l->next;
+                       n_refs++;
+                       bufptr += 45;
+               }
+               else
+                       return error("bad link in commit %s",
+                                    sha1_to_hex(item->object.sha1));
+               while (*bufptr++ != '\n')
+                       ; /* skip over subdirectory name */
+       }

But if your are going to introduce "merge-base" and similar headers that have impact to connectivity traversal code, you can easily change the !memcmp(buptr, "link ", 5) with a sequence of "memcmp(foo) || memcmp(bar) || ...", and use the "l->name" field to point at the header itself, so that the user of the resulting commit object can easily tell what kind of link-like header it is, and enforce further semantics that are specific to each kind of such header on it. The revision traversal change that was done in a later commit (7091fd commit) does not have to change.

The code sharing aspect you brought up is a very important issue. This is revision traversal, which is really the central part of git and needs deep thought to touch without breaking, so we would like to avoid risking breaking it by repeatedly touching it. But that can be done without making the recorded header something like "link <sha1> <type> <metainfo>" which is too generic.