refactor handling of "other" files in ls-files and status
When the "git status" display code was originally converted to C, we copied the code from ls-files to discover whether a pathname returned by read_directory was an "other", or untracked, file. Much later, 5698454e updated the code in ls-files to handle some new cases caused by gitlinks. This left the code in wt-status.c broken: it would display submodule directories as untracked directories. Nobody noticed until now, however, because unless status.showUntrackedFiles was set to "all", submodule directories were not actually reported by read_directory. So the bug was only triggered in the presence of a submodule _and_ this config option. This patch pulls the ls-files code into a new function, cache_name_is_other, and uses it in both places. This should leave the ls-files functionality the same and fix the bug in status. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Oct 16, 2008 at 11:07 UTC
98fa473887d0bebd38d568bb07232a336a642dcf
5 files changed
+40
-44
builtin-ls-files.c
+2
-31
@@ -91,39 +91,10 @@ static void show_other_files(struct dir_struct *dir)
91
{
92
int i;
93
94
-
95
- /*
96
- * Skip matching and unmerged entries for the paths,
97
- * since we want just "others".
98
- *
99
- * (Matching entries are normally pruned during
100
- * the directory tree walk, but will show up for
101
- * gitlinks because we don't necessarily have
102
- * dir->show_other_directories set to suppress
103
- * them).
104
- */
94
for (i = 0; i < dir->nr; i++) {
95
struct dir_entry *ent = dir->entries[i];
107
- int len, pos;
108
- struct cache_entry *ce;
109
-
110
- /*
111
- * Remove the '/' at the end that directory
112
- * walking adds for directory entries.
113
- */
114
- len = ent->len;
115
- if (len && ent->name[len-1] == '/')
116
- len--;
117
- pos = cache_name_pos(ent->name, len);
118
- if (0 <= pos)
119
- continue; /* exact match */
120
- pos = -pos - 1;
121
- if (pos < active_nr) {
122
- ce = active_cache[pos];
123
- if (ce_namelen(ce) == len &&
124
- !memcmp(ce->name, ent->name, len))
125
- continue; /* Yup, this one exists unmerged */
126
- }
96
+ if (!cache_name_is_other(ent->name, ent->len))
97
+ continue;
98
show_dir_entry(tag_other, ent);
99
}
100
}
cache.h
+2
@@ -270,6 +270,7 @@ static inline void remove_name_hash(struct cache_entry *ce)
270
#define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
271
#define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
272
#define cache_name_exists(name, namelen, igncase) index_name_exists(&the_index, (name), (namelen), (igncase))
273
+#define cache_name_is_other(name, namelen) index_name_is_other(&the_index, (name), (namelen))
274
#endif
275
276
enum object_type {
@@ -382,6 +383,7 @@ extern int add_to_index(struct index_state *, const char *path, struct stat *, i
383
extern int add_file_to_index(struct index_state *, const char *path, int flags);
384
extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
385
extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
386
+extern int index_name_is_other(const struct index_state *, const char *, int);
387
388
/* do stat comparison even if CE_VALID is true */
389
#define CE_MATCH_IGNORE_VALID 01
read-cache.c
+28
@@ -1480,3 +1480,31 @@ int read_index_unmerged(struct index_state *istate)
1480
istate->cache_nr = dst - istate->cache;
1481
return !!last;
1482
}
1483
+
1484
+/*
1485
+ * Returns 1 if the path is an "other" path with respect to
1486
+ * the index; that is, the path is not mentioned in the index at all,
1487
+ * either as a file, a directory with some files in the index,
1488
+ * or as an unmerged entry.
1489
+ *
1490
+ * We helpfully remove a trailing "/" from directories so that
1491
+ * the output of read_directory can be used as-is.
1492
+ */
1493
+int index_name_is_other(const struct index_state *istate, const char *name,
1494
+ int namelen)
1495
+{
1496
+ int pos;
1497
+ if (namelen && name[namelen - 1] == '/')
1498
+ namelen--;
1499
+ pos = index_name_pos(istate, name, namelen);
1500
+ if (0 <= pos)
1501
+ return 0; /* exact match */
1502
+ pos = -pos - 1;
1503
+ if (pos < istate->cache_nr) {
1504
+ struct cache_entry *ce = istate->cache[pos];
1505
+ if (ce_namelen(ce) == namelen &&
1506
+ !memcmp(ce->name, name, namelen))
1507
+ return 0; /* Yup, this one exists unmerged */
1508
+ }
1509
+ return 1;
1510
+}
t/t7502-status.sh
+6
@@ -285,6 +285,12 @@ test_expect_success 'status submodule summary is disabled by default' '
285
test_cmp expect output
286
'
287
288
+# we expect the same as the previous test
289
+test_expect_success 'status --untracked-files=all does not show submodule' '
290
+ git status --untracked-files=all >output &&
291
+ test_cmp expect output
292
+'
293
+
294
head=$(cd sm && git rev-parse --short=7 --verify HEAD)
295
296
cat >expect <<EOF
wt-status.c
+2
-13
@@ -275,20 +275,9 @@ static void wt_status_print_untracked(struct wt_status *s)
275
276
read_directory(&dir, ".", "", 0, NULL);
277
for(i = 0; i < dir.nr; i++) {
278
- /* check for matching entry, which is unmerged; lifted from
279
- * builtin-ls-files:show_other_files */
278
struct dir_entry *ent = dir.entries[i];
281
- int pos = cache_name_pos(ent->name, ent->len);
282
- struct cache_entry *ce;
283
- if (0 <= pos)
284
- die("bug in wt_status_print_untracked");
285
- pos = -pos - 1;
286
- if (pos < active_nr) {
287
- ce = active_cache[pos];
288
- if (ce_namelen(ce) == ent->len &&
289
- !memcmp(ce->name, ent->name, ent->len))
290
- continue;
291
- }
279
+ if (!cache_name_is_other(ent->name, ent->len))
280
+ continue;
281
if (!shown_header) {
282
s->workdir_untracked = 1;
283
wt_status_print_header(s, "Untracked files",