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 as a fallback, or we _know_ that we are
looking for a filename, 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.
$ git log HEAD:inexistant
fatal: Path 'inexistant' does not exist in 'HEAD'
Signed-off-by: Matthieu Moy <Matthieu.Moy@xxxxxxx>
---
This one is much less straightforward, hence the RFC. I did check all
the call sites of verify_filename, but I may not have fully understood
the logic at each call site.
A mistake in the diagnose_rev argument is not _that_ serious however,
since it does not change the potential failures, but only the error
message.
builtin/grep.c | 7 +++++--
builtin/reset.c | 2 +-
builtin/rev-parse.c | 4 ++--
cache.h | 9 ++++++++-
revision.c | 5 +++--
setup.c | 8 +++++---
t/t1506-rev-parse-diagnosis.sh | 2 +-
7 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index fe1726f..41924dc 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -927,8 +927,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
/* The rest are paths */
if (!seen_dashdash) {
int j;
- for (j = i; j < argc; j++)
- verify_filename(prefix, argv[j]);
+ if (i < argc) {
+ verify_filename(prefix, argv[i], 1);
+ for (j = i + 1; j < argc; j++)
+ verify_filename(prefix, argv[j], 0);
+ }
}
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 733f626..13495b8 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")) {
@@ -734,7 +734,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 06413e1..d1a4d9e 100644
--- a/cache.h
+++ b/cache.h
@@ -409,7 +409,14 @@ 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);
+/*
+ * Verify that "name" is a filename.
+ * The "diagnose_rev" is used to provide a user-friendly diagnosis. If
+ * 0, the diagnosis will try to diagnose "name" as an invalid object
+ * name (e.g. HEAD:foo). If non-zero, the diagnosis will only complain
+ * about an inexisting file.
+ */
+extern void verify_filename(const char *prefix, const char *name, int diagnose_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 935e7a7..756196a 100644
--- a/revision.c
+++ b/revision.c
@@ -1780,8 +1780,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
* as a valid filename.
* 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, arg, 1);
+ for (j = i + 1; j < argc; j++)
+ verify_filename(revs->prefix, argv[j], 0);
append_prune_data(&prune_data, argv + i);
break;
diff --git a/setup.c b/setup.c
index 731851a..ca33092 100644
--- a/setup.c
+++ b/setup.c
@@ -53,11 +53,13 @@ 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_rev)
{
unsigned char sha1[20];
unsigned mode;
+ if (!diagnose_rev)
+ die("%s: no such path in the working tree.", arg);
/*
* Saying "'(icase)foo' does not exist in the index" when the
* user gave us ":(icase)foo" is just stupid. A magic pathspec
@@ -81,13 +83,13 @@ static void NORETURN die_verify_filename(const char *prefix, const char *arg)
* it to be preceded by the "--" marker (or we want the user to
* use a format like "./-filename")
*/
-void verify_filename(const char *prefix, const char *arg)
+void verify_filename(const char *prefix, const char *arg, int diagnose_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_rev);
}
/*
diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
index 4a39ac5..a3f11e8 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 '<commit>: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
--
1.7.11.rc0.57.g84a04c7
--
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