On Mon, Dec 25 2017, Duy Nguyen jotted:
> On Fri, Dec 22, 2017 at 9:00 PM, Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx> wrote:
>> The untracked cache gets confused when a directory is swapped out for
>> a symlink to another directory. Whatever files are inside the target
>> of the symlink will be incorrectly shown as untracked. This issue does
>> not happen if the symlink links to another file, only if it links to
>> another directory.
>
> Sounds about right (I completely forgot about dir symlinks). Since
> I've been away for some time and have not caught up (probably cannot)
> with the mailing list yet, is anyone working on this? It may be
> easiest to just detect symlinksand disable the cache for now.
I haven't yet, I wanted to see what you had to say about it,
i.e. whether it was a "do'h here's a fix" or if it was more involved
than that.
Being completely unfamiliar with this code, I hacked up [1] to add some
ad-hoc tracing to this. It has some bugs and doesn't actually work, but
is injecting something into valid_cached_dir() and checking if the
directory in question is a symlink the right approach?
Although surely a better solution is to just see that y is a symlink to
x, and use the data we get for x.
I also see that the the untracked_cache_dir struct has a stat_data field
which contains a subset of what we get from stat(), but it doesn't have
st_mode, so you can't see from that if the thing was a symlink (but it
could be added).
Is that the right approach? I.e. saving away whether it was a symlink
and if it changes invalidate the cache, although it could be a symlink
to something else, so may it needs to be keyed on st_ino (but that may
be chagned in-place?).
1.
diff --git a/dir.c b/dir.c
index 3c54366a17..8afe068c72 100644
--- a/dir.c
+++ b/dir.c
@@ -1730,10 +1730,13 @@ static int valid_cached_dir(struct dir_struct *dir,
int check_only)
{
struct stat st;
+ struct stat st2;
if (!untracked)
return 0;
+ fprintf(stderr, "Checking <%s>\n", path->buf);
+
/*
* With fsmonitor, we can trust the untracked cache's valid field.
*/
@@ -1742,6 +1745,7 @@ static int valid_cached_dir(struct dir_struct *dir,
if (stat(path->len ? path->buf : ".", &st)) {
invalidate_directory(dir->untracked, untracked);
memset(&untracked->stat_data, 0, sizeof(untracked->stat_data));
+ fprintf(stderr, "Ret #1 = 0\n");
return 0;
}
if (!untracked->valid ||
@@ -1749,12 +1753,14 @@ static int valid_cached_dir(struct dir_struct *dir,
if (untracked->valid)
invalidate_directory(dir->untracked, untracked);
fill_stat_data(&untracked->stat_data, &st);
+ fprintf(stderr, "Ret #2 = 0\n");
return 0;
}
}
if (untracked->check_only != !!check_only) {
invalidate_directory(dir->untracked, untracked);
+ fprintf(stderr, "Ret #3 = 0\n");
return 0;
}
@@ -1772,6 +1778,28 @@ static int valid_cached_dir(struct dir_struct *dir,
} else
prep_exclude(dir, istate, path->buf, path->len);
+ if (path->len && path->buf[path->len - 1] == '/') {
+ struct strbuf dirbuf = STRBUF_INIT;
+ strbuf_add(&dirbuf, path->buf, path->len - 1);
+ fprintf(stderr, "Just dir = <%s>\n", dirbuf.buf);
+
+ if (lstat(dirbuf.buf, &st2)) {
+ fprintf(stderr, "Ret #4 = 0\n");
+ return 0;
+ } else if (S_ISLNK(st2.st_mode)) {
+ invalidate_directory(dir->untracked, untracked);
+ memset(&untracked->stat_data, 0, sizeof(untracked->stat_data));
+ fill_stat_data(&untracked->stat_data, &st);
+ fprintf(stderr, "Is link = <%s>\n", dirbuf.buf);
+ return 0;
+ } else {
+ fprintf(stderr, "Is not link = <%s> but <%d>\n", dirbuf.buf, st2.st_mode);
+ }
+ }
+
+ fprintf(stderr, "Falling through for <%s>\n", path->buf);
+
+
/* hopefully prep_exclude() haven't invalidated this entry... */
return untracked->valid;
}
@@ -1783,9 +1811,12 @@ static int open_cached_dir(struct cached_dir *cdir,
struct strbuf *path,
int check_only)
{
+ int valid;
memset(cdir, 0, sizeof(*cdir));
cdir->untracked = untracked;
- if (valid_cached_dir(dir, untracked, istate, path, check_only))
+ valid = valid_cached_dir(dir, untracked, istate, path, check_only);
+ fprintf(stderr, "Checked <%s>, valid? <%d>\n", path->buf, valid);
+ if (valid)
return 0;
cdir->fdir = opendir(path->len ? path->buf : ".");
if (dir->untracked)
