The argument parser revision.c::setup_revisions() insists on
path arguments that do not follow the double-dash marker to be
paths (either files or directories) that exist in the working
tree. As the fact that diff-files and update-index have
explicit options to ignore "missing" files reminds us,
traditionally we allowed a sparsely populated working tree, so
this check is not very user friendly.
This patch allows non-existing paths to be given without the
double-dash marker as long as they exist in the index, or they
are leading paths of blobs that exist in the index.
A misspelled tag v2.6.16 is not a very likely name to have only
in the index and not in the working tree, so this should not
break the case the original code wanted to fix ;-).
---
* I noticed a breakage in out test scripts while libifying
diff-files. The two-tree fast-forward merge test stuffs an
entry DF/DF in the index while having a working tree file DF,
and run "diff-files DF/DF" to make sure two-way read-tree
left the index dirty. The libified diff-files naturally uses
revision infrastructure to get its parameters, and the path
argument check causes the test to fail without this patch.
This is just an RFC. We can easily rewrite the particular
test to use the double-dash marker, but I suspect existing
porcelains share the same problem, expecting to be able to do
something like this:
$ names=`git diff-files --name-only`
$ git diff-index HEAD $names
Of course, the kosher way is to always use double-dash, like
this:
$ git diff-files -z --name-only |
xargs -0 git diff-index HEAD --
So I am not pushing this too strongly. Comments?
revision.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/revision.c b/revision.c
index 113dd5a..9f6dd24 100644
--- a/revision.c
+++ b/revision.c
@@ -504,6 +504,43 @@ void init_revisions(struct rev_info *rev
diff_setup(&revs->diffopt);
}
+static int check_path_arg(const char *path)
+{
+ struct stat st;
+ int tmp = lstat(path, &st);
+ if (!tmp)
+ return 0;
+ tmp = errno;
+ if (!active_cache)
+ read_cache();
+ if (active_cache) {
+ int namelen = strlen(path);
+ int pos = cache_name_pos(path, namelen);
+ if (pos < 0)
+ pos = -pos - 1;
+ if (pos < active_nr) {
+ struct cache_entry *ce = active_cache[pos];
+ if (ce_namelen(ce) == namelen &&
+ !memcmp(ce->name, path, namelen))
+ return 0;
+ }
+ /* Try it as a directory name - "foo/" */
+ while (++pos < active_nr) {
+ struct cache_entry *ce = active_cache[pos];
+ if (namelen < ce_namelen(ce) &&
+ !memcmp(ce->name, path, namelen)) {
+ /* prefix still matches */
+ if (ce->name[namelen] == '/')
+ return 0;
+ }
+ else
+ break; /* prefix does not match anymore */
+ }
+ }
+ errno = tmp;
+ return -1;
+}
+
/*
* Parse revision information, filling in the "rev_info" structure,
* and removing the used arguments from the argument list.
@@ -752,16 +789,19 @@ int setup_revisions(int argc, const char
arg++;
}
if (get_sha1(arg, sha1) < 0) {
- struct stat st;
int j;
if (seen_dashdash || local_flags)
die("bad revision '%s'", arg);
- /* If we didn't have a "--", all filenames must exist */
+ /* If we didn't have a "--", all filenames must
+ * exist, either in the working tree or in the
+ * cache.
+ */
for (j = i; j < argc; j++) {
- if (lstat(argv[j], &st) < 0)
- die("'%s': %s", argv[j], strerror(errno));
+ if (check_path_arg(argv[j]))
+ die("'%s': %s", argv[j],
+ strerror(errno));
}
revs->prune_data = get_pathspec(revs->prefix, argv + i);
break;
-
: 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