git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] git-mergetool: show original branch names when possible
@ 2007-08-20  7:53 Jeff King
  2007-08-20  8:28 ` Junio C Hamano
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff King @ 2007-08-20  7:53 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Theodore Ts'o

The actual branch names are used when naming temporary files
and when presenting the files to the user, instead of
calling them "local" and "remote".

The "local" branch name is gathered by dereferencing HEAD.

The "remote" branch name is placed in MERGE_HEAD by
git-merge.

Signed-off-by: Jeff King <peff@peff.net>
---
I have several uncertainties in the implementation:

1. Is it OK to place the extra branch name information in MERGE_HEAD
after the SHA1?

2. It looks like doing an anonymous 'git-pull' leaves GITHEAD_* as the
commit sha1, which means you will end up with that sha1 rather than
'REMOTE', which is less nice than the current behavior. However, I think
that is fixable at the 'git-merge' level to produce a nicer name for the
remote.

3. This is meant to work similar to the GITHEAD_* feature, but needs to
work without using the environment. Many other scripts use GITHEAD_*,
but not MERGE_HEAD, so perhaps MERGE_HEAD is not the best place for
this.

It would be _really_ convenient in this case if we had a "git is in the
middle of something" file, which has been discussed before. Clearly
there are some operations that persist across multiple command
invocations, and it would be nice rather than every command knowing
about every other command's implementation patterns ("Oh, you have a
.dotest file? You must be in the middle of...") to have a single place
with something like:

  $ cat .git/STATE
  operation: merge
  remote: git://git.kernel.org/pub/scm/git/git.git
  branch: master
  branch: octopus

  $ cat .git/STATE
  operation: rebase
  upstream: origin/master
  onto: HEAD~20

where you could have arbitrary keys through which to communicate. Most
command would just look at 'operation' (e.g., if operation == merge,
don't start a new merge), but some could use the information (which
would otherwise be lost) to work more usefully (e.g., git-mergetool
understands remote and branch keys created by git-merge).

Thoughts?

 git-merge.sh     |    2 +-
 git-mergetool.sh |   24 ++++++++++++++++--------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/git-merge.sh b/git-merge.sh
index 5ccf282..e899801 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -477,7 +477,7 @@ then
 else
 	for remote
 	do
-		echo $remote
+		echo $remote $(eval echo \$GITHEAD_$remote)
 	done >"$GIT_DIR/MERGE_HEAD"
 	printf '%s\n' "$merge_msg" >"$GIT_DIR/MERGE_MSG"
 fi
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 47a8055..8bc0a66 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -151,8 +151,16 @@ merge_file () {
     fi
 
     BACKUP="$path.BACKUP.$$"
-    LOCAL="$path.LOCAL.$$"
-    REMOTE="$path.REMOTE.$$"
+    local_branch=$(
+      git-symbolic-ref HEAD 2>/dev/null \
+        | sed -e 's#^refs/[^/]*/##' \
+        | sed -e 's/[^A-Za-z0-9]/-/g')
+    LOCAL="$path.${local_branch:-HEAD}.$$"
+    remote_branch=$(
+      sed -ne '1s/^[0-9a-f]* //p' \
+        <"$GIT_DIR/MERGE_HEAD" \
+        | sed -e 's/[^A-Za-z0-9]/-/g')
+    REMOTE="$path.${remote_branch:-REMOTE}.$$"
     BASE="$path.BASE.$$"
 
     mv -- "$path" "$BACKUP"
@@ -168,23 +176,23 @@ merge_file () {
 
     if test -z "$local_mode" -o -z "$remote_mode"; then
 	echo "Deleted merge conflict for '$path':"
-	describe_file "$local_mode" "local" "$LOCAL"
-	describe_file "$remote_mode" "remote" "$REMOTE"
+	describe_file "$local_mode" "${local_branch:-HEAD}" "$LOCAL"
+	describe_file "$remote_mode" "${remote_branch:-remote}" "$REMOTE"
 	resolve_deleted_merge
 	return
     fi
 
     if is_symlink "$local_mode" || is_symlink "$remote_mode"; then
 	echo "Symbolic link merge conflict for '$path':"
-	describe_file "$local_mode" "local" "$LOCAL"
-	describe_file "$remote_mode" "remote" "$REMOTE"
+	describe_file "$local_mode" "${local_branch:-HEAD}" "$LOCAL"
+	describe_file "$remote_mode" "${remote_branch:-remote}" "$REMOTE"
 	resolve_symlink_merge
 	return
     fi
 
     echo "Normal merge conflict for '$path':"
-    describe_file "$local_mode" "local" "$LOCAL"
-    describe_file "$remote_mode" "remote" "$REMOTE"
+    describe_file "$local_mode" "${local_branch:-HEAD}" "$LOCAL"
+    describe_file "$remote_mode" "${remote_branch:-remote}" "$REMOTE"
     printf "Hit return to start merge resolution tool (%s): " "$merge_tool"
     read ans
 
-- 
1.5.3.rc5.844.g67b3-dirty

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC] git-mergetool: show original branch names when possible
  2007-08-20  7:53 [RFC] git-mergetool: show original branch names when possible Jeff King
@ 2007-08-20  8:28 ` Junio C Hamano
  2007-08-20  8:52   ` Jeff King
  0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2007-08-20  8:28 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Theodore Ts'o

Jeff King <peff@peff.net> writes:

> 1. Is it OK to place the extra branch name information in MERGE_HEAD
> after the SHA1?

I do not think of anything that would barf offhand (we already
do that in FETCH_HEAD), but this would definitely be carefully
audited.

> 2. It looks like doing an anonymous 'git-pull' leaves GITHEAD_* as the
> commit sha1, which means you will end up with that sha1 rather than
> 'REMOTE', which is less nice than the current behavior.

Much less nice indeed.

> It would be _really_ convenient in this case if we had a "git is in the
> middle of something" file, which has been discussed before.
> ...
> there are some operations that persist across multiple command
> invocations, and it would be nice rather than every command knowing
> about every other command's implementation patterns ("Oh, you have a
> .dotest file? You must be in the middle of...") to have a single place
> with something like:
>
>   $ cat .git/STATE
>   operation: merge
>   remote: git://git.kernel.org/pub/scm/git/git.git
>   branch: master
>   branch: octopus

It would be very nice, and I would encourage any wannabe
Porcelain writers to go wild on this.  One worry I have is if we
would need to support nested states.  "I was in the middle of
'foo' and then had to go sideways to do 'bar' which I am now in
the middle of" kind of thing.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] git-mergetool: show original branch names when possible
  2007-08-20  8:28 ` Junio C Hamano
@ 2007-08-20  8:52   ` Jeff King
  2007-08-20 18:17     ` Jan Hudec
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff King @ 2007-08-20  8:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Theodore Ts'o

On Mon, Aug 20, 2007 at 01:28:31AM -0700, Junio C Hamano wrote:

> I do not think of anything that would barf offhand (we already
> do that in FETCH_HEAD), but this would definitely be carefully
> audited.

I didn't say it up front, but I think this is definitely post-1.5.3
material. :)

> > 2. It looks like doing an anonymous 'git-pull' leaves GITHEAD_* as the
> > commit sha1, which means you will end up with that sha1 rather than
> > 'REMOTE', which is less nice than the current behavior.
> 
> Much less nice indeed.

I think this is a failing of git-merge, though, for not including that
nice human-readable information. We can fix it with something like this:

-- >8 --

diff --git a/git-merge.sh b/git-merge.sh
index e899801..742e15d 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -477,7 +477,14 @@ then
 else
 	for remote
 	do
-		echo $remote $(eval echo \$GITHEAD_$remote)
+		friendly_name=$(eval echo \$GITHEAD_$remote)
+		if echo $friendly_name | egrep -q '[0-9a-f]{40}'; then
+			friendly_name=$(
+			  sed -ne "s/$friendly_name	//p" \
+			  <"$GIT_DIR/FETCH_HEAD" 2>/dev/null
+			)
+		fi
+		echo $remote "$friendly_name"
 	done >"$GIT_DIR/MERGE_HEAD"
 	printf '%s\n' "$merge_msg" >"$GIT_DIR/MERGE_MSG"
 fi

-- 8< --

But probably it should wait for a better communications channel for
cross-command state.

> It would be very nice, and I would encourage any wannabe
> Porcelain writers to go wild on this.  One worry I have is if we
> would need to support nested states.  "I was in the middle of
> 'foo' and then had to go sideways to do 'bar' which I am now in
> the middle of" kind of thing.

It's just a stack, so I think you could implement it as a linked
list, with STATE being the head of the list, and each STATE file you
write pointing to the previous one (which you rename when pushing).

I might try to work on this, but definitely not tonight.

-Peff

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC] git-mergetool: show original branch names when possible
  2007-08-20  8:52   ` Jeff King
@ 2007-08-20 18:17     ` Jan Hudec
  2007-08-21  6:10       ` Jeff King
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Hudec @ 2007-08-20 18:17 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Theodore Ts'o

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

On Mon, Aug 20, 2007 at 04:52:47 -0400, Jeff King wrote:
> On Mon, Aug 20, 2007 at 01:28:31AM -0700, Junio C Hamano wrote:
> > > 2. It looks like doing an anonymous 'git-pull' leaves GITHEAD_* as the
> > > commit sha1, which means you will end up with that sha1 rather than
> > > 'REMOTE', which is less nice than the current behavior.
> > 
> > Much less nice indeed.
> 
> I think this is a failing of git-merge, though, for not including that
> nice human-readable information. We can fix it with something like this:

Maybe you could call git-name-rev on it if it does not come with
a human-readable name.

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] git-mergetool: show original branch names when possible
  2007-08-20 18:17     ` Jan Hudec
@ 2007-08-21  6:10       ` Jeff King
  2007-08-21 14:59         ` Jan Hudec
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff King @ 2007-08-21  6:10 UTC (permalink / raw)
  To: Jan Hudec; +Cc: Junio C Hamano, git, Theodore Ts'o

On Mon, Aug 20, 2007 at 08:17:25PM +0200, Jan Hudec wrote:

> > I think this is a failing of git-merge, though, for not including that
> > nice human-readable information. We can fix it with something like this:
> 
> Maybe you could call git-name-rev on it if it does not come with
> a human-readable name.

I considered that, but it has two drawbacks:

  1. It does not handle pulls which have no tracking branch (the only
     ref we have is FETCH_HEAD, which is not a useful name :) ).

  2. In some circumstances, it can come up with counter-intuitive
     names. If more than one ref points to a given commit, then you can
     end up with something like "git-merge foo" telling you all about
     the merge conflicts with "bar". But perhaps that is too obscure a
     corner case to worry about.

-Peff

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] git-mergetool: show original branch names when possible
  2007-08-21  6:10       ` Jeff King
@ 2007-08-21 14:59         ` Jan Hudec
  2007-08-21 20:55           ` Jeff King
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Hudec @ 2007-08-21 14:59 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Theodore Ts'o

[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]

On Tue, Aug 21, 2007 at 02:10:14 -0400, Jeff King wrote:
> On Mon, Aug 20, 2007 at 08:17:25PM +0200, Jan Hudec wrote:
> 
> > > I think this is a failing of git-merge, though, for not including that
> > > nice human-readable information. We can fix it with something like this:
> > 
> > Maybe you could call git-name-rev on it if it does not come with
> > a human-readable name.
> 
> I considered that, but it has two drawbacks:
> 
>   1. It does not handle pulls which have no tracking branch (the only
>      ref we have is FETCH_HEAD, which is not a useful name :) ).

If there's no useful name, than it's probably hard to do anything at all
about it. Though FETCH_HEAD is not all that useless -- it at least says it is
that that you pull.

>   2. In some circumstances, it can come up with counter-intuitive
>      names. If more than one ref points to a given commit, then you can
>      end up with something like "git-merge foo" telling you all about
>      the merge conflicts with "bar". But perhaps that is too obscure a
>      corner case to worry about.

I meant it as a fallback, for cases where it for some reason can't be
recorded or is not recorded. Recording it is obviously better.

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] git-mergetool: show original branch names when possible
  2007-08-21 14:59         ` Jan Hudec
@ 2007-08-21 20:55           ` Jeff King
  2007-08-21 21:17             ` Junio C Hamano
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff King @ 2007-08-21 20:55 UTC (permalink / raw)
  To: Jan Hudec; +Cc: Junio C Hamano, git, Theodore Ts'o

On Tue, Aug 21, 2007 at 04:59:43PM +0200, Jan Hudec wrote:

> >   1. It does not handle pulls which have no tracking branch (the only
> >      ref we have is FETCH_HEAD, which is not a useful name :) ).
> 
> If there's no useful name, than it's probably hard to do anything at all
> about it. Though FETCH_HEAD is not all that useless -- it at least says it is
> that that you pull.

But there _is_ a useful name, it's just not a ref (it's the information
from FETCH_HEAD "branch 'foo' of git://..."). In this case,
git-mergetool could pull it out of FETCH_HEAD, as well, but I feel like
we're starting to make a lot of fragile cross-tool assumptions.

> I meant it as a fallback, for cases where it for some reason can't be
> recorded or is not recorded. Recording it is obviously better.

Right, in which case I think we should just be more complete about
recording it.

-Peff

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] git-mergetool: show original branch names when possible
  2007-08-21 20:55           ` Jeff King
@ 2007-08-21 21:17             ` Junio C Hamano
  0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2007-08-21 21:17 UTC (permalink / raw)
  To: Jeff King; +Cc: Jan Hudec, git, Theodore Ts'o

Jeff King <peff@peff.net> writes:

> On Tue, Aug 21, 2007 at 04:59:43PM +0200, Jan Hudec wrote:
>
>> >   1. It does not handle pulls which have no tracking branch (the only
>> >      ref we have is FETCH_HEAD, which is not a useful name :) ).
>> 
>> If there's no useful name, than it's probably hard to do anything at all
>> about it. Though FETCH_HEAD is not all that useless -- it at least says it is
>> that that you pull.
>
> But there _is_ a useful name, it's just not a ref (it's the information
> from FETCH_HEAD "branch 'foo' of git://..."). In this case,
> git-mergetool could pull it out of FETCH_HEAD, as well, but I feel like
> we're starting to make a lot of fragile cross-tool assumptions.

Isn't the label "FETCH_HEAD" itself clear enough, without even
looking at it to see which commit it is?

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2007-08-21 21:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-20  7:53 [RFC] git-mergetool: show original branch names when possible Jeff King
2007-08-20  8:28 ` Junio C Hamano
2007-08-20  8:52   ` Jeff King
2007-08-20 18:17     ` Jan Hudec
2007-08-21  6:10       ` Jeff King
2007-08-21 14:59         ` Jan Hudec
2007-08-21 20:55           ` Jeff King
2007-08-21 21:17             ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).