rev-parse: be more careful with munging arguments

peff · gitster · commit 62f162f8e7c2 · 2013-12-09T14:39:16.000-08:00
When rev-parse looks at whether an argument like "foo..bar" or
"foobar^@" is a difference or parent-shorthand, it internally
munges the arguments so that it can pass the individual rev
arguments to get_sha1(). However, we do not consistently un-munge
the result.

For cases where we do not match (e.g., "doesnotexist..HEAD"), we
would then want to try to treat the argument as a filename.
try_difference gets() this right, and always unmunges in this case.
However, try_parent_shorthand() never unmunges, leading to incorrect
error messages, or even incorrect results:

  $ git rev-parse foobar^@
  foobar
  fatal: ambiguous argument 'foobar': unknown revision or path not in the working tree.
  Use '--' to separate paths from revisions, like this:
  'git &lt;command&gt; [&lt;revision&gt;...] -- [&lt;file&gt;...]'

  $ &gt;foobar
  $ git rev-parse foobar^@
  foobar

For cases where we do match, neither function unmunges. This does
not currently matter, since we are done with the argument. However,
a future patch will do further processing, and this prepares for
it. In addition, it's simply a confusing interface for some cases to
modify the const argument, and others not to.

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Reviewed-by: Jonathan Nieder &lt;jrnieder@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
@@ -279,6 +279,7 @@ static int try_difference(const char *arg)
 				exclude = n;
 			}
 		}
+		*dotdot = '.';
 		return 1;
 	}
 	*dotdot = '.';
@@ -302,8 +303,10 @@ static int try_parent_shorthands(const char *arg)
 		return 0;
 
 	*dotdot = 0;
-	if (get_sha1_committish(arg, sha1))
+	if (get_sha1_committish(arg, sha1)) {
+		*dotdot = '^';
 		return 0;
+	}
 
 	if (!parents_only)
 		show_rev(NORMAL, sha1, arg);
@@ -312,6 +315,7 @@ static int try_parent_shorthands(const char *arg)
 		show_rev(parents_only ? NORMAL : REVERSED,
 				parents->item->object.sha1, arg);
 
+	*dotdot = '^';
 	return 1;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -279,6 +279,7 @@ static int try_difference(const char *arg)`
`279`	`279`	`exclude = n;`
`280`	`280`	`}`
`281`	`281`	`}`
	`282`	`+ *dotdot = '.';`
`282`	`283`	`return 1;`
`283`	`284`	`}`
`284`	`285`	`*dotdot = '.';`
`@@ -302,8 +303,10 @@ static int try_parent_shorthands(const char *arg)`
`302`	`303`	`return 0;`
`303`	`304`
`304`	`305`	`*dotdot = 0;`
`305`		`- if (get_sha1_committish(arg, sha1))`
	`306`	`+ if (get_sha1_committish(arg, sha1)) {`
	`307`	`+ *dotdot = '^';`
`306`	`308`	`return 0;`
	`309`	`+ }`
`307`	`310`
`308`	`311`	`if (!parents_only)`
`309`	`312`	`show_rev(NORMAL, sha1, arg);`
`@@ -312,6 +315,7 @@ static int try_parent_shorthands(const char *arg)`
`312`	`315`	`show_rev(parents_only ? NORMAL : REVERSED,`
`313`	`316`	`parents->item->object.sha1, arg);`
`314`	`317`
	`318`	`+ *dotdot = '^';`
`315`	`319`	`return 1;`
`316`	`320`	`}`
`317`	`321`