Merge pull request #682 from arrbee/attribute-cache-buster

Attribute cache buster

include/git2/status.h

@@ -139,13 +139,13 @@ GIT_EXTERN(int) git_status_file(unsigned int *status_flags, git_repository *repo
  * would be ignored regardless of whether the file is already in the index
  * or in the repository.
  *
+ * @param ignored boolean returning 0 if the file is not ignored, 1 if it is
  * @param repo a repository object
  * @param path the file to check ignores for, rooted at the repo's workdir
- * @param ignored boolean returning 0 if the file is not ignored, 1 if it is
  * @return GIT_SUCCESS if the ignore rules could be processed for the file
  *		(regardless of whether it exists or not), or an error < 0 if they could not.
  */
-GIT_EXTERN(int) git_status_should_ignore(git_repository *repo, const char *path, int *ignored);
+GIT_EXTERN(int) git_status_should_ignore(int *ignored, git_repository *repo, const char *path);
 
 /** @} */
 GIT_END_DECL

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)

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 {

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;
 

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;

tests-clar/status/ignore.c

@@ -2,12 +2,12 @@
 #include "fileops.h"
 #include "git2/attr.h"
 #include "attr.h"
+#include "status_helpers.h"
 
 static git_repository *g_repo = NULL;
 
 void test_status_ignore__initialize(void)
 {
-	g_repo = cl_git_sandbox_init("attr");
 }
 
 void test_status_ignore__cleanup(void)
@@ -40,9 +40,11 @@ void test_status_ignore__0(void)
 		{ NULL, 0 }
 	}, *one_test;
 
+	g_repo = cl_git_sandbox_init("attr");
+
 	for (one_test = test_cases; one_test->path != NULL; one_test++) {
 		int ignored;
-		cl_git_pass(git_status_should_ignore(g_repo, one_test->path, &ignored));
+		cl_git_pass(git_status_should_ignore(&ignored, g_repo, one_test->path));
 		cl_assert_(ignored == one_test->expected, one_test->path);
 	}
 
@@ -56,25 +58,76 @@ void test_status_ignore__1(void)
 {
 	int ignored;
 
+	g_repo = cl_git_sandbox_init("attr");
+
 	cl_git_rewritefile("attr/.gitignore", "/*.txt\n/dir/\n");
 	git_attr_cache_flush(g_repo);
 
-	cl_git_pass(git_status_should_ignore(g_repo, "root_test4.txt", &ignored));
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "root_test4.txt"));
 	cl_assert(ignored);
 
-	cl_git_pass(git_status_should_ignore(g_repo, "sub/subdir_test2.txt", &ignored));
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "sub/subdir_test2.txt"));
 	cl_assert(!ignored);
 
-	cl_git_pass(git_status_should_ignore(g_repo, "dir", &ignored));
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "dir"));
 	cl_assert(ignored);
 
-	cl_git_pass(git_status_should_ignore(g_repo, "dir/", &ignored));
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "dir/"));
 	cl_assert(ignored);
 
-	cl_git_pass(git_status_should_ignore(g_repo, "sub/dir", &ignored));
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "sub/dir"));
+	cl_assert(!ignored);
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "sub/dir/"));
+	cl_assert(!ignored);
+}
+
+
+void test_status_ignore__empty_repo_with_gitignore_rewrite(void)
+{
+	status_entry_single st;
+	int ignored;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+	cl_git_mkfile(
+		"empty_standard_repo/look-ma.txt", "I'm going to be ignored!");
+
+	memset(&st, 0, sizeof(st));
+	cl_git_pass(git_status_foreach(g_repo, cb_status__single, &st));
+	cl_assert(st.count == 1);
+	cl_assert(st.status == GIT_STATUS_WT_NEW);
+
+	cl_git_pass(git_status_file(&st.status, g_repo, "look-ma.txt"));
+	cl_assert(st.status == GIT_STATUS_WT_NEW);
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "look-ma.txt"));
 	cl_assert(!ignored);
 
-	cl_git_pass(git_status_should_ignore(g_repo, "sub/dir/", &ignored));
+	cl_git_rewritefile("empty_standard_repo/.gitignore", "*.nomatch\n");
+
+	memset(&st, 0, sizeof(st));
+	cl_git_pass(git_status_foreach(g_repo, cb_status__single, &st));
+	cl_assert(st.count == 2);
+	cl_assert(st.status == GIT_STATUS_WT_NEW);
+
+	cl_git_pass(git_status_file(&st.status, g_repo, "look-ma.txt"));
+	cl_assert(st.status == GIT_STATUS_WT_NEW);
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "look-ma.txt"));
 	cl_assert(!ignored);
+
+	cl_git_rewritefile("empty_standard_repo/.gitignore", "*.txt\n");
+
+	memset(&st, 0, sizeof(st));
+	cl_git_pass(git_status_foreach(g_repo, cb_status__single, &st));
+	cl_assert(st.count == 2);
+	cl_assert(st.status == GIT_STATUS_IGNORED);
+
+	cl_git_pass(git_status_file(&st.status, g_repo, "look-ma.txt"));
+	cl_assert(st.status == GIT_STATUS_IGNORED);
+
+	cl_git_pass(git_status_should_ignore(&ignored, g_repo, "look-ma.txt"));
+	cl_assert(ignored);
 }
 

tests-clar/status/status_data.h

-
-struct status_entry_counts {
-	size_t wrong_status_flags_count;
-	size_t wrong_sorted_path;
-	size_t entry_count;
-	const unsigned int* expected_statuses;
-	const char** expected_paths;
-	size_t expected_entry_count;
-};
+#include "status_helpers.h"
 
 /* entries for a plain copy of tests/resources/status */
 

tests-clar/status/status_helpers.c

+#include "clar_libgit2.h"
+#include "status_helpers.h"
+
+int cb_status__normal(
+	const char *path, unsigned int status_flags, void *payload)
+{
+	status_entry_counts *counts = payload;
+
+	if (counts->entry_count >= counts->expected_entry_count) {
+		counts->wrong_status_flags_count++;
+		goto exit;
+	}
+
+	if (strcmp(path, counts->expected_paths[counts->entry_count])) {
+		counts->wrong_sorted_path++;
+		goto exit;
+	}
+
+	if (status_flags != counts->expected_statuses[counts->entry_count])
+		counts->wrong_status_flags_count++;
+
+exit:
+	counts->entry_count++;
+	return 0;
+}
+
+int cb_status__count(const char *p, unsigned int s, void *payload)
+{
+	volatile int *count = (int *)payload;
+
+	GIT_UNUSED(p);
+	GIT_UNUSED(s);
+
+	(*count)++;
+
+	return 0;
+}
+
+int cb_status__single(const char *p, unsigned int s, void *payload)
+{
+	status_entry_single *data = (status_entry_single *)payload;
+
+	GIT_UNUSED(p);
+
+	data->count++;
+	data->status = s;
+
+	return 0;
+}

tests-clar/status/status_helpers.h

+#ifndef INCLUDE_cl_status_helpers_h__
+#define INCLUDE_cl_status_helpers_h__
+
+typedef struct {
+	size_t wrong_status_flags_count;
+	size_t wrong_sorted_path;
+	size_t entry_count;
+	const unsigned int* expected_statuses;
+	const char** expected_paths;
+	size_t expected_entry_count;
+} status_entry_counts;
+
+/* cb_status__normal takes payload of "status_entry_counts *" */
+
+extern int cb_status__normal(
+	const char *path, unsigned int status_flags, void *payload);
+
+
+/* cb_status__count takes payload of "int *" */
+
+extern int cb_status__count(const char *p, unsigned int s, void *payload);
+
+
+typedef struct {
+	int count;
+	unsigned int status;
+} status_entry_single;
+
+/* cb_status__single takes payload of "status_entry_single *" */
+
+extern int cb_status__single(const char *p, unsigned int s, void *payload);
+
+#endif

tests-clar/status/submodules.c

@@ -2,6 +2,7 @@
 #include "buffer.h"
 #include "path.h"
 #include "posix.h"
+#include "status_helpers.h"
 
 static git_repository *g_repo = NULL;
 
@@ -43,19 +44,6 @@ void test_status_submodules__api(void)
 	cl_assert_equal_s("testrepo", sm->path);
 }
 
-static int
-cb_status__submodule_count(const char *p, unsigned int s, void *payload)
-{
-	volatile int *count = (int *)payload;
-
-	GIT_UNUSED(p);
-	GIT_UNUSED(s);
-
-	(*count)++;
-
-	return 0;
-}
-
 void test_status_submodules__0(void)
 {
 	int counts = 0;
@@ -65,7 +53,7 @@ void test_status_submodules__0(void)
 	cl_assert(git_path_isfile("submodules/.gitmodules"));
 
 	cl_git_pass(
-		git_status_foreach(g_repo, cb_status__submodule_count, &counts)
+		git_status_foreach(g_repo, cb_status__count, &counts)
 	);
 
 	cl_assert(counts == 6);

tests-clar/status/worktree.c

@@ -7,45 +7,6 @@
 #include "path.h"
 
 /**
- * Auxiliary methods
- */
-static int
-cb_status__normal( const char *path, unsigned int status_flags, void *payload)
-{
-	struct status_entry_counts *counts = payload;
-
-	if (counts->entry_count >= counts->expected_entry_count) {
-		counts->wrong_status_flags_count++;
-		goto exit;
-	}
-
-	if (strcmp(path, counts->expected_paths[counts->entry_count])) {
-		counts->wrong_sorted_path++;
-		goto exit;
-	}
-
-	if (status_flags != counts->expected_statuses[counts->entry_count])
-		counts->wrong_status_flags_count++;
-
-exit:
-	counts->entry_count++;
-	return 0;
-}
-
-static int
-cb_status__count(const char *p, unsigned int s, void *payload)
-{
-	volatile int *count = (int *)payload;
-
-	GIT_UNUSED(p);
-	GIT_UNUSED(s);
-
-	(*count)++;
-
-	return 0;
-}
-
-/**
  * Initializer
  *
  * Not all of the tests in this file use the same fixtures, so we allow each
@@ -72,10 +33,10 @@ void test_status_worktree__cleanup(void)
 /* this test is equivalent to t18-status.c:statuscb0 */
 void test_status_worktree__whole_repository(void)
 {
-	struct status_entry_counts counts;
+	status_entry_counts counts;
 	git_repository *repo = cl_git_sandbox_init("status");
 
-	memset(&counts, 0x0, sizeof(struct status_entry_counts));
+	memset(&counts, 0x0, sizeof(status_entry_counts));
 	counts.expected_entry_count = entry_count0;
 	counts.expected_paths = entry_paths0;
 	counts.expected_statuses = entry_statuses0;
@@ -120,7 +81,7 @@ static int remove_file_cb(void *data, git_buf *file)
 /* this test is equivalent to t18-status.c:statuscb2 */
 void test_status_worktree__purged_worktree(void)
 {
-	struct status_entry_counts counts;
+	status_entry_counts counts;
 	git_repository *repo = cl_git_sandbox_init("status");
 	git_buf workdir = GIT_BUF_INIT;
 
@@ -130,7 +91,7 @@ void test_status_worktree__purged_worktree(void)
 	git_buf_free(&workdir);
 
 	/* now get status */
-	memset(&counts, 0x0, sizeof(struct status_entry_counts));
+	memset(&counts, 0x0, sizeof(status_entry_counts));
 	counts.expected_entry_count = entry_count2;
 	counts.expected_paths = entry_paths2;
 	counts.expected_statuses = entry_statuses2;
@@ -147,7 +108,7 @@ void test_status_worktree__purged_worktree(void)
 /* this test is similar to t18-status.c:statuscb3 */
 void test_status_worktree__swap_subdir_and_file(void)
 {
-	struct status_entry_counts counts;
+	status_entry_counts counts;
 	git_repository *repo = cl_git_sandbox_init("status");
 	git_status_options opts;
 
@@ -161,7 +122,7 @@ void test_status_worktree__swap_subdir_and_file(void)
 	cl_git_mkfile("status/README.md", "dummy");
 
 	/* now get status */
-	memset(&counts, 0x0, sizeof(struct status_entry_counts));
+	memset(&counts, 0x0, sizeof(status_entry_counts));
 	counts.expected_entry_count = entry_count3;
 	counts.expected_paths = entry_paths3;
 	counts.expected_statuses = entry_statuses3;
@@ -182,7 +143,7 @@ void test_status_worktree__swap_subdir_and_file(void)
 
 void test_status_worktree__swap_subdir_with_recurse_and_pathspec(void)
 {
-	struct status_entry_counts counts;
+	status_entry_counts counts;
 	git_repository *repo = cl_git_sandbox_init("status");
 	git_status_options opts;
 
@@ -196,7 +157,7 @@ void test_status_worktree__swap_subdir_with_recurse_and_pathspec(void)
 	cl_git_mkfile("status/zzz_new_file", "dummy");
 
 	/* now get status */
-	memset(&counts, 0x0, sizeof(struct status_entry_counts));
+	memset(&counts, 0x0, sizeof(status_entry_counts));
 	counts.expected_entry_count = entry_count4;
 	counts.expected_paths = entry_paths4;
 	counts.expected_statuses = entry_statuses4;
@@ -286,18 +247,18 @@ void test_status_worktree__ignores(void)
 
 	for (i = 0; i < (int)entry_count0; i++) {
 		cl_git_pass(
-			git_status_should_ignore(repo, entry_paths0[i], &ignored)
+			git_status_should_ignore(&ignored, repo, entry_paths0[i])
 		);
 		cl_assert(ignored == (entry_statuses0[i] == GIT_STATUS_IGNORED));
 	}
 
 	cl_git_pass(
-		git_status_should_ignore(repo, "nonexistent_file", &ignored)
+		git_status_should_ignore(&ignored, repo, "nonexistent_file")
 	);
 	cl_assert(!ignored);
 
 	cl_git_pass(
-		git_status_should_ignore(repo, "ignored_nonexistent_file", &ignored)
+		git_status_should_ignore(&ignored, repo, "ignored_nonexistent_file")
 	);
 	cl_assert(ignored);
 }
@@ -402,24 +363,6 @@ void test_status_worktree__cannot_retrieve_the_status_of_a_bare_repository(void)
 	git_repository_free(repo);
 }
 
-typedef struct {
-	int count;
-	unsigned int status;
-} status_entry_single;
-
-static int
-cb_status__single(const char *p, unsigned int s, void *payload)
-{
-	status_entry_single *data = (status_entry_single *)payload;
-
-	GIT_UNUSED(p);
-
-	data->count++;
-	data->status = s;
-
-	return 0;
-}
-
 void test_status_worktree__first_commit_in_progress(void)
 {
 	git_repository *repo;