Skip to content

Commit

Permalink
Detailed diagnosis when parsing an object name fails.
Browse files Browse the repository at this point in the history
The previous error message was the same in many situations (unknown
revision or path not in the working tree). We try to help the user as
much as possible to understand the error, especially with the
sha1:filename notation. In this case, we say whether the sha1 or the
filename is problematic, and diagnose the confusion between
relative-to-root and relative-to-$PWD confusion precisely.

The 7 new error messages are tested.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
moy authored and gitster committed Dec 7, 2009
1 parent 9a424b2 commit 009fee4
Show file tree
Hide file tree
Showing 4 changed files with 198 additions and 7 deletions.
6 changes: 5 additions & 1 deletion cache.h
Expand Up @@ -702,7 +702,11 @@ static inline unsigned int hexval(unsigned char c)
#define DEFAULT_ABBREV 7

extern int get_sha1(const char *str, unsigned char *sha1);
extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);
extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int gently, const char *prefix);
static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
{
return get_sha1_with_mode_1(str, sha1, mode, 1, NULL);
}
extern int get_sha1_hex(const char *hex, unsigned char *sha1);
extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */
extern int read_ref(const char *filename, unsigned char *sha1);
Expand Down
15 changes: 13 additions & 2 deletions setup.c
Expand Up @@ -74,6 +74,18 @@ int check_filename(const char *prefix, const char *arg)
die_errno("failed to stat '%s'", arg);
}

static void NORETURN die_verify_filename(const char *prefix, const char *arg)
{
unsigned char sha1[20];
unsigned mode;
/* try a detailed diagnostic ... */
get_sha1_with_mode_1(arg, sha1, &mode, 0, prefix);
/* ... or fall back the most general message. */
die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
"Use '--' to separate paths from revisions", arg);

}

/*
* Verify a filename that we got as an argument for a pathspec
* entry. Note that a filename that begins with "-" never verifies
Expand All @@ -87,8 +99,7 @@ void verify_filename(const char *prefix, const char *arg)
die("bad flag '%s' used after filename", arg);
if (check_filename(prefix, arg))
return;
die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
"Use '--' to separate paths from revisions", arg);
die_verify_filename(prefix, arg);
}

/*
Expand Down
115 changes: 111 additions & 4 deletions sha1_name.c
Expand Up @@ -804,7 +804,96 @@ int get_sha1(const char *name, unsigned char *sha1)
return get_sha1_with_mode(name, sha1, &unused);
}

int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
/* Must be called only when object_name:filename doesn't exist. */
static void diagnose_invalid_sha1_path(const char *prefix,
const char *filename,
const unsigned char *tree_sha1,
const char *object_name)
{
struct stat st;
unsigned char sha1[20];
unsigned mode;

if (!prefix)
prefix = "";

if (!lstat(filename, &st))
die("Path '%s' exists on disk, but not in '%s'.",
filename, object_name);
if (errno == ENOENT || errno == ENOTDIR) {
char *fullname = xmalloc(strlen(filename)
+ strlen(prefix) + 1);
strcpy(fullname, prefix);
strcat(fullname, filename);

if (!get_tree_entry(tree_sha1, fullname,
sha1, &mode)) {
die("Path '%s' exists, but not '%s'.\n"
"Did you mean '%s:%s'?",
fullname,
filename,
object_name,
fullname);
}
die("Path '%s' does not exist in '%s'",
filename, object_name);
}
}

/* Must be called only when :stage:filename doesn't exist. */
static void diagnose_invalid_index_path(int stage,
const char *prefix,
const char *filename)
{
struct stat st;
struct cache_entry *ce;
int pos;
unsigned namelen = strlen(filename);
unsigned fullnamelen;
char *fullname;

if (!prefix)
prefix = "";

/* Wrong stage number? */
pos = cache_name_pos(filename, namelen);
if (pos < 0)
pos = -pos - 1;
ce = active_cache[pos];
if (ce_namelen(ce) == namelen &&
!memcmp(ce->name, filename, namelen))
die("Path '%s' is in the index, but not at stage %d.\n"
"Did you mean ':%d:%s'?",
filename, stage,
ce_stage(ce), filename);

/* Confusion between relative and absolute filenames? */
fullnamelen = namelen + strlen(prefix);
fullname = xmalloc(fullnamelen + 1);
strcpy(fullname, prefix);
strcat(fullname, filename);
pos = cache_name_pos(fullname, fullnamelen);
if (pos < 0)
pos = -pos - 1;
ce = active_cache[pos];
if (ce_namelen(ce) == fullnamelen &&
!memcmp(ce->name, fullname, fullnamelen))
die("Path '%s' is in the index, but not '%s'.\n"
"Did you mean ':%d:%s'?",
fullname, filename,
ce_stage(ce), fullname);

if (!lstat(filename, &st))
die("Path '%s' exists on disk, but not in the index.", filename);
if (errno == ENOENT || errno == ENOTDIR)
die("Path '%s' does not exist (neither on disk nor in the index).",
filename);

free(fullname);
}


int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int gently, const char *prefix)
{
int ret, bracket_depth;
int namelen = strlen(name);
Expand Down Expand Up @@ -850,6 +939,8 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
}
pos++;
}
if (!gently)
diagnose_invalid_index_path(stage, prefix, cp);
return -1;
}
for (cp = name, bracket_depth = 0; *cp; cp++) {
Expand All @@ -862,9 +953,25 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
}
if (*cp == ':') {
unsigned char tree_sha1[20];
if (!get_sha1_1(name, cp-name, tree_sha1))
return get_tree_entry(tree_sha1, cp+1, sha1,
mode);
char *object_name = NULL;
if (!gently) {
object_name = xmalloc(cp-name+1);
strncpy(object_name, name, cp-name);
object_name[cp-name] = '\0';
}
if (!get_sha1_1(name, cp-name, tree_sha1)) {
const char *filename = cp+1;
ret = get_tree_entry(tree_sha1, filename, sha1, mode);
if (!gently) {
diagnose_invalid_sha1_path(prefix, filename,
tree_sha1, object_name);
free(object_name);
}
return ret;
} else {
if (!gently)
die("Invalid object name '%s'.", object_name);
}
}
return ret;
}
69 changes: 69 additions & 0 deletions t/t1506-rev-parse-diagnosis.sh
@@ -0,0 +1,69 @@
#!/bin/sh

test_description='test git rev-parse diagnosis for invalid argument'

exec </dev/null

. ./test-lib.sh

HASH_file=

test_expect_success 'set up basic repo' '
echo one > file.txt &&
mkdir subdir &&
echo two > subdir/file.txt &&
echo three > subdir/file2.txt &&
git add . &&
git commit -m init &&
echo four > index-only.txt &&
git add index-only.txt &&
echo five > disk-only.txt
'

test_expect_success 'correct file objects' '
HASH_file=$(git rev-parse HEAD:file.txt) &&
git rev-parse HEAD:subdir/file.txt &&
git rev-parse :index-only.txt &&
(cd subdir &&
git rev-parse HEAD:subdir/file2.txt &&
test $HASH_file = $(git rev-parse HEAD:file.txt) &&
test $HASH_file = $(git rev-parse :file.txt) &&
test $HASH_file = $(git rev-parse :0:file.txt) )
'

test_expect_success 'incorrect revision id' '
test_must_fail git rev-parse foobar:file.txt 2>error &&
grep "Invalid object name '"'"'foobar'"'"'." error &&
test_must_fail git rev-parse foobar 2> error &&
grep "unknown revision or path not in the working tree." error
'

test_expect_success 'incorrect file in sha1:path' '
test_must_fail git rev-parse HEAD:nothing.txt 2> error &&
grep "fatal: Path '"'"'nothing.txt'"'"' does not exist in '"'"'HEAD'"'"'" error &&
test_must_fail git rev-parse HEAD:index-only.txt 2> error &&
grep "fatal: Path '"'"'index-only.txt'"'"' exists on disk, but not in '"'"'HEAD'"'"'." error &&
(cd subdir &&
test_must_fail git rev-parse HEAD:file2.txt 2> error &&
grep "Did you mean '"'"'HEAD:subdir/file2.txt'"'"'?" error )
'

test_expect_success 'incorrect file in :path and :N:path' '
test_must_fail git rev-parse :nothing.txt 2> error &&
grep "fatal: Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." error &&
test_must_fail git rev-parse :1:nothing.txt 2> error &&
grep "Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." error &&
test_must_fail git rev-parse :1:file.txt 2> error &&
grep "Did you mean '"'"':0:file.txt'"'"'?" error &&
(cd subdir &&
test_must_fail git rev-parse :1:file.txt 2> error &&
grep "Did you mean '"'"':0:file.txt'"'"'?" error &&
test_must_fail git rev-parse :file2.txt 2> error &&
grep "Did you mean '"'"':0:subdir/file2.txt'"'"'?" error &&
test_must_fail git rev-parse :2:file2.txt 2> error &&
grep "Did you mean '"'"':0:subdir/file2.txt'"'"'?" error) &&
test_must_fail git rev-parse :disk-only.txt 2> error &&
grep "fatal: Path '"'"'disk-only.txt'"'"' exists on disk, but not in the index." error
'

test_done

0 comments on commit 009fee4

Please sign in to comment.