Skip to content

Commit 009fee4

Browse files
moygitster
moy
authored and
gitster
committed
Detailed diagnosis when parsing an object name fails.
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 9a424b2 commit 009fee4

File tree

4 files changed

+198
-7
lines changed

4 files changed

+198
-7
lines changed

‎cache.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -702,7 +702,11 @@ static inline unsigned int hexval(unsigned char c)
702702
#define DEFAULT_ABBREV 7
703703

704704
extern int get_sha1(const char *str, unsigned char *sha1);
705-
extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);
705+
extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int gently, const char *prefix);
706+
static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
707+
{
708+
return get_sha1_with_mode_1(str, sha1, mode, 1, NULL);
709+
}
706710
extern int get_sha1_hex(const char *hex, unsigned char *sha1);
707711
extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */
708712
extern int read_ref(const char *filename, unsigned char *sha1);

‎setup.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,18 @@ int check_filename(const char *prefix, const char *arg)
7474
die_errno("failed to stat '%s'", arg);
7575
}
7676

77+
static void NORETURN die_verify_filename(const char *prefix, const char *arg)
78+
{
79+
unsigned char sha1[20];
80+
unsigned mode;
81+
/* try a detailed diagnostic ... */
82+
get_sha1_with_mode_1(arg, sha1, &mode, 0, prefix);
83+
/* ... or fall back the most general message. */
84+
die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
85+
"Use '--' to separate paths from revisions", arg);
86+
87+
}
88+
7789
/*
7890
* Verify a filename that we got as an argument for a pathspec
7991
* entry. Note that a filename that begins with "-" never verifies
@@ -87,8 +99,7 @@ void verify_filename(const char *prefix, const char *arg)
8799
die("bad flag '%s' used after filename", arg);
88100
if (check_filename(prefix, arg))
89101
return;
90-
die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
91-
"Use '--' to separate paths from revisions", arg);
102+
die_verify_filename(prefix, arg);
92103
}
93104

94105
/*

‎sha1_name.c

Lines changed: 111 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,96 @@ int get_sha1(const char *name, unsigned char *sha1)
804804
return get_sha1_with_mode(name, sha1, &unused);
805805
}
806806

807-
int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
807+
/* Must be called only when object_name:filename doesn't exist. */
808+
static void diagnose_invalid_sha1_path(const char *prefix,
809+
const char *filename,
810+
const unsigned char *tree_sha1,
811+
const char *object_name)
812+
{
813+
struct stat st;
814+
unsigned char sha1[20];
815+
unsigned mode;
816+
817+
if (!prefix)
818+
prefix = "";
819+
820+
if (!lstat(filename, &st))
821+
die("Path '%s' exists on disk, but not in '%s'.",
822+
filename, object_name);
823+
if (errno == ENOENT || errno == ENOTDIR) {
824+
char *fullname = xmalloc(strlen(filename)
825+
+ strlen(prefix) + 1);
826+
strcpy(fullname, prefix);
827+
strcat(fullname, filename);
828+
829+
if (!get_tree_entry(tree_sha1, fullname,
830+
sha1, &mode)) {
831+
die("Path '%s' exists, but not '%s'.\n"
832+
"Did you mean '%s:%s'?",
833+
fullname,
834+
filename,
835+
object_name,
836+
fullname);
837+
}
838+
die("Path '%s' does not exist in '%s'",
839+
filename, object_name);
840+
}
841+
}
842+
843+
/* Must be called only when :stage:filename doesn't exist. */
844+
static void diagnose_invalid_index_path(int stage,
845+
const char *prefix,
846+
const char *filename)
847+
{
848+
struct stat st;
849+
struct cache_entry *ce;
850+
int pos;
851+
unsigned namelen = strlen(filename);
852+
unsigned fullnamelen;
853+
char *fullname;
854+
855+
if (!prefix)
856+
prefix = "";
857+
858+
/* Wrong stage number? */
859+
pos = cache_name_pos(filename, namelen);
860+
if (pos < 0)
861+
pos = -pos - 1;
862+
ce = active_cache[pos];
863+
if (ce_namelen(ce) == namelen &&
864+
!memcmp(ce->name, filename, namelen))
865+
die("Path '%s' is in the index, but not at stage %d.\n"
866+
"Did you mean ':%d:%s'?",
867+
filename, stage,
868+
ce_stage(ce), filename);
869+
870+
/* Confusion between relative and absolute filenames? */
871+
fullnamelen = namelen + strlen(prefix);
872+
fullname = xmalloc(fullnamelen + 1);
873+
strcpy(fullname, prefix);
874+
strcat(fullname, filename);
875+
pos = cache_name_pos(fullname, fullnamelen);
876+
if (pos < 0)
877+
pos = -pos - 1;
878+
ce = active_cache[pos];
879+
if (ce_namelen(ce) == fullnamelen &&
880+
!memcmp(ce->name, fullname, fullnamelen))
881+
die("Path '%s' is in the index, but not '%s'.\n"
882+
"Did you mean ':%d:%s'?",
883+
fullname, filename,
884+
ce_stage(ce), fullname);
885+
886+
if (!lstat(filename, &st))
887+
die("Path '%s' exists on disk, but not in the index.", filename);
888+
if (errno == ENOENT || errno == ENOTDIR)
889+
die("Path '%s' does not exist (neither on disk nor in the index).",
890+
filename);
891+
892+
free(fullname);
893+
}
894+
895+
896+
int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int gently, const char *prefix)
808897
{
809898
int ret, bracket_depth;
810899
int namelen = strlen(name);
@@ -850,6 +939,8 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
850939
}
851940
pos++;
852941
}
942+
if (!gently)
943+
diagnose_invalid_index_path(stage, prefix, cp);
853944
return -1;
854945
}
855946
for (cp = name, bracket_depth = 0; *cp; cp++) {
@@ -862,9 +953,25 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
862953
}
863954
if (*cp == ':') {
864955
unsigned char tree_sha1[20];
865-
if (!get_sha1_1(name, cp-name, tree_sha1))
866-
return get_tree_entry(tree_sha1, cp+1, sha1,
867-
mode);
956+
char *object_name = NULL;
957+
if (!gently) {
958+
object_name = xmalloc(cp-name+1);
959+
strncpy(object_name, name, cp-name);
960+
object_name[cp-name] = '\0';
961+
}
962+
if (!get_sha1_1(name, cp-name, tree_sha1)) {
963+
const char *filename = cp+1;
964+
ret = get_tree_entry(tree_sha1, filename, sha1, mode);
965+
if (!gently) {
966+
diagnose_invalid_sha1_path(prefix, filename,
967+
tree_sha1, object_name);
968+
free(object_name);
969+
}
970+
return ret;
971+
} else {
972+
if (!gently)
973+
die("Invalid object name '%s'.", object_name);
974+
}
868975
}
869976
return ret;
870977
}

‎t/t1506-rev-parse-diagnosis.sh

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
#!/bin/sh
2+
3+
test_description='test git rev-parse diagnosis for invalid argument'
4+
5+
exec </dev/null
6+
7+
. ./test-lib.sh
8+
9+
HASH_file=
10+
11+
test_expect_success 'set up basic repo' '
12+
echo one > file.txt &&
13+
mkdir subdir &&
14+
echo two > subdir/file.txt &&
15+
echo three > subdir/file2.txt &&
16+
git add . &&
17+
git commit -m init &&
18+
echo four > index-only.txt &&
19+
git add index-only.txt &&
20+
echo five > disk-only.txt
21+
'
22+
23+
test_expect_success 'correct file objects' '
24+
HASH_file=$(git rev-parse HEAD:file.txt) &&
25+
git rev-parse HEAD:subdir/file.txt &&
26+
git rev-parse :index-only.txt &&
27+
(cd subdir &&
28+
git rev-parse HEAD:subdir/file2.txt &&
29+
test $HASH_file = $(git rev-parse HEAD:file.txt) &&
30+
test $HASH_file = $(git rev-parse :file.txt) &&
31+
test $HASH_file = $(git rev-parse :0:file.txt) )
32+
'
33+
34+
test_expect_success 'incorrect revision id' '
35+
test_must_fail git rev-parse foobar:file.txt 2>error &&
36+
grep "Invalid object name '"'"'foobar'"'"'." error &&
37+
test_must_fail git rev-parse foobar 2> error &&
38+
grep "unknown revision or path not in the working tree." error
39+
'
40+
41+
test_expect_success 'incorrect file in sha1:path' '
42+
test_must_fail git rev-parse HEAD:nothing.txt 2> error &&
43+
grep "fatal: Path '"'"'nothing.txt'"'"' does not exist in '"'"'HEAD'"'"'" error &&
44+
test_must_fail git rev-parse HEAD:index-only.txt 2> error &&
45+
grep "fatal: Path '"'"'index-only.txt'"'"' exists on disk, but not in '"'"'HEAD'"'"'." error &&
46+
(cd subdir &&
47+
test_must_fail git rev-parse HEAD:file2.txt 2> error &&
48+
grep "Did you mean '"'"'HEAD:subdir/file2.txt'"'"'?" error )
49+
'
50+
51+
test_expect_success 'incorrect file in :path and :N:path' '
52+
test_must_fail git rev-parse :nothing.txt 2> error &&
53+
grep "fatal: Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." error &&
54+
test_must_fail git rev-parse :1:nothing.txt 2> error &&
55+
grep "Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." error &&
56+
test_must_fail git rev-parse :1:file.txt 2> error &&
57+
grep "Did you mean '"'"':0:file.txt'"'"'?" error &&
58+
(cd subdir &&
59+
test_must_fail git rev-parse :1:file.txt 2> error &&
60+
grep "Did you mean '"'"':0:file.txt'"'"'?" error &&
61+
test_must_fail git rev-parse :file2.txt 2> error &&
62+
grep "Did you mean '"'"':0:subdir/file2.txt'"'"'?" error &&
63+
test_must_fail git rev-parse :2:file2.txt 2> error &&
64+
grep "Did you mean '"'"':0:subdir/file2.txt'"'"'?" error) &&
65+
test_must_fail git rev-parse :disk-only.txt 2> error &&
66+
grep "fatal: Path '"'"'disk-only.txt'"'"' exists on disk, but not in the index." error
67+
'
68+
69+
test_done

0 commit comments

Comments
 (0)