009fee4 (Detailed diagnosis when parsing an object name fails., 2009-12-07)
added a logic to try to see what kind of breakage the object name
user gave in the $treeish:$path or /:$search syntax.
This logic however triggers in a funny way, leading to an idiotic
error message. You can try this in the git repository:
$ git log COPYING HEAD^:COPYING
fatal: Path 'COPYING' exists on disk, but not in 'HEAD^'.
When setup_revisions() tries to parse the command line arguments, it
notices that there is no "--", and it makes sure that earlier
arguments are all revs that cannot possibly refer to working tree
files, and later arguments all refer to working tree files and
cannot possibly refer to objects.
It first looks at "COPYING", notices that it is _not_ a rev. Which
means that anything that follows must _not_ be an object name. It
calls verify_filename() on "COPYING", which is a happy filename, and
then calls verify_filename() on "HEAD^:COPYING", and then calls
die_verify_filename() on it, assuming that it is being fed a badly
formed rev.
Obviously, this assumption is *WRONG*.
Then get_sha1_with_mode_1() is called with "gently" set to 0 (in
later code, we have a fix to flip the value of this flag and renamed
it to "only-to-die"---we are calling this function only to let it die
with diagnostics). It tries to interpret HEAD^:COPYING as a rev,
and even though get_tree_entry() call for "COPYING" inside "HEAD^"
tree-ish succeeds, it ignores the fact that it successfully names an
object, and calls diagnose_invalid_sha1_path() to die.
Side note. The original motivation of the "detailed diag"
patch was to catch something like
$ git rev-parse HEAD^:COPYIGN
and the codepath is called when the caller _knows_
HEAD^:COPYIGN must resolve to an object name, and made sure
that it does not, so in that case, it results in expected
behaviour. But for this "earlier must be all revs, later
must be all paths" caller, it was a wrong conversion.
That causes "Path exists on disk, but not in 'HEAD^'", which is only
half correct (because the caller just checked and it knows the path
exists in 'HEAD^', but it ignored the result of the check), and more
importantly, completely misses the point.
At least, the following patch seems to work around _this_ particular
codepath, but I suspect that we need to check other places that can
reach diagnose_invalid_foo() functions your commit introduced for
similar breakages.
Ideas for better fix?
sha1_name.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sha1_name.c b/sha1_name.c
index c633113..574c68a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1127,7 +1127,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
if (new_filename)
filename = new_filename;
ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
- if (only_to_die) {
+ if (only_to_die && ret) {
diagnose_invalid_sha1_path(prefix, filename,
tree_sha1, object_name);
free(object_name);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html