Merge pull request #1182 from libgit2/odb-file-refresh

Sane refresh logic for #1180

include/git2/odb.h

@@ -192,6 +192,26 @@ GIT_EXTERN(int) git_odb_read_header(size_t *len_out, git_otype *type_out, git_od
 GIT_EXTERN(int) git_odb_exists(git_odb *db, const git_oid *id);
 
 /**
+ * Refresh the object database to load newly added files.
+ *
+ * If the object databases have changed on disk while the library
+ * is running, this function will force a reload of the underlying
+ * indexes.
+ *
+ * Use this function when you're confident that an external
+ * application has tampered with the ODB.
+ *
+ * NOTE that it is not necessary to call this function at all. The
+ * library will automatically attempt to refresh the ODB
+ * when a lookup fails, to see if the looked up object exists
+ * on disk but hasn't been loaded yet.
+ *
+ * @param db database to refresh
+ * @return 0 on success, error code otherwise
+ */
+GIT_EXTERN(int) git_odb_refresh(struct git_odb *db);
+
+/**
  * List all objects available in the database
  *
  * The callback will be called for each object available in the

include/git2/odb_backend.h

@@ -89,6 +89,8 @@ struct git_odb_backend {
 			struct git_odb_backend *,
 			const git_oid *);
 
+	int (* refresh)(struct git_odb_backend *);
+
 	int (* foreach)(
 			struct git_odb_backend *,
 			git_odb_foreach_cb cb,

src/odb.c

@@ -529,6 +529,7 @@ int git_odb_exists(git_odb *db, const git_oid *id)
 	git_odb_object *object;
 	unsigned int i;
 	bool found = false;
+	bool refreshed = false;
 
 	assert(db && id);
 
@@ -537,6 +538,7 @@ int git_odb_exists(git_odb *db, const git_oid *id)
 		return (int)true;
 	}
 
+attempt_lookup:
 	for (i = 0; i < db->backends.length && !found; ++i) {
 		backend_internal *internal = git_vector_get(&db->backends, i);
 		git_odb_backend *b = internal->backend;
@@ -545,6 +547,16 @@ int git_odb_exists(git_odb *db, const git_oid *id)
 			found = b->exists(b, id);
 	}
 
+	if (!found && !refreshed) {
+		if (git_odb_refresh(db) < 0) {
+			giterr_clear();
+			return (int)false;
+		}
+
+		refreshed = true;
+		goto attempt_lookup;
+	}
+
 	return (int)found;
 }
 
@@ -608,15 +620,24 @@ int git_odb__read_header_or_object(
 int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id)
 {
 	unsigned int i;
-	int error = GIT_ENOTFOUND;
+	int error;
+	bool refreshed = false;
 	git_rawobj raw;
 
 	assert(out && db && id);
 
+	if (db->backends.length == 0) {
+		giterr_set(GITERR_ODB, "Failed to lookup object: no backends loaded");
+		return GIT_ENOTFOUND;
+	}
+
 	*out = git_cache_get(&db->cache, id);
 	if (*out != NULL)
 		return 0;
 
+attempt_lookup:
+	error = GIT_ENOTFOUND;
+
 	for (i = 0; i < db->backends.length && error < 0; ++i) {
 		backend_internal *internal = git_vector_get(&db->backends, i);
 		git_odb_backend *b = internal->backend;
@@ -625,9 +646,13 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id)
 			error = b->read(&raw.data, &raw.len, &raw.type, b, id);
 	}
 
-	/* TODO: If no backends are configured, this returns GIT_ENOTFOUND but
-	 * will never have called giterr_set().
-	 */
+	if (error == GIT_ENOTFOUND && !refreshed) {
+		if ((error = git_odb_refresh(db)) < 0)
+			return error;
+
+		refreshed = true;
+		goto attempt_lookup;
+	}
 
 	if (error && error != GIT_PASSTHROUGH)
 		return error;
@@ -644,7 +669,7 @@ int git_odb_read_prefix(
 	git_oid found_full_oid = {{0}};
 	git_rawobj raw;
 	void *data = NULL;
-	bool found = false;
+	bool found = false, refreshed = false;
 
 	assert(out && db);
 
@@ -660,11 +685,12 @@ int git_odb_read_prefix(
 			return 0;
 	}
 
+attempt_lookup:
 	for (i = 0; i < db->backends.length; ++i) {
 		backend_internal *internal = git_vector_get(&db->backends, i);
 		git_odb_backend *b = internal->backend;
 
-		if (b->read != NULL) {
+		if (b->read_prefix != NULL) {
 			git_oid full_oid;
 			error = b->read_prefix(&full_oid, &raw.data, &raw.len, &raw.type, b, short_id, len);
 			if (error == GIT_ENOTFOUND || error == GIT_PASSTHROUGH)
@@ -675,13 +701,23 @@ int git_odb_read_prefix(
 
 			git__free(data);
 			data = raw.data;
+
 			if (found && git_oid_cmp(&full_oid, &found_full_oid))
 				return git_odb__error_ambiguous("multiple matches for prefix");
+
 			found_full_oid = full_oid;
 			found = true;
 		}
 	}
 
+	if (!found && !refreshed) {
+		if ((error = git_odb_refresh(db)) < 0)
+			return error;
+
+		refreshed = true;
+		goto attempt_lookup;
+	}
+
 	if (!found)
 		return git_odb__error_notfound("no match for prefix", short_id);
 
@@ -820,12 +856,31 @@ int git_odb_write_pack(struct git_odb_writepack **out, git_odb *db, git_transfer
 	return error;
 }
 
-void * git_odb_backend_malloc(git_odb_backend *backend, size_t len)
+void *git_odb_backend_malloc(git_odb_backend *backend, size_t len)
 {
 	GIT_UNUSED(backend);
 	return git__malloc(len);
 }
 
+int git_odb_refresh(struct git_odb *db)
+{
+	unsigned int i;
+	assert(db);
+
+	for (i = 0; i < db->backends.length; ++i) {
+		backend_internal *internal = git_vector_get(&db->backends, i);
+		git_odb_backend *b = internal->backend;
+
+		if (b->refresh != NULL) {
+			int error = b->refresh(b);
+			if (error < 0)
+				return error;
+		}
+	}
+
+	return 0;
+}
+
 int git_odb__error_notfound(const char *message, const git_oid *oid)
 {
 	if (oid != NULL) {

src/odb_pack.c

@@ -138,7 +138,6 @@ static int pack_window_contains(git_mwindow *win, off_t offset);
 static int packfile_sort__cb(const void *a_, const void *b_);
 
 static int packfile_load__cb(void *_data, git_buf *path);
-static int packfile_refresh_all(struct pack_backend *backend);
 
 static int pack_entry_find(struct git_pack_entry *e,
 	struct pack_backend *backend, const git_oid *oid);
@@ -237,33 +236,6 @@ static int packfile_load__cb(void *_data, git_buf *path)
 	return git_vector_insert(&backend->packs, pack);
 }
 
-static int packfile_refresh_all(struct pack_backend *backend)
-{
-	int error;
-	struct stat st;
-	git_buf path = GIT_BUF_INIT;
-
-	if (backend->pack_folder == NULL)
-		return 0;
-
-	if (p_stat(backend->pack_folder, &st) < 0 || !S_ISDIR(st.st_mode))
-		return git_odb__error_notfound("failed to refresh packfiles", NULL);
-
-	git_buf_sets(&path, backend->pack_folder);
-
-	/* reload all packs */
-	error = git_path_direach(&path, packfile_load__cb, (void *)backend);
-
-	git_buf_free(&path);
-
-	if (error < 0)
-		return error;
-
-	git_vector_sort(&backend->packs);
-
-	return 0;
-}
-
 static int pack_entry_find_inner(
 	struct git_pack_entry *e,
 	struct pack_backend *backend,
@@ -294,7 +266,6 @@ static int pack_entry_find_inner(
 
 static int pack_entry_find(struct git_pack_entry *e, struct pack_backend *backend, const git_oid *oid)
 {
-	int error;
 	struct git_pack_file *last_found = backend->last_found;
 
 	if (backend->last_found &&
@@ -303,10 +274,6 @@ static int pack_entry_find(struct git_pack_entry *e, struct pack_backend *backen
 
 	if (!pack_entry_find_inner(e, backend, oid, last_found))
 		return 0;
-	if ((error = packfile_refresh_all(backend)) < 0)
-		return error;
-	if (!pack_entry_find_inner(e, backend, oid, last_found))
-		return 0;
 
 	return git_odb__error_notfound("failed to find pack entry", oid);
 }
@@ -356,17 +323,9 @@ static int pack_entry_find_prefix(
 	const git_oid *short_oid,
 	size_t len)
 {
-	unsigned found = 0;
-	int error;
 	struct git_pack_file *last_found = backend->last_found;
+	unsigned int found = pack_entry_find_prefix_inner(e, backend, short_oid, len, last_found);
 
-	if ((found = pack_entry_find_prefix_inner(e, backend, short_oid, len, last_found)) > 0)
-		goto cleanup;
-	if ((error = packfile_refresh_all(backend)) < 0)
-		return error;
-	found = pack_entry_find_prefix_inner(e, backend, short_oid, len, last_found);
-
-cleanup:
 	if (!found)
 		return git_odb__error_notfound("no matching pack entry for prefix", short_oid);
 	else if (found > 1)
@@ -383,6 +342,34 @@ cleanup:
  * Implement the git_odb_backend API calls
  *
  ***********************************************************/
+static int pack_backend__refresh(git_odb_backend *_backend)
+{
+	struct pack_backend *backend = (struct pack_backend *)_backend;
+
+	int error;
+	struct stat st;
+	git_buf path = GIT_BUF_INIT;
+
+	if (backend->pack_folder == NULL)
+		return 0;
+
+	if (p_stat(backend->pack_folder, &st) < 0 || !S_ISDIR(st.st_mode))
+		return git_odb__error_notfound("failed to refresh packfiles", NULL);
+
+	git_buf_sets(&path, backend->pack_folder);
+
+	/* reload all packs */
+	error = git_path_direach(&path, packfile_load__cb, (void *)backend);
+
+	git_buf_free(&path);
+
+	if (error < 0)
+		return error;
+
+	git_vector_sort(&backend->packs);
+	return 0;
+}
+
 
 static int pack_backend__read_header(size_t *len_p, git_otype *type_p, struct git_odb_backend *backend, const git_oid *oid)
 {
@@ -468,7 +455,7 @@ static int pack_backend__foreach(git_odb_backend *_backend, git_odb_foreach_cb c
 	backend = (struct pack_backend *)_backend;
 
 	/* Make sure we know about the packfiles */
-	if ((error = packfile_refresh_all(backend)) < 0)
+	if ((error = pack_backend__refresh(_backend)) < 0)
 		return error;
 
 	git_vector_foreach(&backend->packs, i, p) {
@@ -581,6 +568,7 @@ int git_odb_backend_one_pack(git_odb_backend **backend_out, const char *idx)
 	backend->parent.read_prefix = &pack_backend__read_prefix;
 	backend->parent.read_header = &pack_backend__read_header;
 	backend->parent.exists = &pack_backend__exists;
+	backend->parent.refresh = &pack_backend__refresh;
 	backend->parent.foreach = &pack_backend__foreach;
 	backend->parent.free = &pack_backend__free;
 
@@ -612,13 +600,19 @@ int git_odb_backend_pack(git_odb_backend **backend_out, const char *objects_dir)
 	}
 
 	if (git_path_isdir(git_buf_cstr(&path)) == true) {
+		int error;
+
 		backend->pack_folder = git_buf_detach(&path);
+		error = pack_backend__refresh((git_odb_backend *)backend);
+		if (error < 0)
+			return error;
 	}
 
 	backend->parent.read = &pack_backend__read;
 	backend->parent.read_prefix = &pack_backend__read_prefix;
 	backend->parent.read_header = &pack_backend__read_header;
 	backend->parent.exists = &pack_backend__exists;
+	backend->parent.refresh = &pack_backend__refresh;
 	backend->parent.foreach = &pack_backend__foreach;
 	backend->parent.writepack = &pack_backend__writepack;
 	backend->parent.free = &pack_backend__free;