Skip to content

Commit

Permalink
dir.c: regression fix for add_excludes with fscache
Browse files Browse the repository at this point in the history
Fix regression described in:
#1392

which was introduced in:
b235337

Problem Symptoms
================
When the user has a .gitignore file that is a symlink, the fscache
optimization introduced above caused the stat-data from the symlink,
rather that of the target file, to be returned.  Later when the ignore
file was read, the buffer length did not match the stat.st_size field
and we called die("cannot use <path> as an exclude file")

Optimization Rationale
======================
The above optimization calls lstat() before open() primarily to ask
fscache if the file exists.  It gets the current stat-data as a side
effect essentially for free (since we already have it in memory).
If the file does not exist, it does not need to call open().  And
since very few directories have .gitignore files, we can greatly
reduce time spent in the filesystem.

Discussion of Fix
=================
The above optimization calls lstat() rather than stat() because the
fscache only intercepts lstat() calls.  Calls to stat() stay directed
to the mingw_stat() completly bypassing fscache.  Furthermore, calls
to mingw_stat() always call {open, fstat, close} so that symlinks are
properly dereferenced, which adds *additional* open/close calls on top
of what the original code in dir.c is doing.

Since the problem only manifests for symlinks, we add code to overwrite
the stat-data when the path is a symlink.  This preserves the effect of
the performance gains provided by the fscache in the normal case.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
  • Loading branch information
jeffhostetler authored and dscho committed Sep 18, 2024
1 parent 5a2c256 commit 07cec99
Showing 1 changed file with 28 additions and 0 deletions.
28 changes: 28 additions & 0 deletions dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -1113,13 +1113,41 @@ static int add_patterns(const char *fname, const char *base, int baselen,
size_t size = 0;
char *buf;

/*
* A performance optimization for status.
*
* During a status scan, git looks in each directory for a .gitignore
* file before scanning the directory. Since .gitignore files are not
* that common, we can waste a lot of time looking for files that are
* not there. Fortunately, the fscache already knows if the directory
* contains a .gitignore file, since it has already read the directory
* and it already has the stat-data.
*
* If the fscache is enabled, use the fscache-lstat() interlude to see
* if the file exists (in the fscache hash maps) before trying to open()
* it.
*
* This causes problem when the .gitignore file is a symlink, because
* we call lstat() rather than stat() on the symlnk and the resulting
* stat-data is for the symlink itself rather than the target file.
* We CANNOT use stat() here because the fscache DOES NOT install an
* interlude for stat() and mingw_stat() always calls "open-fstat-close"
* on the file and defeats the purpose of the optimization here. Since
* symlinks are even more rare than .gitignore files, we force a fstat()
* after our open() to get stat-data for the target file.
*/
if (is_fscache_enabled(fname)) {
if (lstat(fname, &st) < 0) {
fd = -1;
} else {
fd = open(fname, O_RDONLY);
if (fd < 0)
warn_on_fopen_errors(fname);
else if (S_ISLNK(st.st_mode) && fstat(fd, &st) < 0) {
warn_on_fopen_errors(fname);
close(fd);
fd = -1;
}
}
} else {
if (flags & PATTERN_NOFOLLOW)
Expand Down

0 comments on commit 07cec99

Please sign in to comment.