Re: [PATCH v6 25/25] refs: break out ref conflict checks

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> Junio, if there are no more comments, would you mind
>>
>>     s/verify_no_descendants/find_descendant_ref/
>>
>> in the log message of this commit? And then, if you are also OK with the
>> new subdirectory introduced in this patch series, David and I seem to be
>> in agreement that it is ready to go. It would be great if this patch
>> series could be merged in a timely manner, as it will conflict with
>> nearly any other changes that people might want to undertake in the refs
>> code.
>
> Thanks for working well together.  Let me see what I can do today...

What I'll push out later today merges this to the tip of 'pu'.
The resolution is the same for 'jch' or 'next' (I checked).

I have to say that the merge of this topioc is not pretty.  A topic
that is already in flight has changed ref_is_hidden() in refs.c; you
move this block of code first to refs/refs-backend.c and then to
refs/refs.c, and the recursive merge ends up saying "The trunk side
changed this block of code in refs/refs-backend.c while the side
branch removed that block".

The resolution has to become an evil merge that brings in a new file
refs/refs.c from the tip of your topic to the trunk while replaying
that change in the lost block to that new file.  Because an
in-flight topic like this one needs to be merged over and over until
it gets merged to 'master' I'd prepare an evil merge-fix to be
squashed into the result of an auto-merge to help this process for
the interim maintainer, but this topic is placing more burden than
it otherwise would to the entire process.

Incidentally, that was why I originally asked you if you want to be
an interim maintainer for this cycle.  Whoever is doing a large code
movement with a large patch series must be the one who would know
the best how its interaction with other topics is best managed.

I wonder if refs.c -> refs/refs.c rename is a good idea.  I do agree
that refs/ subdirectory that collects the backend implementation
details is a very sensible thing to do, but if the public and
generic API were left in refs.c, this particular conflict might have
been less severe and easier to handle.  Whatever.

For those who are listening in from sideline, in case when they need
to deal with a similar situation "the code our side changed gets
moved to elsewhere by a side branch", here is what I did:

 * let "git merge --conflict=diff3" attempt and fail.

 * a conflicted file will have something like this:

    <<<< ours
    ... the code with changes made by our side (trunk) ...
    |||| base
    ... the code before our side (trunk) made the above changes ...
    ====
    >>>> theirs

 * create two new files, OURS and BASE.  Save the part in that
   conflicted file between <<<< and |||| to OURS, and between ||||
   and ==== to BASE.

 * look at "diff -u BASE OURS", find in the (failed) automerge
   result where the original went (a sample of it is at the end of
   this message), and apply that change manually.

The above is only to resolve this conflict *once*.

Automating future merges of this branch into a slightly updated
codebase needs help from rerere and also merge-fix/ machinery (this
is not in core-git proper, but the tool is in the 'todo' branch and
its use is described in howto/maintain-git.txt).

Essentially the procedure were:

 * "git checkout pu^0"

 * let "git merge --conflict=diff3" attempt and fail.

 * accept removal of the conflicted block in refs/files-backend.c in
   the editor, do "git rerere" to record it.  commit the result.

 * apply the above diff between BASE and OURS, commit the result.

 * git update-ref refs/merge-fix/dt/refs-backend-pre-vtable HEAD

With this, the Reintegrate script (on 'todo', checked out in "Meta/"
subdirectory) will be able to reproduce the evil merge, e.g.

 $ git checkout pu
 $ echo dt/refs-backend-pre-vtable | Meta/Reintegrate

would first let "git rerere" replay the removal of conflicted block
in refs/files-backend.c and then amend the result by squashing the
change in merge-fix/dt/refs-backend-pre-vtable.


--- V_BASE	2015-11-06 14:51:10.150197900 -0800
+++ V_OURS	2015-11-06 14:51:05.638059250 -0800
@@ -117,7 +117,7 @@
 	return 0;
 }
 
-int ref_is_hidden(const char *refname)
+int ref_is_hidden(const char *refname, const char *refname_full)
 {
 	int i;
 
@@ -125,6 +125,7 @@
 		return 0;
 	for (i = hide_refs->nr - 1; i >= 0; i--) {
 		const char *match = hide_refs->items[i].string;
+		const char *subject;
 		int neg = 0;
 		int len;
 
@@ -133,10 +134,18 @@
 			match++;
 		}
 
-		if (!starts_with(refname, match))
+		if (*match == '^') {
+			subject = refname_full;
+			match++;
+		} else {
+			subject = refname;
+		}
+
+		/* refname can be NULL when namespaces are used. */
+		if (!subject || !starts_with(subject, match))
 			continue;
 		len = strlen(match);
-		if (!refname[len] || refname[len] == '/')
+		if (!subject[len] || subject[len] == '/')
 			return !neg;
 	}
 	return 0;