git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* git-name-rev off-by-one bug
@ 2005-11-28 23:42 linux
  2005-11-29  5:54 ` Junio C Hamano
  2005-12-01 10:14 ` Junio C Hamano
  0 siblings, 2 replies; 88+ messages in thread
From: linux @ 2005-11-28 23:42 UTC (permalink / raw)
  To: git; +Cc: linux

I've been trying to wrap my head around git for a while now, and finding
things a bit confusing.  Basically, the reason that I'm scared to trust
it with my code is that all sharing is done via push and pull, and they
are done by merging, and merging isn't described very well anywhere.

There's lots of intimate *detail* of merge algorithms (hiding in, of all
places, the git-read-tree documentation, which is not the obvious place
for a beginner to look), but the important high-level questions like "what
happens to all my hard work if there's a merge conflict?" or "what if I
forget to git-update-index before doing the merge?" are not really clear.
I don't like to go ahead if I'm not confident I can get back.

(Being able to back up the object database is obviously simple, but what
happens if the index holds HEAD+1, the working directory holds HEAD+2,
and I try to mere the latest changes from origin?  Are either HEAD+1 or
HEAD+2 in danger of being lost, or will checking them in later overwrite
the merge, or what?)

Anyway, I'm doing some experiments and trying to understand it, and writing
what I learn as I go, which will hopefully be useful to someone.


Another very confusing thing is the ref syntax with all those ~12^3^22^2
suffixes.  The git tutorial uses "master^" and "master^2" syntax, but
doesn't actually explain it.

The meaning can be found on the second page of the git-rev-parse manual.
If, that is, you think to read that man page, and if you don't stop
reading after the first page tells you that it's a helper for scripts
not meant to be invoked directly by the end-user.

Trying to see if I understood what was going on, I picked a random rev out of
git-show-branch output and tried git-name-rev:

> $ git-name-rev 365a00a3f280f8697e4735e1ac5b42a1c50f7887
> 365a00a3f280f8697e4735e1ac5b42a1c50f7887 maint~404^1~7

(If you care, maint=93dcab2937624ebb97f91807576cddb242a55a46)

And was very confused when git-rev-parse didn't invert the operation:

> $ git-rev-parse maint~404^1~7
> f69714c38c6f3296a4bfba0d057e0f1605373f49

I spent a while verifying that I understood that ^1 == ^ == ~1, so
~404^1~7 = ~412, and that gave the same unwanted result:

> $ git-rev-parse maint~412
> f69714c38c6f3296a4bfba0d057e0f1605373f49

After confusing myself for a while, I looked to see why git-name-rev
would output such a redundant name and found that it was simply
wrong.  Fixing the symbolic name worked:

> $ git-rev-parse maint~404^2~7
> 365a00a3f280f8697e4735e1ac5b42a1c50f7887

You can either go with a minimal fix:
diff --git a/name-rev.c b/name-rev.c
index 7d89401..f7fa18c 100644
--- a/name-rev.c
+++ b/name-rev.c
@@ -61,9 +61,10 @@ copy_data:
 
 			if (generation > 0)
 				sprintf(new_name, "%s~%d^%d", tip_name,
-						generation, parent_number);
+						generation, parent_number+1);
 			else
-				sprintf(new_name, "%s^%d", tip_name, parent_number);
+				sprintf(new_name, "%s^%d", tip_name,
+						parent_number+1);
 
 			name_rev(parents->item, new_name,
 				merge_traversals + 1 , 0, 0);


Or you can get a bit more ambitious and write ~1 as ^:

diff --git a/name-rev.c b/name-rev.c
index 7d89401..82053c8 100644
--- a/name-rev.c
+++ b/name-rev.c
@@ -57,13 +57,17 @@ copy_data:
 			parents;
 			parents = parents->next, parent_number++) {
 		if (parent_number > 0) {
-			char *new_name = xmalloc(strlen(tip_name)+8);
+			unsigned const len = strlen(tip_name);
+			char *new_name = xmalloc(len+8);
 
-			if (generation > 0)
-				sprintf(new_name, "%s~%d^%d", tip_name,
-						generation, parent_number);
-			else
-				sprintf(new_name, "%s^%d", tip_name, parent_number);
+			memcpy(new_name, tip_name, len);
+
+			if (generation == 1)
+				new_name[len++] = '^';
+			else if (generation > 1)
+				len += sprintf(new_name+len, "~%d", generation);
+
+			sprintf(new_name+len, "^%d", parent_number+1);
 
 			name_rev(parents->item, new_name,
 				merge_traversals + 1 , 0, 0);


While I'm at it, I notice some unnecessary invocations of expr in some
of the shell scripts.  You can do it far more simply using the ${var#pat}
and ${var%pat} expansions to strip off leading and trailing patterns.
For example:

diff --git a/git-cherry.sh b/git-cherry.sh
index 867522b..c653a6a 100755
--- a/git-cherry.sh
+++ b/git-cherry.sh
@@ -23,8 +23,7 @@ case "$1" in -v) verbose=t; shift ;; esa
 
 case "$#,$1" in
 1,*..*)
-    upstream=$(expr "$1" : '\(.*\)\.\.') ours=$(expr "$1" : '.*\.\.\(.*\)$')
-    set x "$upstream" "$ours"
+    set x "${1%..*}" "${1#*..}"
     shift ;;
 esac
 
This works in dash and is in the POSIX spec.  It doesn't work in some
very old /bin/sh implementations (such as Solaris still ships), but I'm
pretty sure it was introduced at the same time as $(), and the scripts
use *that* all over the place.

% sh
$ uname -s -r
SunOS 5.9
$ foo=bar
$ echo ${foo#b}
bad substitution
$ echo `echo $foo`
bar
$ echo $(echo $foo)
syntax error: `(' unexpected

Anyway, if it's portable enough, it's faster.  Ah... I just found discussion
of this in late September, but it's not clear what the resolution was.
http://marc.theaimsgroup.com/?t=112746188000003


(Oh, yes: all of the above patches are released into the public domain.
Copyright abandoned.  Have fun.)

^ permalink raw reply related	[flat|nested] 88+ messages in thread
[parent not found: <7vbqzrcmgr.fsf@assigned-by-dhcp.cox.net>]

end of thread, other threads:[~2005-12-13 13:58 UTC | newest]

Thread overview: 88+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-11-28 23:42 git-name-rev off-by-one bug linux
2005-11-29  5:54 ` Junio C Hamano
2005-11-29  8:05   ` linux
2005-11-29  9:29     ` Junio C Hamano
2005-11-30  8:37       ` Junio C Hamano
2005-11-29 10:31     ` Petr Baudis
2005-11-29 18:46       ` Junio C Hamano
2005-12-04 21:34         ` Petr Baudis
2005-12-08  6:34           ` as promised, docs: git for the confused linux
2005-12-08 21:53             ` Junio C Hamano
2005-12-08 22:02               ` H. Peter Anvin
2005-12-09  0:47             ` Alan Chandler
2005-12-09  1:45               ` Petr Baudis
2005-12-09  1:19             ` Josef Weidendorfer
2005-11-29 21:40       ` git-name-rev off-by-one bug linux
2005-11-29 23:14         ` Junio C Hamano
2005-11-30  0:15           ` linux
2005-11-30  0:53             ` Junio C Hamano
2005-11-30  1:27               ` Junio C Hamano
2005-11-30  1:51             ` Linus Torvalds
2005-11-30  2:06               ` Junio C Hamano
2005-11-30  2:33               ` Junio C Hamano
2005-11-30  3:12                 ` Linus Torvalds
2005-11-30  5:06                   ` Linus Torvalds
2005-11-30  5:51                     ` Junio C Hamano
2005-11-30  6:11                       ` Junio C Hamano
2005-11-30 16:13                         ` Linus Torvalds
2005-11-30 16:08                       ` Linus Torvalds
2005-12-02  8:25                       ` Junio C Hamano
2005-12-02  9:14                         ` [PATCH] merge-one-file: make sure we create the merged file Junio C Hamano
2005-12-02  9:15                         ` [PATCH] merge-one-file: make sure we do not mismerge symbolic links Junio C Hamano
2005-12-02  9:16                         ` [PATCH] git-merge documentation: conflicting merge leaves higher stages in index Junio C Hamano
2005-11-30  6:09                     ` git-name-rev off-by-one bug linux
2005-11-30  6:39                       ` Junio C Hamano
2005-11-30 13:10                         ` More merge questions linux
2005-11-30 18:37                           ` Daniel Barkalow
2005-11-30 20:23                           ` Junio C Hamano
2005-12-02  9:19                             ` More merge questions (why doesn't this work?) linux
2005-12-02 10:12                               ` Junio C Hamano
2005-12-02 13:09                                 ` Sven Verdoolaege
2005-12-02 20:32                                   ` Junio C Hamano
2005-12-05 15:01                                     ` Sven Verdoolaege
2005-12-02 11:37                               ` linux
2005-12-02 20:31                                 ` Junio C Hamano
2005-12-02 21:32                                   ` linux
2005-12-02 22:00                                     ` Junio C Hamano
2005-12-02 22:12                                     ` Linus Torvalds
2005-12-02 23:14                                       ` linux
2005-12-02 21:56                                   ` More merge questions linux
2005-11-30 16:12                       ` git-name-rev off-by-one bug Linus Torvalds
2005-11-30  7:18                   ` Junio C Hamano
2005-11-30  9:05                     ` Junio C Hamano
2005-11-30  9:42                     ` Junio C Hamano
2005-11-30  3:15                 ` linux
2005-11-30 18:11               ` Daniel Barkalow
2005-11-30 17:46   ` Daniel Barkalow
2005-11-30 20:05     ` Junio C Hamano
2005-11-30 21:06       ` Daniel Barkalow
2005-11-30 22:00         ` Junio C Hamano
2005-11-30 23:12           ` Daniel Barkalow
2005-12-01  7:46             ` Junio C Hamano
2005-12-01 10:14 ` Junio C Hamano
2005-12-01 21:50   ` Petr Baudis
2005-12-01 21:53     ` Randal L. Schwartz
     [not found] <7vbqzrcmgr.fsf@assigned-by-dhcp.cox.net>
2005-12-09  5:43 ` as promised, docs: git for the confused linux
2005-12-09  9:43   ` Petr Baudis
2005-12-09 14:01     ` linux
2005-12-09 16:49       ` Randy.Dunlap
2005-12-09 19:12       ` Junio C Hamano
2005-12-09 21:54         ` linux
2005-12-09 23:23           ` Junio C Hamano
2005-12-12 16:34             ` Linus Torvalds
2005-12-12 17:53               ` Timo Hirvonen
2005-12-12 18:18                 ` Linus Torvalds
2005-12-12 20:39                   ` Randal L. Schwartz
2005-12-13  3:58                     ` Joshua N Pritikin
2005-12-13  3:59                       ` Randal L. Schwartz
2005-12-13  5:19                         ` Junio C Hamano
2005-12-13  5:29                           ` Linus Torvalds
2005-12-13  7:18                             ` H. Peter Anvin
2005-12-13  8:01                           ` Junio C Hamano
2005-12-13 13:58                             ` Randal L. Schwartz
2005-12-12 17:54               ` Junio C Hamano
2005-12-09 21:33       ` Petr Baudis
2005-12-09  5:44 ` linux
2005-12-10  1:22   ` Junio C Hamano
2005-12-10  8:00   ` Junio C Hamano
2005-12-10 10:56     ` linux

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).