Add cache busting to attribute cache

This makes the git attributes and git ignores cache check
stat information before using the file contents from the
cache.  For cached files from the index, it checks the SHA
of the file instead.  This should reduce the need to ever
call `git_attr_cache_flush()` in most situations.

This commit also fixes the `git_status_should_ignore` API
to use the libgit2 standard parameter ordering.

4 files changed, 100 insertions, 34 deletions

diff --git a/src/attr.c b/src/attr.c
index 616cec6ff..1aa965de3 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -235,31 +235,91 @@ bool git_attr_cache__is_cached(
 	return rval;
 }
 
-static int load_attr_file(const char *filename, const char **data)
+static int load_attr_file(
+	const char **data,
+	git_attr_file_stat_sig *sig,
+	const char *filename)
 {
 	int error;
 	git_buf content = GIT_BUF_INIT;
+	struct stat st;
 
-	error = git_futils_readbuffer(&content, filename);
-	*data = error ? NULL : git_buf_detach(&content);
+	if (p_stat(filename, &st) < 0)
+		return GIT_ENOTFOUND;
 
-	return error;
+	if (sig != NULL &&
+		(git_time_t)st.st_mtime == sig->seconds &&
+		(git_off_t)st.st_size == sig->size &&
+		(unsigned int)st.st_ino == sig->ino)
+		return GIT_ENOTFOUND;
+
+	error = git_futils_readbuffer_updated(&content, filename, NULL, NULL);
+	if (error < 0)
+		return error;
+
+	if (sig != NULL) {
+		sig->seconds = (git_time_t)st.st_mtime;
+		sig->size    = (git_off_t)st.st_size;
+		sig->ino     = (unsigned int)st.st_ino;
+	}
+
+	*data = git_buf_detach(&content);
+
+	return 0;
 }
 
 static int load_attr_blob_from_index(
-	git_repository *repo, const char *filename, git_blob **blob)
+	const char **content,
+	git_blob **blob,
+	git_repository *repo,
+	const git_oid *old_oid,
+	const char *relfile)
 {
 	int error;
 	git_index *index;
 	git_index_entry *entry;
 
 	if ((error = git_repository_index__weakptr(&index, repo)) < 0 ||
-		(error = git_index_find(index, filename)) < 0)
+		(error = git_index_find(index, relfile)) < 0)
 		return error;
 
 	entry = git_index_get(index, error);
 
-	return git_blob_lookup(blob, repo, &entry->oid);
+	if (old_oid && git_oid_cmp(old_oid, &entry->oid) == 0)
+		return GIT_ENOTFOUND;
+
+	if ((error = git_blob_lookup(blob, repo, &entry->oid)) < 0)
+		return error;
+
+	*content = git_blob_rawcontent(*blob);
+	return 0;
+}
+
+static int load_attr_from_cache(
+	git_attr_file **file,
+	git_attr_cache *cache,
+	git_attr_file_source source,
+	const char *relative_path)
+{
+	git_buf  cache_key = GIT_BUF_INIT;
+	khiter_t cache_pos;
+
+	*file = NULL;
+
+	if (!cache || !cache->files)
+		return 0;
+
+	if (git_buf_printf(&cache_key, "%d#%s", (int)source, relative_path) < 0)
+		return -1;
+
+	cache_pos = git_strmap_lookup_index(cache->files, cache_key.ptr);
+
+	git_buf_free(&cache_key);
+
+	if (git_strmap_valid_index(cache->files, cache_pos))
+		*file = git_strmap_value_at(cache->files, cache_pos);
+
+	return 0;
 }
 
 int git_attr_cache__internal_file(
@@ -301,6 +361,7 @@ int git_attr_cache__push_file(
 	git_attr_cache *cache = git_repository_attr_cache(repo);
 	git_attr_file *file = NULL;
 	git_blob *blob = NULL;
+	git_attr_file_stat_sig st;
 
 	assert(filename && stack);
 
@@ -316,30 +377,23 @@ int git_attr_cache__push_file(
 		relfile += strlen(workdir);
 
 	/* check cache */
-	if (cache && cache->files) {
-		git_buf  cache_key = GIT_BUF_INIT;
-		khiter_t cache_pos;
-
-		if (git_buf_printf(&cache_key, "%d#%s", (int)source, relfile) < 0)
-			return -1;
+	if (load_attr_from_cache(&file, cache, source, relfile) < 0)
+		return -1;
 
-		cache_pos = git_strmap_lookup_index(cache->files, cache_key.ptr);
+	/* if not in cache, load data, parse, and cache */
 
-		git_buf_free(&cache_key);
+	if (source == GIT_ATTR_FILE_FROM_FILE) {
+		if (file)
+			memcpy(&st, &file->cache_data.st, sizeof(st));
+		else
+			memset(&st, 0, sizeof(st));
 
-		if (git_strmap_valid_index(cache->files, cache_pos)) {
-			file = git_strmap_value_at(cache->files, cache_pos);
-			goto finish;
-		}
+		error = load_attr_file(&content, &st, filename);
+	} else {
+		error = load_attr_blob_from_index(&content, &blob,
+			repo, file ? &file->cache_data.oid : NULL, relfile);
 	}
 
-	/* if not in cache, load data, parse, and cache */
-
-	if (source == GIT_ATTR_FILE_FROM_FILE)
-		error = load_attr_file(filename, &content);
-	else
-		error = load_attr_blob_from_index(repo, relfile, &blob);
-
 	if (error) {
 		/* not finding a file is not an error for this function */
 		if (error == GIT_ENOTFOUND) {
@@ -349,10 +403,8 @@ int git_attr_cache__push_file(
 		goto finish;
 	}
 
-	if (blob)
-		content = git_blob_rawcontent(blob);
-
-	if ((error = git_attr_file__new(&file, source, relfile, &cache->pool)) < 0)
+	if (!file &&
+		(error = git_attr_file__new(&file, source, relfile, &cache->pool)) < 0)
 		goto finish;
 
 	if (parse && (error = parse(repo, content, file)) < 0)
@@ -362,6 +414,12 @@ int git_attr_cache__push_file(
 	if (error > 0)
 		error = 0;
 
+	/* remember "cache buster" file signature */
+	if (blob)
+		git_oid_cpy(&file->cache_data.oid, git_object_id((git_object *)blob));
+	else
+		memcpy(&file->cache_data.st, &st, sizeof(st));
+
 finish:
 	/* push file onto vector if we found one*/
 	if (!error && file != NULL)
diff --git a/src/attr_file.h b/src/attr_file.h
index ec488c4dc..3718f4bda 100644
--- a/src/attr_file.h
+++ b/src/attr_file.h
@@ -48,10 +48,20 @@ typedef struct {
 } git_attr_assignment;
 
 typedef struct {
+	git_time_t seconds;
+	git_off_t  size;
+	unsigned int ino;
+} git_attr_file_stat_sig;
+
+typedef struct {
 	char *key;				/* cache "source#path" this was loaded from */
 	git_vector rules;		/* vector of <rule*> or <fnmatch*> */
 	git_pool *pool;
 	bool pool_is_allocated;
+	union {
+		git_oid oid;
+		git_attr_file_stat_sig st;
+	} cache_data;
 } git_attr_file;
 
 typedef struct {
diff --git a/src/fileops.c b/src/fileops.c
index bf95f769c..6b9d78381 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -185,9 +185,6 @@ int git_futils_readbuffer_updated(git_buf *buf, const char *path, time_t *mtime,
 
 	p_close(fd);
 
-	if (mtime != NULL)
-		*mtime = st.st_mtime;
-
 	if (updated != NULL)
 		*updated = 1;
 
diff --git a/src/status.c b/src/status.c
index d07b0c41c..1c5609cd8 100644
--- a/src/status.c
+++ b/src/status.c
@@ -400,7 +400,8 @@ cleanup:
 	return error;
 }
 
-int git_status_should_ignore(git_repository *repo, const char *path, int *ignored)
+int git_status_should_ignore(
+	int *ignored, git_repository *repo, const char *path)
 {
 	int error;
 	git_ignores ignores;