Skip to content

Commit

Permalink
refactor handling of "other" files in ls-files and status
Browse files Browse the repository at this point in the history
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, 5698454 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>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Oct 17, 2008
1 parent 8ed0a74 commit 98fa473
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 44 deletions.
33 changes: 2 additions & 31 deletions builtin-ls-files.c
Original file line number Diff line number Diff line change
Expand Up @@ -91,39 +91,10 @@ static void show_other_files(struct dir_struct *dir)
{
int i;


/*
* Skip matching and unmerged entries for the paths,
* since we want just "others".
*
* (Matching entries are normally pruned during
* the directory tree walk, but will show up for
* gitlinks because we don't necessarily have
* dir->show_other_directories set to suppress
* them).
*/
for (i = 0; i < dir->nr; i++) {
struct dir_entry *ent = dir->entries[i];
int len, pos;
struct cache_entry *ce;

/*
* Remove the '/' at the end that directory
* walking adds for directory entries.
*/
len = ent->len;
if (len && ent->name[len-1] == '/')
len--;
pos = cache_name_pos(ent->name, len);
if (0 <= pos)
continue; /* exact match */
pos = -pos - 1;
if (pos < active_nr) {
ce = active_cache[pos];
if (ce_namelen(ce) == len &&
!memcmp(ce->name, ent->name, len))
continue; /* Yup, this one exists unmerged */
}
if (!cache_name_is_other(ent->name, ent->len))
continue;
show_dir_entry(tag_other, ent);
}
}
Expand Down
2 changes: 2 additions & 0 deletions cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ static inline void remove_name_hash(struct cache_entry *ce)
#define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
#define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
#define cache_name_exists(name, namelen, igncase) index_name_exists(&the_index, (name), (namelen), (igncase))
#define cache_name_is_other(name, namelen) index_name_is_other(&the_index, (name), (namelen))
#endif

enum object_type {
Expand Down Expand Up @@ -382,6 +383,7 @@ extern int add_to_index(struct index_state *, const char *path, struct stat *, i
extern int add_file_to_index(struct index_state *, const char *path, int flags);
extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
extern int index_name_is_other(const struct index_state *, const char *, int);

/* do stat comparison even if CE_VALID is true */
#define CE_MATCH_IGNORE_VALID 01
Expand Down
28 changes: 28 additions & 0 deletions read-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -1480,3 +1480,31 @@ int read_index_unmerged(struct index_state *istate)
istate->cache_nr = dst - istate->cache;
return !!last;
}

/*
* Returns 1 if the path is an "other" path with respect to
* the index; that is, the path is not mentioned in the index at all,
* either as a file, a directory with some files in the index,
* or as an unmerged entry.
*
* We helpfully remove a trailing "/" from directories so that
* the output of read_directory can be used as-is.
*/
int index_name_is_other(const struct index_state *istate, const char *name,
int namelen)
{
int pos;
if (namelen && name[namelen - 1] == '/')
namelen--;
pos = index_name_pos(istate, name, namelen);
if (0 <= pos)
return 0; /* exact match */
pos = -pos - 1;
if (pos < istate->cache_nr) {
struct cache_entry *ce = istate->cache[pos];
if (ce_namelen(ce) == namelen &&
!memcmp(ce->name, name, namelen))
return 0; /* Yup, this one exists unmerged */
}
return 1;
}
6 changes: 6 additions & 0 deletions t/t7502-status.sh
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,12 @@ test_expect_success 'status submodule summary is disabled by default' '
test_cmp expect output
'

# we expect the same as the previous test
test_expect_success 'status --untracked-files=all does not show submodule' '
git status --untracked-files=all >output &&
test_cmp expect output
'

head=$(cd sm && git rev-parse --short=7 --verify HEAD)

cat >expect <<EOF
Expand Down
15 changes: 2 additions & 13 deletions wt-status.c
Original file line number Diff line number Diff line change
Expand Up @@ -275,20 +275,9 @@ static void wt_status_print_untracked(struct wt_status *s)

read_directory(&dir, ".", "", 0, NULL);
for(i = 0; i < dir.nr; i++) {
/* check for matching entry, which is unmerged; lifted from
* builtin-ls-files:show_other_files */
struct dir_entry *ent = dir.entries[i];
int pos = cache_name_pos(ent->name, ent->len);
struct cache_entry *ce;
if (0 <= pos)
die("bug in wt_status_print_untracked");
pos = -pos - 1;
if (pos < active_nr) {
ce = active_cache[pos];
if (ce_namelen(ce) == ent->len &&
!memcmp(ce->name, ent->name, ent->len))
continue;
}
if (!cache_name_is_other(ent->name, ent->len))
continue;
if (!shown_header) {
s->workdir_untracked = 1;
wt_status_print_header(s, "Untracked files",
Expand Down

0 comments on commit 98fa473

Please sign in to comment.