Commit 3d22394ab5e5f708e3f01296cbca071d3770ff2d

Patrick Steinhardt 2019-06-27T10:11:23

Merge pull request #4967 from tiennou/fix/4671 Incomplete commondir support

diff --git a/include/git2/repository.h b/include/git2/repository.h
index 364e0ea..45d7962 100644
--- a/include/git2/repository.h
+++ b/include/git2/repository.h
@@ -438,7 +438,8 @@ typedef enum {
 	GIT_REPOSITORY_ITEM_HOOKS,
 	GIT_REPOSITORY_ITEM_LOGS,
 	GIT_REPOSITORY_ITEM_MODULES,
-	GIT_REPOSITORY_ITEM_WORKTREES
+	GIT_REPOSITORY_ITEM_WORKTREES,
+	GIT_REPOSITORY_ITEM__LAST
 } git_repository_item_t;
 
 /**
diff --git a/include/git2/sys/repository.h b/include/git2/sys/repository.h
index 0c91421..ea2773f 100644
--- a/include/git2/sys/repository.h
+++ b/include/git2/sys/repository.h
@@ -25,6 +25,10 @@ GIT_BEGIN_DECL
  * Note that this is only useful if you wish to associate the repository
  * with a non-filesystem-backed object database and config store.
  *
+ * Caveats: since this repository has no physical location, some systems
+ * can fail to function properly: locations under $GIT_DIR, $GIT_COMMON_DIR,
+ * or $GIT_INFO_DIR are impacted.
+ *
  * @param out The blank repository
  * @return 0 on success, or an error code
  */
@@ -39,7 +43,7 @@ GIT_EXTERN(int) git_repository_new(git_repository **out);
  * There's no need to call this function directly unless you're
  * trying to aggressively cleanup the repo before its
  * deallocation. `git_repository_free` already performs this operation
- * before deallocation the repo.
+ * before deallocating the repo.
  */
 GIT_EXTERN(void) git_repository__cleanup(git_repository *repo);
 
diff --git a/src/attr.c b/src/attr.c
index bd8e415..f50d28c 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -306,10 +306,10 @@ static int system_attr_file(
 
 static int attr_setup(git_repository *repo, git_attr_session *attr_session)
 {
-	int error = 0;
-	const char *workdir = git_repository_workdir(repo);
-	git_index *idx = NULL;
 	git_buf path = GIT_BUF_INIT;
+	git_index *idx = NULL;
+	const char *workdir;
+	int error = 0;
 
 	if (attr_session && attr_session->init_setup)
 		return 0;
@@ -317,42 +317,38 @@ static int attr_setup(git_repository *repo, git_attr_session *attr_session)
 	if ((error = git_attr_cache__init(repo)) < 0)
 		return error;
 
-	/* preload attribute files that could contain macros so the
-	 * definitions will be available for later file parsing
+	/*
+	 * Preload attribute files that could contain macros so the
+	 * definitions will be available for later file parsing.
 	 */
 
-	error = system_attr_file(&path, attr_session);
-
-	if (error == 0)
-		error = preload_attr_file(
-			repo, attr_session, GIT_ATTR_FILE__FROM_FILE, NULL, path.ptr);
-
-	if (error != GIT_ENOTFOUND)
-		goto out;
-
-	if ((error = preload_attr_file(
-			repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
-			NULL, git_repository_attr_cache(repo)->cfg_attr_file)) < 0)
-		goto out;
+	if ((error = system_attr_file(&path, attr_session)) < 0 ||
+	    (error = preload_attr_file(repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
+				       NULL, path.ptr)) < 0) {
+		if (error != GIT_ENOTFOUND)
+			goto out;
+	}
 
-	if ((error = git_repository_item_path(&path,
-			repo, GIT_REPOSITORY_ITEM_INFO)) < 0)
+	if ((error = preload_attr_file(repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
+				       NULL, git_repository_attr_cache(repo)->cfg_attr_file)) < 0)
 		goto out;
 
-	if ((error = preload_attr_file(
-			repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
-			path.ptr, GIT_ATTR_FILE_INREPO)) < 0)
-		goto out;
+	if ((error = git_repository_item_path(&path, repo, GIT_REPOSITORY_ITEM_INFO)) < 0 ||
+	    (error = preload_attr_file(repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
+				       path.ptr, GIT_ATTR_FILE_INREPO)) < 0) {
+		if (error != GIT_ENOTFOUND)
+			goto out;
+	}
 
-	if (workdir != NULL &&
-		(error = preload_attr_file(
-			repo, attr_session, GIT_ATTR_FILE__FROM_FILE, workdir, GIT_ATTR_FILE)) < 0)
-		goto out;
+	if ((workdir = git_repository_workdir(repo)) != NULL &&
+	    (error = preload_attr_file(repo, attr_session, GIT_ATTR_FILE__FROM_FILE,
+				       workdir, GIT_ATTR_FILE)) < 0)
+			goto out;
 
 	if ((error = git_repository_index__weakptr(&idx, repo)) < 0 ||
-		(error = preload_attr_file(
-			repo, attr_session, GIT_ATTR_FILE__FROM_INDEX, NULL, GIT_ATTR_FILE)) < 0)
-		goto out;
+	    (error = preload_attr_file(repo, attr_session, GIT_ATTR_FILE__FROM_INDEX,
+				       NULL, GIT_ATTR_FILE)) < 0)
+			goto out;
 
 	if (attr_session)
 		attr_session->init_setup = 1;
@@ -516,15 +512,12 @@ static int collect_attr_files(
 	 * - $GIT_PREFIX/etc/gitattributes
 	 */
 
-	error = git_repository_item_path(&attrfile, repo, GIT_REPOSITORY_ITEM_INFO);
-	if (error < 0)
-		goto cleanup;
-
-	error = push_attr_file(
-		repo, attr_session, files, GIT_ATTR_FILE__FROM_FILE,
-		attrfile.ptr, GIT_ATTR_FILE_INREPO);
-	if (error < 0)
-		goto cleanup;
+	if ((error = git_repository_item_path(&attrfile, repo, GIT_REPOSITORY_ITEM_INFO)) < 0 ||
+	    (error = push_attr_file(repo, attr_session, files, GIT_ATTR_FILE__FROM_FILE,
+				    attrfile.ptr, GIT_ATTR_FILE_INREPO)) < 0) {
+		if (error != GIT_ENOTFOUND)
+			goto cleanup;
+	}
 
 	info.repo = repo;
 	info.attr_session = attr_session;
diff --git a/src/ignore.c b/src/ignore.c
index 3f748b8..b17714b 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -335,16 +335,13 @@ int git_ignore__for_path(
 			goto cleanup;
 	}
 
-	if ((error = git_repository_item_path(&infopath,
-			repo, GIT_REPOSITORY_ITEM_INFO)) < 0)
-		goto cleanup;
-
-	/* load .git/info/exclude */
-	error = push_ignore_file(
-		ignores, &ignores->ign_global,
-		infopath.ptr, GIT_IGNORE_FILE_INREPO);
-	if (error < 0)
-		goto cleanup;
+	/* load .git/info/exclude if possible */
+	if ((error = git_repository_item_path(&infopath, repo, GIT_REPOSITORY_ITEM_INFO)) < 0 ||
+		(error = push_ignore_file(ignores, &ignores->ign_global, infopath.ptr, GIT_IGNORE_FILE_INREPO)) < 0) {
+		if (error != GIT_ENOTFOUND)
+			goto cleanup;
+		error = 0;
+	}
 
 	/* load core.excludesfile */
 	if (git_repository_attr_cache(repo)->cfg_excl_file != NULL)
diff --git a/src/odb.c b/src/odb.c
index 0ede4a3..e04cf02 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -548,6 +548,7 @@ int git_odb__add_default_backends(
 #else
 	if (p_stat(objects_dir, &st) < 0) {
 		if (as_alternates)
+			/* this should warn */
 			return 0;
 
 		git_error_set(GIT_ERROR_ODB, "failed to load object database in '%s'", objects_dir);
diff --git a/src/odb.h b/src/odb.h
index 8c73515..79ed6b4 100644
--- a/src/odb.h
+++ b/src/odb.h
@@ -71,6 +71,7 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj);
  * Format the object header such as it would appear in the on-disk object
  */
 int git_odb__format_object_header(size_t *out_len, char *hdr, size_t hdr_size, git_off_t obj_len, git_object_t obj_type);
+
 /*
  * Hash an open file descriptor.
  * This is a performance call when the contents of a fd need to be hashed,
@@ -95,7 +96,7 @@ int git_odb__hashfd_filtered(
  * symlink, then the raw contents of the symlink will be hashed. Otherwise,
  * this will fallback to `git_odb__hashfd`.
  *
- * The hash type for this call is always `GIT_OBJIECT_BLOB` because
+ * The hash type for this call is always `GIT_OBJECT_BLOB` because
  * symlinks may only point to blobs.
  */
 int git_odb__hashlink(git_oid *out, const char *path);
diff --git a/src/repository.c b/src/repository.c
index 6d7954a..71386d6 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -42,23 +42,24 @@ bool git_repository__fsync_gitdir = false;
 
 static const struct {
     git_repository_item_t parent;
+	git_repository_item_t fallback;
     const char *name;
     bool directory;
 } items[] = {
-	{ GIT_REPOSITORY_ITEM_GITDIR, NULL, true },
-	{ GIT_REPOSITORY_ITEM_WORKDIR, NULL, true },
-	{ GIT_REPOSITORY_ITEM_COMMONDIR, NULL, true },
-	{ GIT_REPOSITORY_ITEM_GITDIR, "index", false },
-	{ GIT_REPOSITORY_ITEM_COMMONDIR, "objects", true },
-	{ GIT_REPOSITORY_ITEM_COMMONDIR, "refs", true },
-	{ GIT_REPOSITORY_ITEM_COMMONDIR, "packed-refs", false },
-	{ GIT_REPOSITORY_ITEM_COMMONDIR, "remotes", true },
-	{ GIT_REPOSITORY_ITEM_COMMONDIR, "config", false },
-	{ GIT_REPOSITORY_ITEM_COMMONDIR, "info", true },
-	{ GIT_REPOSITORY_ITEM_COMMONDIR, "hooks", true },
-	{ GIT_REPOSITORY_ITEM_COMMONDIR, "logs", true },
-	{ GIT_REPOSITORY_ITEM_GITDIR, "modules", true },
-	{ GIT_REPOSITORY_ITEM_COMMONDIR, "worktrees", true }
+	{ GIT_REPOSITORY_ITEM_GITDIR, GIT_REPOSITORY_ITEM__LAST, NULL, true },
+	{ GIT_REPOSITORY_ITEM_WORKDIR, GIT_REPOSITORY_ITEM__LAST, NULL, true },
+	{ GIT_REPOSITORY_ITEM_COMMONDIR, GIT_REPOSITORY_ITEM__LAST, NULL, true },
+	{ GIT_REPOSITORY_ITEM_GITDIR, GIT_REPOSITORY_ITEM__LAST, "index", false },
+	{ GIT_REPOSITORY_ITEM_COMMONDIR, GIT_REPOSITORY_ITEM_GITDIR, "objects", true },
+	{ GIT_REPOSITORY_ITEM_COMMONDIR, GIT_REPOSITORY_ITEM_GITDIR, "refs", true },
+	{ GIT_REPOSITORY_ITEM_COMMONDIR, GIT_REPOSITORY_ITEM_GITDIR, "packed-refs", false },
+	{ GIT_REPOSITORY_ITEM_COMMONDIR, GIT_REPOSITORY_ITEM_GITDIR, "remotes", true },
+	{ GIT_REPOSITORY_ITEM_COMMONDIR, GIT_REPOSITORY_ITEM_GITDIR, "config", false },
+	{ GIT_REPOSITORY_ITEM_COMMONDIR, GIT_REPOSITORY_ITEM_GITDIR, "info", true },
+	{ GIT_REPOSITORY_ITEM_COMMONDIR, GIT_REPOSITORY_ITEM_GITDIR, "hooks", true },
+	{ GIT_REPOSITORY_ITEM_COMMONDIR, GIT_REPOSITORY_ITEM_GITDIR, "logs", true },
+	{ GIT_REPOSITORY_ITEM_GITDIR, GIT_REPOSITORY_ITEM__LAST, "modules", true },
+	{ GIT_REPOSITORY_ITEM_COMMONDIR, GIT_REPOSITORY_ITEM_GITDIR, "worktrees", true }
 };
 
 static int check_repositoryformatversion(git_config *config);
@@ -2308,11 +2309,11 @@ int git_repository_is_empty(git_repository *repo)
 	return is_empty;
 }
 
-int git_repository_item_path(git_buf *out, const git_repository *repo, git_repository_item_t item)
+static const char *resolved_parent_path(const git_repository *repo, git_repository_item_t item, git_repository_item_t fallback)
 {
 	const char *parent;
 
-	switch (items[item].parent) {
+	switch (item) {
 		case GIT_REPOSITORY_ITEM_GITDIR:
 			parent = git_repository_path(repo);
 			break;
@@ -2324,9 +2325,17 @@ int git_repository_item_path(git_buf *out, const git_repository *repo, git_repos
 			break;
 		default:
 			git_error_set(GIT_ERROR_INVALID, "invalid item directory");
-			return -1;
+			return NULL;
 	}
+	if (!parent && fallback != GIT_REPOSITORY_ITEM__LAST)
+		return resolved_parent_path(repo, fallback, GIT_REPOSITORY_ITEM__LAST);
+
+	return parent;
+}
 
+int git_repository_item_path(git_buf *out, const git_repository *repo, git_repository_item_t item)
+{
+	const char *parent = resolved_parent_path(repo, items[item].parent, items[item].fallback);
 	if (parent == NULL) {
 		git_error_set(GIT_ERROR_INVALID, "path cannot exist in repository");
 		return GIT_ENOTFOUND;