From d7236c4395a0c8742871a72d920f789b5bd4abf6 Mon Sep 17 00:00:00 2001 From: Matthieu Moy Date: Mon, 18 Jun 2012 20:18:20 +0200 Subject: sha1_name: do not trigger detailed diagnosis for file arguments diagnose_invalid_sha1_path() is meant to be called to diagnose a misspelt : when does not exist in . However, the code may call it if : is invalid (which triggers another call with only_to_die == 1), but for another reason. This happens when calling e.g. git log existing-file HEAD:existing-file because existing-file is a path and not a revision, the code verifies that the arguments that follow to be paths. This leads to an incorrect message like "existing-file does not exist in HEAD", even though the path exists in HEAD. Check that the search for in fails before triggering the diagnosis. Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano diff --git a/sha1_name.c b/sha1_name.c index 03ffc2c..aff224b 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -1115,7 +1115,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 (ret && only_to_die) { diagnose_invalid_sha1_path(prefix, filename, tree_sha1, object_name); free(object_name); diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh index 0843a1c..e81dcd6 100755 --- a/t/t1506-rev-parse-diagnosis.sh +++ b/t/t1506-rev-parse-diagnosis.sh @@ -171,4 +171,15 @@ test_expect_success 'relative path when startup_info is NULL' ' grep "BUG: startup_info struct is not initialized." error ' +test_expect_success ':file correctly diagnosed after a pathname' ' + test_must_fail git rev-parse file.txt HEAD:file.txt 1>actual 2>error && + test_i18ngrep ! "exists on disk" error && + test_i18ngrep "unknown revision or path not in the working tree" error && + cat >expect <<-\EOF && + file.txt + HEAD:file.txt + EOF + test_cmp expect actual +' + test_done -- cgit v0.10.2-6-g49f6 From 023e37c37780d6a56f2870a979c8eb3a9ee9a44d Mon Sep 17 00:00:00 2001 From: Matthieu Moy Date: Mon, 18 Jun 2012 20:18:21 +0200 Subject: verify_filename(): ask the caller to chose the kind of diagnosis verify_filename() can be called in two different contexts. Either we just tried to interpret a string as an object name, and it fails, so we try looking for a working tree file (i.e. we finished looking at revs that come earlier on the command line, and the next argument must be a pathname), or we _know_ that we are looking for a pathname, and shouldn't even try interpreting the string as an object name. For example, with this change, we get: $ git log COPYING HEAD:inexistant fatal: HEAD:inexistant: no such path in the working tree. Use '-- ...' to specify paths that do not exist locally. $ git log HEAD:inexistant fatal: Path 'inexistant' does not exist in 'HEAD' Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano diff --git a/builtin/grep.c b/builtin/grep.c index 9ce064a..d5655565 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -1045,7 +1045,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (!seen_dashdash) { int j; for (j = i; j < argc; j++) - verify_filename(prefix, argv[j]); + verify_filename(prefix, argv[j], j == i); } paths = get_pathspec(prefix, argv + i); diff --git a/builtin/reset.c b/builtin/reset.c index 8c2c1d5..4cc34c9 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -285,7 +285,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) rev = argv[i++]; } else { /* Otherwise we treat this as a filename */ - verify_filename(prefix, argv[i]); + verify_filename(prefix, argv[i], 1); } } diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 98d1cbe..3e2f5bd 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -486,7 +486,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) if (as_is) { if (show_file(arg) && as_is < 2) - verify_filename(prefix, arg); + verify_filename(prefix, arg, 0); continue; } if (!strcmp(arg,"-n")) { @@ -732,7 +732,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) as_is = 1; if (!show_file(arg)) continue; - verify_filename(prefix, arg); + verify_filename(prefix, arg, 1); } if (verify) { if (revs_count == 1) { diff --git a/cache.h b/cache.h index 10afd71..35383e3 100644 --- a/cache.h +++ b/cache.h @@ -452,7 +452,9 @@ extern const char *setup_git_directory(void); extern char *prefix_path(const char *prefix, int len, const char *path); extern const char *prefix_filename(const char *prefix, int len, const char *path); extern int check_filename(const char *prefix, const char *name); -extern void verify_filename(const char *prefix, const char *name); +extern void verify_filename(const char *prefix, + const char *name, + int diagnose_misspelt_rev); extern void verify_non_filename(const char *prefix, const char *name); #define INIT_DB_QUIET 0x0001 diff --git a/revision.c b/revision.c index 064e351..b3c61e1 100644 --- a/revision.c +++ b/revision.c @@ -1755,7 +1755,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s * but the latter we have checked in the main loop. */ for (j = i; j < argc; j++) - verify_filename(revs->prefix, argv[j]); + verify_filename(revs->prefix, argv[j], j == i); append_prune_data(&prune_data, argv + i); break; diff --git a/setup.c b/setup.c index 61c22e6..d121b6a 100644 --- a/setup.c +++ b/setup.c @@ -53,11 +53,17 @@ 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) +static void NORETURN die_verify_filename(const char *prefix, + const char *arg, + int diagnose_misspelt_rev) { unsigned char sha1[20]; unsigned mode; + if (!diagnose_misspelt_rev) + die("%s: no such path in the working tree.\n" + "Use '-- ...' to specify paths that do not exist locally.", + arg); /* * Saying "'(icase)foo' does not exist in the index" when the * user gave us ":(icase)foo" is just stupid. A magic pathspec @@ -80,14 +86,29 @@ static void NORETURN die_verify_filename(const char *prefix, const char *arg) * as true, because even if such a filename were to exist, we want * it to be preceded by the "--" marker (or we want the user to * use a format like "./-filename") + * + * The "diagnose_misspelt_rev" is used to provide a user-friendly + * diagnosis when dying upon finding that "name" is not a pathname. + * If set to 1, the diagnosis will try to diagnose "name" as an + * invalid object name (e.g. HEAD:foo). If set to 0, the diagnosis + * will only complain about an inexisting file. + * + * This function is typically called to check that a "file or rev" + * argument is unambiguous. In this case, the caller will want + * diagnose_misspelt_rev == 1 when verifying the first non-rev + * argument (which could have been a revision), and + * diagnose_misspelt_rev == 0 for the next ones (because we already + * saw a filename, there's not ambiguity anymore). */ -void verify_filename(const char *prefix, const char *arg) +void verify_filename(const char *prefix, + const char *arg, + int diagnose_misspelt_rev) { if (*arg == '-') die("bad flag '%s' used after filename", arg); if (check_filename(prefix, arg)) return; - die_verify_filename(prefix, arg); + die_verify_filename(prefix, arg, diagnose_misspelt_rev); } /* diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh index e81dcd6..c5cb77a 100755 --- a/t/t1506-rev-parse-diagnosis.sh +++ b/t/t1506-rev-parse-diagnosis.sh @@ -174,7 +174,7 @@ test_expect_success 'relative path when startup_info is NULL' ' test_expect_success ':file correctly diagnosed after a pathname' ' test_must_fail git rev-parse file.txt HEAD:file.txt 1>actual 2>error && test_i18ngrep ! "exists on disk" error && - test_i18ngrep "unknown revision or path not in the working tree" error && + test_i18ngrep "no such path in the working tree" error && cat >expect <<-\EOF && file.txt HEAD:file.txt -- cgit v0.10.2-6-g49f6