Attribute file cache refactor

This is a big refactoring of the attribute file cache to be a bit
simpler which in turn makes it easier to enforce a lock around any
updates to the cache so that it can be used in a threaded env.
Tons of changes to the attributes and ignores code.

src/attr.h

@@ -8,38 +8,6 @@
 #define INCLUDE_attr_h__
 
 #include "attr_file.h"
-
-#define GIT_ATTR_CONFIG       "core.attributesfile"
-#define GIT_IGNORE_CONFIG     "core.excludesfile"
-
-typedef int (*git_attr_file_parser)(
-	git_repository *, void *, const char *, git_attr_file *);
-
-extern int git_attr_cache__insert_macro(
-	git_repository *repo, git_attr_rule *macro);
-
-extern git_attr_rule *git_attr_cache__lookup_macro(
-	git_repository *repo, const char *name);
-
-extern int git_attr_cache__push_file(
-	git_repository *repo,
-	const char *base,
-	const char *filename,
-	git_attr_file_source source,
-	git_attr_file_parser parse,
-	void *parsedata, /* passed through to parse function */
-	git_vector *stack);
-
-extern int git_attr_cache__internal_file(
-	git_repository *repo,
-	const char *key,
-	git_attr_file **file_ptr);
-
-/* returns true if path is in cache */
-extern bool git_attr_cache__is_cached(
-	git_repository *repo, git_attr_file_source source, const char *path);
-
-extern int git_attr_cache__decide_sources(
-	uint32_t flags, bool has_wd, bool has_index, git_attr_file_source *srcs);
+#include "attrcache.h"
 
 #endif

src/attr_file.h

@@ -13,6 +13,7 @@
 #include "pool.h"
 #include "buffer.h"
 #include "fileops.h"
+#include "attrcache.h"
 
 #define GIT_ATTR_FILE			".gitattributes"
 #define GIT_ATTR_FILE_INREPO	"info/attributes"
@@ -45,10 +46,10 @@ typedef struct {
 	unsigned int flags;
 } git_attr_fnmatch;
 
-typedef struct {
+struct git_attr_rule {
 	git_attr_fnmatch match;
 	git_vector assigns;		/* vector of <git_attr_assignment*> */
-} git_attr_rule;
+};
 
 typedef struct {
 	git_refcount unused;
@@ -63,17 +64,17 @@ typedef struct {
 	const char *value;
 } git_attr_assignment;
 
-typedef struct {
+struct git_attr_file {
 	git_refcount rc;
-	char *key;				/* cache "source#path" this was loaded from */
-	git_vector rules;		/* vector of <rule*> or <fnmatch*> */
-	git_pool *pool;
-	bool pool_is_allocated;
+	git_attr_cache_entry *ce;
+	git_attr_cache_source source;
+	git_vector rules;			/* vector of <rule*> or <fnmatch*> */
+	git_pool pool;
 	union {
 		git_oid oid;
 		git_futils_filestamp stamp;
 	} cache_data;
-} git_attr_file;
+};
 
 typedef struct {
 	git_buf  full;
@@ -82,29 +83,41 @@ typedef struct {
 	int      is_dir;
 } git_attr_path;
 
-typedef enum {
-	GIT_ATTR_FILE_FROM_FILE = 0,
-	GIT_ATTR_FILE_FROM_INDEX = 1
-} git_attr_file_source;
-
 /*
  * git_attr_file API
  */
 
-extern int git_attr_file__new(
-	git_attr_file **attrs_ptr, git_attr_file_source src, const char *path, git_pool *pool);
+int git_attr_file__new(
+	git_attr_file **out,
+	git_attr_cache_entry *ce,
+	git_attr_cache_source source);
+
+void git_attr_file__free(git_attr_file *file);
+
+int git_attr_file__load(
+	git_attr_file **out,
+	git_repository *repo,
+	git_attr_cache_entry *ce,
+	git_attr_cache_source source,
+	git_attr_cache_parser parser,
+	void *payload);
 
-extern int git_attr_file__new_and_load(
-	git_attr_file **attrs_ptr, const char *path);
+int git_attr_file__load_standalone(
+	git_attr_file **out,
+	const char *path);
 
-extern void git_attr_file__free(git_attr_file *file);
+int git_attr_file__out_of_date(
+	git_repository *repo, git_attr_file *file);
 
-extern void git_attr_file__clear_rules(git_attr_file *file);
+int git_attr_file__parse_buffer(
+	git_repository *repo,
+	git_attr_file *attrs,
+	const char *data,
+	void *payload);
 
-extern int git_attr_file__parse_buffer(
-	git_repository *repo, void *parsedata, const char *buf, git_attr_file *file);
+void git_attr_file__clear_rules(git_attr_file *file);
 
-extern int git_attr_file__lookup_one(
+int git_attr_file__lookup_one(
 	git_attr_file *file,
 	const git_attr_path *path,
 	const char *attr,
@@ -115,7 +128,7 @@ extern int git_attr_file__lookup_one(
 	git_vector_rforeach(&(file)->rules, (iter), (rule)) \
 		if (git_attr_rule__match((rule), (path)))
 
-extern uint32_t git_attr_file__name_hash(const char *name);
+uint32_t git_attr_file__name_hash(const char *name);
 
 
 /*

src/attrcache.c

+#include "common.h"
+#include "repository.h"
+#include "attr_file.h"
+#include "config.h"
+#include "sysdir.h"
+#include "ignore.h"
+
+GIT__USE_STRMAP;
+
+GIT_INLINE(int) attr_cache_lock(git_attr_cache *cache)
+{
+	GIT_UNUSED(cache); /* avoid warning if threading is off */
+
+	if (git_mutex_lock(&cache->lock) < 0) {
+		giterr_set(GITERR_OS, "Unable to get attr cache lock");
+		return -1;
+	}
+	return 0;
+}
+
+GIT_INLINE(void) attr_cache_unlock(git_attr_cache *cache)
+{
+	GIT_UNUSED(cache); /* avoid warning if threading is off */
+	git_mutex_unlock(&cache->lock);
+}
+
+GIT_INLINE(git_attr_cache_entry *) attr_cache_lookup_entry(
+	git_attr_cache *cache, const char *path)
+{
+	khiter_t pos = git_strmap_lookup_index(cache->files, path);
+
+	if (git_strmap_valid_index(cache->files, pos))
+		return git_strmap_value_at(cache->files, pos);
+	else
+		return NULL;
+}
+
+int git_attr_cache_entry__new(
+	git_attr_cache_entry **out,
+	const char *base,
+	const char *path,
+	git_pool *pool)
+{
+	size_t baselen = base ? strlen(base) : 0, pathlen = strlen(path);
+	size_t cachesize = sizeof(git_attr_cache_entry) + baselen + pathlen + 1;
+	git_attr_cache_entry *ce;
+
+	ce = git_pool_mallocz(pool, cachesize);
+	GITERR_CHECK_ALLOC(ce);
+
+	if (baselen)
+		memcpy(ce->fullpath, base, baselen);
+	memcpy(&ce->fullpath[baselen], path, pathlen);
+	ce->path = &ce->fullpath[baselen];
+	*out = ce;
+
+	return 0;
+}
+
+/* call with attrcache locked */
+static int attr_cache_make_entry(
+	git_attr_cache_entry **out, git_repository *repo, const char *path)
+{
+	int error = 0;
+	git_attr_cache *cache = git_repository_attr_cache(repo);
+	git_attr_cache_entry *ce = NULL;
+
+	error = git_attr_cache_entry__new(
+		&ce, git_repository_workdir(repo), path, &cache->pool);
+
+	if (!error) {
+		git_strmap_insert(cache->files, ce->path, ce, error);
+		if (error > 0)
+			error = 0;
+	}
+
+	*out = ce;
+	return error;
+}
+
+/* insert entry or replace existing if we raced with another thread */
+static int attr_cache_upsert(git_attr_cache *cache, git_attr_file *file)
+{
+	git_attr_cache_entry *ce;
+	git_attr_file *old;
+
+	if (attr_cache_lock(cache) < 0)
+		return -1;
+
+	ce = attr_cache_lookup_entry(cache, file->ce->path);
+
+	old = ce->file[file->source];
+
+	GIT_REFCOUNT_OWN(file, ce);
+	GIT_REFCOUNT_INC(file);
+	ce->file[file->source] = file;
+
+	if (old) {
+		GIT_REFCOUNT_OWN(old, NULL);
+		git_attr_file__free(old);
+	}
+
+	attr_cache_unlock(cache);
+	return 0;
+}
+
+static int attr_cache_remove(git_attr_cache *cache, git_attr_file *file)
+{
+	int error = 0;
+	git_attr_cache_entry *ce;
+	bool found = false;
+
+	if (!file)
+		return 0;
+	if ((error = attr_cache_lock(cache)) < 0)
+		return error;
+
+	if ((ce = attr_cache_lookup_entry(cache, file->ce->path)) != NULL &&
+		ce->file[file->source] == file)
+	{
+		ce->file[file->source] = NULL;
+		found = true;
+	}
+
+	attr_cache_unlock(cache);
+
+	if (found)
+		git_attr_file__free(file);
+
+	return error;
+}
+
+int git_attr_cache__get(
+	git_attr_file **out,
+	git_repository *repo,
+	git_attr_cache_source source,
+	const char *base,
+	const char *filename,
+	git_attr_cache_parser parser,
+	void *payload)
+{
+	int error = 0;
+	git_buf path = GIT_BUF_INIT;
+	const char *wd = git_repository_workdir(repo), *relfile;
+	git_attr_cache *cache = git_repository_attr_cache(repo);
+	git_attr_cache_entry *ce = NULL;
+	git_attr_file *file = NULL;
+
+	/* join base and path as needed */
+	if (base != NULL && git_path_root(filename) < 0) {
+		if (git_buf_joinpath(&path, base, filename) < 0)
+			return -1;
+		filename = path.ptr;
+	}
+
+	relfile = filename;
+	if (wd && !git__prefixcmp(relfile, wd))
+		relfile += strlen(wd);
+
+	/* check cache for existing entry */
+	if ((error = attr_cache_lock(cache)) < 0)
+		goto cleanup;
+
+	ce = attr_cache_lookup_entry(cache, relfile);
+	if (!ce) {
+		if ((error = attr_cache_make_entry(&ce, repo, relfile)) < 0)
+			goto cleanup;
+	} else if (ce->file[source] != NULL) {
+		file = ce->file[source];
+		GIT_REFCOUNT_INC(file);
+	}
+
+	attr_cache_unlock(cache);
+
+	/* if this is not a file backed entry, just create a new empty one */
+	if (!parser) {
+		error = git_attr_file__new(&file, ce, source);
+		goto cleanup;
+	}
+
+	/* otherwise load and/or reload as needed */
+	switch (git_attr_file__out_of_date(repo, file)) {
+	case 1:
+		if (!(error = git_attr_file__load(
+				  &file, repo, ce, source, parser, payload)))
+			error = attr_cache_upsert(cache, file);
+		break;
+	case 0:
+		/* just use the file */
+		break;
+	case GIT_ENOTFOUND:
+		/* did exist and now does not - remove from cache */
+		error = attr_cache_remove(cache, file);
+		file = NULL;
+		break;
+	default:
+		/* other error (e.g. out of memory, can't read index) */
+		giterr_clear();
+		break;
+	}
+
+cleanup:
+	*out = error ? NULL : file;
+	git_buf_free(&path);
+	return error;
+}
+
+bool git_attr_cache__is_cached(
+	git_repository *repo,
+	git_attr_cache_source source,
+	const char *filename)
+{
+	git_attr_cache *cache = git_repository_attr_cache(repo);
+	git_strmap *files;
+	khiter_t pos;
+	git_attr_cache_entry *ce;
+
+	if (!(cache = git_repository_attr_cache(repo)) ||
+		!(files = cache->files))
+		return false;
+
+	pos = git_strmap_lookup_index(files, filename);
+	if (!git_strmap_valid_index(files, pos))
+		return false;
+
+	ce = git_strmap_value_at(files, pos);
+
+	return ce && (ce->file[source] != NULL);
+}
+
+
+static int attr_cache__lookup_path(
+	char **out, git_config *cfg, const char *key, const char *fallback)
+{
+	git_buf buf = GIT_BUF_INIT;
+	int error;
+	const git_config_entry *entry = NULL;
+
+	*out = NULL;
+
+	if ((error = git_config__lookup_entry(&entry, cfg, key, false)) < 0)
+		return error;
+
+	if (entry) {
+		const char *cfgval = entry->value;
+
+		/* expand leading ~/ as needed */
+		if (cfgval && cfgval[0] == '~' && cfgval[1] == '/' &&
+			!git_sysdir_find_global_file(&buf, &cfgval[2]))
+			*out = git_buf_detach(&buf);
+		else if (cfgval)
+			*out = git__strdup(cfgval);
+
+	}
+	else if (!git_sysdir_find_xdg_file(&buf, fallback))
+		*out = git_buf_detach(&buf);
+
+	git_buf_free(&buf);
+
+	return error;
+}
+
+static void attr_cache__free(git_attr_cache *cache)
+{
+	if (!cache)
+		return;
+
+	if (cache->files != NULL) {
+		git_attr_file *file;
+
+		git_strmap_foreach_value(cache->files, file, {
+			git_attr_file__free(file);
+		});
+		git_strmap_free(cache->files);
+	}
+
+	if (cache->macros != NULL) {
+		git_attr_rule *rule;
+
+		git_strmap_foreach_value(cache->macros, rule, {
+			git_attr_rule__free(rule);
+		});
+		git_strmap_free(cache->macros);
+	}
+
+	git_pool_clear(&cache->pool);
+
+	git__free(cache->cfg_attr_file);
+	cache->cfg_attr_file = NULL;
+
+	git__free(cache->cfg_excl_file);
+	cache->cfg_excl_file = NULL;
+
+	git_mutex_free(&cache->lock);
+
+	git__free(cache);
+}
+
+int git_attr_cache__init(git_repository *repo)
+{
+	int ret = 0;
+	git_attr_cache *cache = git_repository_attr_cache(repo);
+	git_config *cfg;
+
+	if (cache)
+		return 0;
+
+	if ((ret = git_repository_config__weakptr(&cfg, repo)) < 0)
+		return ret;
+
+	cache = git__calloc(1, sizeof(git_attr_cache));
+	GITERR_CHECK_ALLOC(cache);
+
+	/* set up lock */
+	if (git_mutex_init(&cache->lock) < 0) {
+		giterr_set(GITERR_OS, "Unable to initialize lock for attr cache");
+		git__free(cache);
+		return -1;
+	}
+
+	/* cache config settings for attributes and ignores */
+	ret = attr_cache__lookup_path(
+		&cache->cfg_attr_file, cfg, GIT_ATTR_CONFIG, GIT_ATTR_FILE_XDG);
+	if (ret < 0)
+		goto cancel;
+
+	ret = attr_cache__lookup_path(
+		&cache->cfg_excl_file, cfg, GIT_IGNORE_CONFIG, GIT_IGNORE_FILE_XDG);
+	if (ret < 0)
+		goto cancel;
+
+	/* allocate hashtable for attribute and ignore file contents,
+	 * hashtable for attribute macros, and string pool
+	 */
+	if ((ret = git_strmap_alloc(&cache->files)) < 0 ||
+		(ret = git_strmap_alloc(&cache->macros)) < 0 ||
+		(ret = git_pool_init(&cache->pool, 1, 0)) < 0)
+		goto cancel;
+
+	cache = git__compare_and_swap(&repo->attrcache, NULL, cache);
+	if (cache)
+		goto cancel; /* raced with another thread, free this but no error */
+
+	/* insert default macros */
+	return git_attr_add_macro(repo, "binary", "-diff -crlf -text");
+
+cancel:
+	attr_cache__free(cache);
+	return ret;
+}
+
+void git_attr_cache_flush(git_repository *repo)
+{
+	git_attr_cache *cache;
+
+	/* this could be done less expensively, but for now, we'll just free
+	 * the entire attrcache and let the next use reinitialize it...
+	 */
+	if (repo && (cache = git__swap(repo->attrcache, NULL)) != NULL)
+		attr_cache__free(cache);
+}
+
+int git_attr_cache__insert_macro(git_repository *repo, git_attr_rule *macro)
+{
+	git_attr_cache *cache = git_repository_attr_cache(repo);
+	git_strmap *macros = cache->macros;
+	int error;
+
+	/* TODO: generate warning log if (macro->assigns.length == 0) */
+	if (macro->assigns.length == 0)
+		return 0;
+
+	if (git_mutex_lock(&cache->lock) < 0) {
+		giterr_set(GITERR_OS, "Unable to get attr cache lock");
+		error = -1;
+	} else {
+		git_strmap_insert(macros, macro->match.pattern, macro, error);
+		git_mutex_unlock(&cache->lock);
+	}
+
+	return (error < 0) ? -1 : 0;
+}
+
+git_attr_rule *git_attr_cache__lookup_macro(
+	git_repository *repo, const char *name)
+{
+	git_strmap *macros = git_repository_attr_cache(repo)->macros;
+	khiter_t pos;
+
+	pos = git_strmap_lookup_index(macros, name);
+
+	if (!git_strmap_valid_index(macros, pos))
+		return NULL;
+
+	return (git_attr_rule *)git_strmap_value_at(macros, pos);
+}
+

src/attrcache.h

@@ -9,11 +9,15 @@
 
 #include "pool.h"
 #include "strmap.h"
+#include "buffer.h"
+
+#define GIT_ATTR_CONFIG       "core.attributesfile"
+#define GIT_IGNORE_CONFIG     "core.excludesfile"
 
 typedef struct {
 	char *cfg_attr_file; /* cached value of core.attributesfile */
 	char *cfg_excl_file; /* cached value of core.excludesfile */
-	git_strmap *files;	 /* hash path to git_attr_file of rules */
+	git_strmap *files;	 /* hash path to git_attr_cache_entry records */
 	git_strmap *macros;	 /* hash name to vector<git_attr_assignment> */
 	git_mutex lock;
 	git_pool  pool;
@@ -21,4 +25,53 @@ typedef struct {
 
 extern int git_attr_cache__init(git_repository *repo);
 
+typedef enum {
+	GIT_ATTR_CACHE__FROM_FILE = 0,
+	GIT_ATTR_CACHE__FROM_INDEX = 1,
+
+	GIT_ATTR_CACHE_NUM_SOURCES = 2
+} git_attr_cache_source;
+
+typedef struct git_attr_file git_attr_file;
+typedef struct git_attr_rule git_attr_rule;
+
+typedef struct {
+	git_attr_file *file[GIT_ATTR_CACHE_NUM_SOURCES];
+	const char *path; /* points into fullpath */
+	char fullpath[GIT_FLEX_ARRAY];
+} git_attr_cache_entry;
+
+typedef int (*git_attr_cache_parser)(
+	git_repository *repo,
+	git_attr_file *file,
+	const char *data,
+	void *payload);
+
+/* get file - loading and reload as needed */
+extern int git_attr_cache__get(
+	git_attr_file **file,
+	git_repository *repo,
+	git_attr_cache_source source,
+	const char *base,
+	const char *filename,
+	git_attr_cache_parser parser,
+	void *payload);
+
+extern bool git_attr_cache__is_cached(
+	git_repository *repo,
+	git_attr_cache_source source,
+	const char *path);
+
+extern int git_attr_cache__insert_macro(
+	git_repository *repo, git_attr_rule *macro);
+
+extern git_attr_rule *git_attr_cache__lookup_macro(
+	git_repository *repo, const char *name);
+
+extern int git_attr_cache_entry__new(
+	git_attr_cache_entry **out,
+	const char *base,
+	const char *path,
+	git_pool *pool);
+
 #endif

src/fileops.c

@@ -804,10 +804,8 @@ int git_futils_filestamp_check(
 	if (stamp == NULL)
 		return 1;
 
-	if (p_stat(path, &st) < 0) {
-		giterr_set(GITERR_OS, "Could not stat '%s'", path);
+	if (p_stat(path, &st) < 0)
 		return GIT_ENOTFOUND;
-	}
 
 	if (stamp->mtime == (git_time_t)st.st_mtime &&
 		stamp->size  == (git_off_t)st.st_size   &&

src/fileops.h

@@ -292,13 +292,14 @@ typedef struct {
  * Compare stat information for file with reference info.
  *
  * This function updates the file stamp to current data for the given path
- * and returns 0 if the file is up-to-date relative to the prior setting or
- * 1 if the file has been changed.  (This also may return GIT_ENOTFOUND if
- * the file doesn't exist.)
+ * and returns 0 if the file is up-to-date relative to the prior setting,
+ * 1 if the file has been changed, or GIT_ENOTFOUND if the file doesn't
+ * exist.  This will not call giterr_set, so you must set the error if you
+ * plan to return an error.
  *
  * @param stamp File stamp to be checked
  * @param path Path to stat and check if changed
- * @return 0 if up-to-date, 1 if out-of-date, <0 on error
+ * @return 0 if up-to-date, 1 if out-of-date, GIT_ENOTFOUND if cannot stat
  */
 extern int git_futils_filestamp_check(
 	git_futils_filestamp *stamp, const char *path);

src/ignore.c

 #include "git2/ignore.h"
 #include "common.h"
 #include "ignore.h"
-#include "attr.h"
+#include "attr_file.h"
 #include "path.h"
 #include "config.h"
 
@@ -10,26 +10,27 @@
 #define GIT_IGNORE_DEFAULT_RULES ".\n..\n.git\n"
 
 static int parse_ignore_file(
-	git_repository *repo, void *parsedata, const char *buffer, git_attr_file *ignores)
+	git_repository *repo,
+	git_attr_file *attrs,
+	const char *data,
+	void *payload)
 {
 	int error = 0;
-	git_attr_fnmatch *match = NULL;
-	const char *scan = NULL, *context = NULL;
 	int ignore_case = false;
+	const char *scan = data, *context = NULL;
+	git_attr_fnmatch *match = NULL;
 
-	/* Prefer to have the caller pass in a git_ignores as the parsedata
-	 * object.  If they did not, then look up the value of ignore_case */
-	if (parsedata != NULL)
-		ignore_case = ((git_ignores *)parsedata)->ignore_case;
+	/* either read ignore_case from ignores structure or use repo config */
+	if (payload != NULL)
+		ignore_case = ((git_ignores *)payload)->ignore_case;
 	else if (git_repository__cvar(&ignore_case, repo, GIT_CVAR_IGNORECASE) < 0)
-		return error;
-
-	if (ignores->key &&
-		git_path_root(ignores->key + 2) < 0 &&
-		git__suffixcmp(ignores->key, "/" GIT_IGNORE_FILE) == 0)
-		context = ignores->key + 2;
+		giterr_clear();
 
-	scan = buffer;
+	/* if subdir file path, convert context for file paths */
+	if (attrs->ce &&
+		git_path_root(attrs->ce->path) < 0 &&
+		!git__suffixcmp(attrs->ce->path, "/" GIT_IGNORE_FILE))
+		context = attrs->ce->path;
 
 	while (!error && *scan) {
 		if (!match) {
@@ -40,7 +41,7 @@ static int parse_ignore_file(
 		match->flags = GIT_ATTR_FNMATCH_ALLOWSPACE | GIT_ATTR_FNMATCH_ALLOWNEG;
 
 		if (!(error = git_attr_fnmatch__parse(
-			match, ignores->pool, context, &scan)))
+			match, &attrs->pool, context, &scan)))
 		{
 			match->flags |= GIT_ATTR_FNMATCH_IGNORE;
 
@@ -48,7 +49,7 @@ static int parse_ignore_file(
 				match->flags |= GIT_ATTR_FNMATCH_ICASE;
 
 			scan = git__next_line(scan);
-			error = git_vector_insert(&ignores->rules, match);
+			error = git_vector_insert(&attrs->rules, match);
 		}
 
 		if (error != 0) {
@@ -67,28 +68,46 @@ static int parse_ignore_file(
 	return error;
 }
 
-#define push_ignore_file(R,IGN,S,B,F) \
-	git_attr_cache__push_file((R),(B),(F),GIT_ATTR_FILE_FROM_FILE,parse_ignore_file,(IGN),(S))
+static int push_ignore_file(
+	git_ignores *ignores,
+	git_vector *which_list,
+	const char *base,
+	const char *filename)
+{
+	int error = 0;
+	git_attr_file *file = NULL;
+
+	if ((error = git_attr_cache__get(
+			&file, ignores->repo, GIT_ATTR_CACHE__FROM_FILE,
+			base, filename, parse_ignore_file, ignores)) < 0 ||
+		(error = git_vector_insert(which_list, file)) < 0)
+		git_attr_file__free(file);
+
+	return error;
+}
 
 static int push_one_ignore(void *payload, git_buf *path)
 {
 	git_ignores *ign = payload;
-
 	ign->depth++;
-
-	return push_ignore_file(
-		ign->repo, ign, &ign->ign_path, path->ptr, GIT_IGNORE_FILE);
+	return push_ignore_file(ign, &ign->ign_path, path->ptr, GIT_IGNORE_FILE);
 }
 
-static int get_internal_ignores(git_attr_file **ign, git_repository *repo)
+static int get_internal_ignores(git_attr_file **out, git_repository *repo)
 {
 	int error;
 
-	if (!(error = git_attr_cache__init(repo)))
-		error = git_attr_cache__internal_file(repo, GIT_IGNORE_INTERNAL, ign);
+	if ((error = git_attr_cache__init(repo)) < 0)
+		return error;
+
+	/* get with NULL parser, gives existing or empty git_attr_file */
+	error = git_attr_cache__get(
+		out, repo, GIT_ATTR_CACHE__FROM_FILE,
+		NULL, GIT_IGNORE_INTERNAL, NULL, NULL);
 
-	if (!error && !(*ign)->rules.length)
-		error = parse_ignore_file(repo, NULL, GIT_IGNORE_DEFAULT_RULES, *ign);
+	/* if internal rules list is empty, insert default rules */
+	if (!error && !(*out)->rules.length)
+		error = parse_ignore_file(repo, *out, GIT_IGNORE_DEFAULT_RULES, NULL);
 
 	return error;
 }
@@ -127,8 +146,7 @@ int git_ignore__for_path(
 		goto cleanup;
 
 	/* set up internals */
-	error = get_internal_ignores(&ignores->ign_internal, repo);
-	if (error < 0)
+	if ((error = get_internal_ignores(&ignores->ign_internal, repo)) < 0)
 		goto cleanup;
 
 	/* load .gitignore up the path */
@@ -140,14 +158,16 @@ int git_ignore__for_path(
 	}
 
 	/* load .git/info/exclude */
-	error = push_ignore_file(repo, ignores, &ignores->ign_global,
+	error = push_ignore_file(
+		ignores, &ignores->ign_global,
 		git_repository_path(repo), GIT_IGNORE_FILE_INREPO);
 	if (error < 0)
 		goto cleanup;
 
 	/* load core.excludesfile */
 	if (git_repository_attr_cache(repo)->cfg_excl_file != NULL)
-		error = push_ignore_file(repo, ignores, &ignores->ign_global, NULL,
+		error = push_ignore_file(
+			ignores, &ignores->ign_global, NULL,
 			git_repository_attr_cache(repo)->cfg_excl_file);
 
 cleanup:
@@ -165,35 +185,33 @@ int git_ignore__push_dir(git_ignores *ign, const char *dir)
 	ign->depth++;
 
 	return push_ignore_file(
-		ign->repo, ign, &ign->ign_path, ign->dir.ptr, GIT_IGNORE_FILE);
+		ign, &ign->ign_path, ign->dir.ptr, GIT_IGNORE_FILE);
 }
 
 int git_ignore__pop_dir(git_ignores *ign)
 {
 	if (ign->ign_path.length > 0) {
 		git_attr_file *file = git_vector_last(&ign->ign_path);
-		const char *start, *end, *scan;
-		size_t keylen;
+		const char *start = file->ce->path, *end;
 
-		/* - ign->dir looks something like "a/b/" (or "a/b/c/d/")
-		 * - file->key looks something like "0#a/b/.gitignore
+		/* - ign->dir looks something like "/home/user/a/b/" (or "a/b/c/d/")
+		 * - file->path looks something like "a/b/.gitignore
 		 *
-		 * We are popping the last directory off ign->dir.  We also want to
-		 * remove the file from the vector if the directory part of the key
-		 * matches the ign->dir path.  We need to test if the "a/b" part of
+		 * We are popping the last directory off ign->dir.  We also want
+		 * to remove the file from the vector if the popped directory
+		 * matches the ignore path.  We need to test if the "a/b" part of
 		 * the file key matches the path we are about to pop.
 		 */
 
-		for (start = end = scan = &file->key[2]; *scan; ++scan)
-			if (*scan == '/')
-				end = scan; /* point 'end' to last '/' in key */
-		keylen = (end - start) + 1;
+		if ((end = strrchr(start, '/')) != NULL) {
+			size_t dirlen = (end - start) + 1;
 
-		if (ign->dir.size >= keylen &&
-			!memcmp(ign->dir.ptr + ign->dir.size - keylen, start, keylen))
-		{
-			git_attr_file__free(git_vector_last(&ign->ign_path));
-			git_vector_pop(&ign->ign_path);
+			if (ign->dir.size >= dirlen &&
+				!memcmp(ign->dir.ptr + ign->dir.size - dirlen, start, dirlen))
+			{
+				git_vector_pop(&ign->ign_path);
+				git_attr_file__free(file);
+			}
 		}
 	}
 
@@ -210,7 +228,7 @@ void git_ignore__free(git_ignores *ignores)
 	unsigned int i;
 	git_attr_file *file;
 
-	/* don't need to free ignores->ign_internal it is cached exactly once */
+	git_attr_file__free(ignores->ign_internal);
 
 	git_vector_foreach(&ignores->ign_path, i, file) {
 		git_attr_file__free(file);
@@ -283,10 +301,12 @@ int git_ignore_add_rule(
 	const char *rules)
 {
 	int error;
-	git_attr_file *ign_internal;
+	git_attr_file *ign_internal = NULL;
 
-	if (!(error = get_internal_ignores(&ign_internal, repo)))
+	if (!(error = get_internal_ignores(&ign_internal, repo))) {
 		error = parse_ignore_file(repo, NULL, rules, ign_internal);
+		git_attr_file__free(ign_internal);
+	}
 
 	return error;
 }
@@ -300,8 +320,10 @@ int git_ignore_clear_internal_rules(
 	if (!(error = get_internal_ignores(&ign_internal, repo))) {
 		git_attr_file__clear_rules(ign_internal);
 
-		return parse_ignore_file(
+		error = parse_ignore_file(
 			repo, NULL, GIT_IGNORE_DEFAULT_RULES, ign_internal);
+
+		git_attr_file__free(ign_internal);
 	}
 
 	return error;

src/index.c

@@ -607,8 +607,15 @@ int git_index_read(git_index *index, int force)
 	}
 
 	updated = git_futils_filestamp_check(&stamp, index->index_file_path);
-	if (updated < 0 || (!updated && !force))
+	if (updated < 0) {
+		giterr_set(
+			GITERR_INDEX,
+			"Failed to read index: '%s' no longer exists",
+			index->index_file_path);
 		return updated;
+	}
+	if (!updated && !force)
+		return 0;
 
 	error = git_futils_readbuffer(&buffer, index->index_file_path);
 	if (error < 0)
@@ -667,11 +674,11 @@ int git_index_write(git_index *index)
 	if ((error = git_filebuf_commit(&file)) < 0)
 		return error;
 
-	error = git_futils_filestamp_check(&index->stamp, index->index_file_path);
-	if (error < 0)
-		return error;
+	if (git_futils_filestamp_check(&index->stamp, index->index_file_path) < 0)
+		/* index could not be read from disk! */;
+	else
+		index->on_disk = 1;
 
-	index->on_disk = 1;
 	return 0;
 }
 

src/sortedcache.c

@@ -232,9 +232,8 @@ unlock:
 
 void git_sortedcache_updated(git_sortedcache *sc)
 {
-	 /* update filestamp to latest value */
-	if (git_futils_filestamp_check(&sc->stamp, sc->path) < 0)
-		giterr_clear();
+	/* update filestamp to latest value */
+	git_futils_filestamp_check(&sc->stamp, sc->path);
 }
 
 /* release all items in sorted cache */

src/submodule.c

@@ -1693,8 +1693,6 @@ static int submodule_cache_refresh(git_submodule_cache *cache, int refresh)
 		update_gitmod = (wd != NULL) ?
 			git_futils_filestamp_check(&cache->gitmodules_stamp, path.ptr) :
 			(cache->gitmodules_stamp.mtime != 0);
-		if (update_gitmod < 0)
-			giterr_clear();
 	}
 
 	/* clear submodule flags that are to be refreshed */

tests/attr/file.c

@@ -11,9 +11,9 @@ void test_attr_file__simple_read(void)
 	git_attr_assignment *assign;
 	git_attr_rule *rule;
 
-	cl_git_pass(git_attr_file__new_and_load(&file, cl_fixture("attr/attr0")));
+	cl_git_pass(git_attr_file__load_standalone(&file, cl_fixture("attr/attr0")));
 
-	cl_assert_equal_s(cl_fixture("attr/attr0"), file->key + 2);
+	cl_assert_equal_s(cl_fixture("attr/attr0"), file->ce->path);
 	cl_assert(file->rules.length == 1);
 
 	rule = get_rule(0);
@@ -37,9 +37,9 @@ void test_attr_file__match_variants(void)
 	git_attr_rule *rule;
 	git_attr_assignment *assign;
 
-	cl_git_pass(git_attr_file__new_and_load(&file, cl_fixture("attr/attr1")));
+	cl_git_pass(git_attr_file__load_standalone(&file, cl_fixture("attr/attr1")));
 
-	cl_assert_equal_s(cl_fixture("attr/attr1"), file->key + 2);
+	cl_assert_equal_s(cl_fixture("attr/attr1"), file->ce->path);
 	cl_assert(file->rules.length == 10);
 
 	/* let's do a thorough check of this rule, then just verify
@@ -121,9 +121,9 @@ void test_attr_file__assign_variants(void)
 	git_attr_rule *rule;
 	git_attr_assignment *assign;
 
-	cl_git_pass(git_attr_file__new_and_load(&file, cl_fixture("attr/attr2")));
+	cl_git_pass(git_attr_file__load_standalone(&file, cl_fixture("attr/attr2")));
 
-	cl_assert_equal_s(cl_fixture("attr/attr2"), file->key + 2);
+	cl_assert_equal_s(cl_fixture("attr/attr2"), file->ce->path);
 	cl_assert(file->rules.length == 11);
 
 	check_one_assign(file, 0, 0, "pat0", "simple", EXPECT_TRUE, NULL);
@@ -187,8 +187,8 @@ void test_attr_file__check_attr_examples(void)
 	git_attr_rule *rule;
 	git_attr_assignment *assign;
 
-	cl_git_pass(git_attr_file__new_and_load(&file, cl_fixture("attr/attr3")));
-	cl_assert_equal_s(cl_fixture("attr/attr3"), file->key + 2);
+	cl_git_pass(git_attr_file__load_standalone(&file, cl_fixture("attr/attr3")));
+	cl_assert_equal_s(cl_fixture("attr/attr3"), file->ce->path);
 	cl_assert(file->rules.length == 3);
 
 	rule = get_rule(0);

tests/attr/lookup.c

@@ -9,8 +9,8 @@ void test_attr_lookup__simple(void)
 	git_attr_path path;
 	const char *value = NULL;
 
-	cl_git_pass(git_attr_file__new_and_load(&file, cl_fixture("attr/attr0")));
-	cl_assert_equal_s(cl_fixture("attr/attr0"), file->key + 2);
+	cl_git_pass(git_attr_file__load_standalone(&file, cl_fixture("attr/attr0")));
+	cl_assert_equal_s(cl_fixture("attr/attr0"), file->ce->path);
 	cl_assert(file->rules.length == 1);
 
 	cl_git_pass(git_attr_path__init(&path, "test", NULL));
@@ -129,8 +129,8 @@ void test_attr_lookup__match_variants(void)
 		{ NULL, NULL, 0, NULL }
 	};
 
-	cl_git_pass(git_attr_file__new_and_load(&file, cl_fixture("attr/attr1")));
-	cl_assert_equal_s(cl_fixture("attr/attr1"), file->key + 2);
+	cl_git_pass(git_attr_file__load_standalone(&file, cl_fixture("attr/attr1")));
+	cl_assert_equal_s(cl_fixture("attr/attr1"), file->ce->path);
 	cl_assert(file->rules.length == 10);
 
 	cl_git_pass(git_attr_path__init(&path, "/testing/for/pat0", NULL));
@@ -190,7 +190,7 @@ void test_attr_lookup__assign_variants(void)
 		{ NULL, NULL, 0, NULL }
 	};
 
-	cl_git_pass(git_attr_file__new_and_load(&file, cl_fixture("attr/attr2")));
+	cl_git_pass(git_attr_file__load_standalone(&file, cl_fixture("attr/attr2")));
 	cl_assert(file->rules.length == 11);
 
 	run_test_cases(file, cases, 0);
@@ -225,7 +225,7 @@ void test_attr_lookup__check_attr_examples(void)
 		{ NULL, NULL, 0, NULL }
 	};
 
-	cl_git_pass(git_attr_file__new_and_load(&file, cl_fixture("attr/attr3")));
+	cl_git_pass(git_attr_file__load_standalone(&file, cl_fixture("attr/attr3")));
 	cl_assert(file->rules.length == 3);
 
 	run_test_cases(file, cases, 0);
@@ -250,9 +250,9 @@ void test_attr_lookup__from_buffer(void)
 		{ NULL, NULL, 0, NULL }
 	};
 
-	cl_git_pass(git_attr_file__new(&file, 0, NULL, NULL));
+	cl_git_pass(git_attr_file__new(&file, NULL, 0));
 
-	cl_git_pass(git_attr_file__parse_buffer(NULL, NULL, "a* foo\nabc bar\n* baz", file));
+	cl_git_pass(git_attr_file__parse_buffer(NULL, file, "a* foo\nabc bar\n* baz", NULL));
 
 	cl_assert(file->rules.length == 3);
 

tests/threads/diff.c

 #include "clar_libgit2.h"
-#include "thread-utils.h"
+#include "thread_helpers.h"
 
 static git_repository *_repo;
 static git_tree *_a, *_b;
 static git_atomic _counts[4];
 static int _check_counts;
 
+#define THREADS 20
+
 void test_threads_diff__cleanup(void)
 {
 	cl_git_sandbox_cleanup();
 }
 
-static void run_in_parallel(
-	int repeats, int threads, void *(*func)(void *),
-	void (*before_test)(void), void (*after_test)(void))
-{
-	int r, t, *id = git__calloc(threads, sizeof(int));
-#ifdef GIT_THREADS
-	git_thread *th = git__calloc(threads, sizeof(git_thread));
-	cl_assert(th != NULL);
-#else
-	void *th = NULL;
-#endif
-
-	cl_assert(id != NULL);
-
-	for (r = 0; r < repeats; ++r) {
-		_repo = cl_git_sandbox_reopen(); /* reopen sandbox to flush caches */
-
-		if (before_test) before_test();
-
-		for (t = 0; t < threads; ++t) {
-			id[t] = t;
-#ifdef GIT_THREADS
-			cl_git_pass(git_thread_create(&th[t], NULL, func, &id[t]));
-#else
-			cl_assert(func(&id[t]) == &id[t]);
-#endif
-		}
-
-#ifdef GIT_THREADS
-		for (t = 0; t < threads; ++t)
-			cl_git_pass(git_thread_join(th[t], NULL));
-		memset(th, 0, threads * sizeof(git_thread));
-#endif
-
-		if (after_test) after_test();
-	}
-
-	git__free(id);
-	git__free(th);
-}
-
 static void setup_trees(void)
 {
+	_repo = cl_git_sandbox_reopen(); /* reopen sandbox to flush caches */
+
 	cl_git_pass(git_revparse_single(
 		(git_object **)&_a, _repo, "0017bd4ab1^{tree}"));
 	cl_git_pass(git_revparse_single(
@@ -62,8 +25,6 @@ static void setup_trees(void)
 	memset(_counts, 0, sizeof(_counts));
 }
 
-#define THREADS 20
-
 static void free_trees(void)
 {
 	git_tree_free(_a); _a = NULL;

tests/threads/iterator.c

+#include "clar_libgit2.h"
+#include "thread_helpers.h"
+#include "iterator.h"
+
+static git_repository *_repo;
+
+void test_threads_iterator__cleanup(void)
+{
+	cl_git_sandbox_cleanup();
+}
+
+static void *run_workdir_iterator(void *arg)
+{
+	int error = 0, thread = *(int *)arg;
+	git_iterator *iter;
+	const git_index_entry *entry = NULL;
+
+	cl_git_pass(git_iterator_for_workdir(
+		&iter, _repo, GIT_ITERATOR_DONT_AUTOEXPAND, NULL, NULL));
+
+	while (!error) {
+		if (entry && entry->mode == GIT_FILEMODE_TREE) {
+			error = git_iterator_advance_into(&entry, iter);
+
+			if (error == GIT_ENOTFOUND)
+				error = git_iterator_advance(&entry, iter);
+		} else {
+			error = git_iterator_advance(&entry, iter);
+		}
+
+		if (!error)
+			(void)git_iterator_current_is_ignored(iter);
+	}
+
+	cl_assert_equal_i(GIT_ITEROVER, error);
+
+	git_iterator_free(iter);
+
+	return arg;
+}
+
+
+void test_threads_iterator__workdir(void)
+{
+	_repo = cl_git_sandbox_init("status");
+
+	run_in_parallel(
+		1, 20, run_workdir_iterator, NULL, NULL);
+}

tests/threads/thread_helpers.c

+#include "clar_libgit2.h"
+#include "thread_helpers.h"
+
+void run_in_parallel(
+	int repeats,
+	int threads,
+	void *(*func)(void *),
+	void (*before_test)(void),
+	void (*after_test)(void))
+{
+	int r, t, *id = git__calloc(threads, sizeof(int));
+#ifdef GIT_THREADS
+	git_thread *th = git__calloc(threads, sizeof(git_thread));
+	cl_assert(th != NULL);
+#else
+	void *th = NULL;
+#endif
+
+	cl_assert(id != NULL);
+
+	for (r = 0; r < repeats; ++r) {
+		if (before_test) before_test();
+
+		for (t = 0; t < threads; ++t) {
+			id[t] = t;
+#ifdef GIT_THREADS
+			cl_git_pass(git_thread_create(&th[t], NULL, func, &id[t]));
+#else
+			cl_assert(func(&id[t]) == &id[t]);
+#endif
+		}
+
+#ifdef GIT_THREADS
+		for (t = 0; t < threads; ++t)
+			cl_git_pass(git_thread_join(th[t], NULL));
+		memset(th, 0, threads * sizeof(git_thread));
+#endif
+
+		if (after_test) after_test();
+	}
+
+	git__free(id);
+	git__free(th);
+}

tests/threads/thread_helpers.h

+#include "thread-utils.h"
+
+void run_in_parallel(
+	int repeats,
+	int threads,
+	void *(*func)(void *),
+	void (*before_test)(void),
+	void (*after_test)(void));