filter: use a `git_oid` in filter options, not a pointerethomson/filter_commit_id

Using a `git_oid *` in filter options was a mistake; it is a deviation
from our typical pattern, and callers in some languages that GC may need
very special treatment in order to pass both an options structure and a
pointer outside of it.

9 files changed, 81 insertions, 33 deletions

diff --git a/include/git2/attr.h b/include/git2/attr.h
index 62c2ed6e7..3891a0c97 100644
--- a/include/git2/attr.h
+++ b/include/git2/attr.h
@@ -147,11 +147,17 @@ typedef struct {
 	/** A combination of GIT_ATTR_CHECK flags */
 	unsigned int flags;
 
+#ifdef GIT_DEPRECATE_HARD
+	void *reserved;
+#else
+	git_oid *commit_id;
+#endif
+
 	/**
 	 * The commit to load attributes from, when
 	 * `GIT_ATTR_CHECK_INCLUDE_COMMIT` is specified.
 	 */
-	git_oid *commit_id;
+	git_oid attr_commit_id;
 } git_attr_options;
 
 #define GIT_ATTR_OPTIONS_VERSION 1
diff --git a/include/git2/blob.h b/include/git2/blob.h
index fceb5c771..8fc73919d 100644
--- a/include/git2/blob.h
+++ b/include/git2/blob.h
@@ -135,11 +135,17 @@ typedef struct {
 	/** Flags to control the filtering process, see `git_blob_filter_flag_t` above */
 	uint32_t flags;
 
+#ifdef GIT_DEPRECATE_HARD
+	void *reserved;
+#else
+	git_oid *commit_id;
+#endif
+
 	/**
 	 * The commit to load attributes from, when
 	 * `GIT_BLOB_FILTER_ATTRIBUTES_FROM_COMMIT` is specified.
 	 */
-	git_oid *commit_id;
+	git_oid attr_commit_id;
 } git_blob_filter_options;
 
 #define GIT_BLOB_FILTER_OPTIONS_VERSION 1
diff --git a/include/git2/filter.h b/include/git2/filter.h
index 044c3b870..0465e5b14 100644
--- a/include/git2/filter.h
+++ b/include/git2/filter.h
@@ -66,11 +66,17 @@ typedef struct {
 	/** See `git_filter_flag_t` above */
 	uint32_t flags;
 
+#ifdef GIT_DEPRECATE_HARD
+	void *reserved;
+#else
+	git_oid *commit_id;
+#endif
+
 	/**
 	 * The commit to load attributes from, when
 	 * `GIT_FILTER_ATTRIBUTES_FROM_COMMIT` is specified.
 	 */
-	git_oid *commit_id;
+	git_oid attr_commit_id;
 } git_filter_options;
 
  #define GIT_FILTER_OPTIONS_VERSION 1
diff --git a/src/attr.c b/src/attr.c
index 03b720c5a..14eab5b46 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -382,7 +382,7 @@ static int attr_setup(
 {
 	git_buf system = GIT_BUF_INIT, info = GIT_BUF_INIT;
 	git_attr_file_source index_source = { GIT_ATTR_FILE_SOURCE_INDEX, NULL, GIT_ATTR_FILE, NULL };
-	git_attr_file_source head_source = { GIT_ATTR_FILE_SOURCE_COMMIT, NULL, GIT_ATTR_FILE, NULL };
+	git_attr_file_source head_source = { GIT_ATTR_FILE_SOURCE_HEAD, NULL, GIT_ATTR_FILE, NULL };
 	git_attr_file_source commit_source = { GIT_ATTR_FILE_SOURCE_COMMIT, NULL, GIT_ATTR_FILE, NULL };
 	git_index *idx = NULL;
 	const char *workdir;
@@ -432,7 +432,12 @@ static int attr_setup(
 		goto out;
 
 	if ((opts && (opts->flags & GIT_ATTR_CHECK_INCLUDE_COMMIT) != 0)) {
-		commit_source.commit_id = opts->commit_id;
+#ifndef GIT_DEPRECATE_HARD
+		if (opts->commit_id)
+			commit_source.commit_id = opts->commit_id;
+		else
+#endif
+		commit_source.commit_id = &opts->attr_commit_id;
 
 		if ((error = preload_attr_source(repo, attr_session, &commit_source)) < 0)
 			goto out;
@@ -521,8 +526,10 @@ static int attr_decide_sources(
 		break;
 	}
 
-	if ((flags & GIT_ATTR_CHECK_INCLUDE_HEAD) != 0 ||
-	    (flags & GIT_ATTR_CHECK_INCLUDE_COMMIT) != 0)
+	if ((flags & GIT_ATTR_CHECK_INCLUDE_HEAD) != 0)
+		srcs[count++] = GIT_ATTR_FILE_SOURCE_HEAD;
+
+	if ((flags & GIT_ATTR_CHECK_INCLUDE_COMMIT) != 0)
 		srcs[count++] = GIT_ATTR_FILE_SOURCE_COMMIT;
 
 	return count;
@@ -582,8 +589,14 @@ static int push_one_attr(void *ref, const char *path)
 	for (i = 0; !error && i < n_src; ++i) {
 		git_attr_file_source source = { src[i], path, GIT_ATTR_FILE };
 
-		if (src[i] == GIT_ATTR_FILE_SOURCE_COMMIT && info->opts)
-			source.commit_id = info->opts->commit_id;
+		if (src[i] == GIT_ATTR_FILE_SOURCE_COMMIT && info->opts) {
+#ifndef GIT_DEPRECATE_HARD
+			if (info->opts->commit_id)
+				source.commit_id = info->opts->commit_id;
+			else
+#endif
+			source.commit_id = &info->opts->attr_commit_id;
+		}
 
 		error = push_attr_source(info->repo, info->attr_session, info->files,
 		                       &source, allow_macros);
diff --git a/src/attr_file.c b/src/attr_file.c
index f8627381c..694967a1c 100644
--- a/src/attr_file.c
+++ b/src/attr_file.c
@@ -163,8 +163,9 @@ int git_attr_file__load(
 
 		break;
 	}
+	case GIT_ATTR_FILE_SOURCE_HEAD:
 	case GIT_ATTR_FILE_SOURCE_COMMIT: {
-		if (source->commit_id) {
+		if (source->type == GIT_ATTR_FILE_SOURCE_COMMIT) {
 			if ((error = git_commit_lookup(&commit, repo, source->commit_id)) < 0 ||
 			    (error = git_commit_tree(&tree, commit)) < 0)
 				goto cleanup;
@@ -234,6 +235,8 @@ int git_attr_file__load(
 		file->nonexistent = 1;
 	else if (source->type == GIT_ATTR_FILE_SOURCE_INDEX)
 		git_oid_cpy(&file->cache_data.oid, git_blob_id(blob));
+	else if (source->type == GIT_ATTR_FILE_SOURCE_HEAD)
+		git_oid_cpy(&file->cache_data.oid, git_tree_id(tree));
 	else if (source->type == GIT_ATTR_FILE_SOURCE_COMMIT)
 		git_oid_cpy(&file->cache_data.oid, git_tree_id(tree));
 	else if (source->type == GIT_ATTR_FILE_SOURCE_FILE)
@@ -288,22 +291,29 @@ int git_attr_file__out_of_date(
 		return (git_oid__cmp(&file->cache_data.oid, &id) != 0);
 	}
 
-	case GIT_ATTR_FILE_SOURCE_COMMIT: {
+	case GIT_ATTR_FILE_SOURCE_HEAD: {
 		git_tree *tree = NULL;
-		int error;
+		int error = git_repository_head_tree(&tree, repo);
 
-		if (source->commit_id) {
-			git_commit *commit = NULL;
+		if (error < 0)
+			return error;
 
-			if ((error = git_commit_lookup(&commit, repo, source->commit_id)) < 0)
-				return error;
+		error = (git_oid__cmp(&file->cache_data.oid, git_tree_id(tree)) != 0);
 
-			error = git_commit_tree(&tree, commit);
+		git_tree_free(tree);
+		return error;
+	}
 
-			git_commit_free(commit);
-		} else {
-			error = git_repository_head_tree(&tree, repo);
-		}
+	case GIT_ATTR_FILE_SOURCE_COMMIT: {
+		git_commit *commit = NULL;
+		git_tree *tree = NULL;
+		int error;
+
+		if ((error = git_commit_lookup(&commit, repo, source->commit_id)) < 0)
+			return error;
+
+		error = git_commit_tree(&tree, commit);
+		git_commit_free(commit);
 
 		if (error < 0)
 			return error;
diff --git a/src/attr_file.h b/src/attr_file.h
index 16e33caf1..a07cb4268 100644
--- a/src/attr_file.h
+++ b/src/attr_file.h
@@ -40,9 +40,10 @@ typedef enum {
 	GIT_ATTR_FILE_SOURCE_MEMORY = 0,
 	GIT_ATTR_FILE_SOURCE_FILE   = 1,
 	GIT_ATTR_FILE_SOURCE_INDEX  = 2,
-	GIT_ATTR_FILE_SOURCE_COMMIT = 3,
+	GIT_ATTR_FILE_SOURCE_HEAD   = 3,
+	GIT_ATTR_FILE_SOURCE_COMMIT = 4,
 
-	GIT_ATTR_FILE_NUM_SOURCES   = 4
+	GIT_ATTR_FILE_NUM_SOURCES   = 5
 } git_attr_file_source_t;
 
 typedef struct {
diff --git a/src/blob.c b/src/blob.c
index 79096ee95..06a4a0026 100644
--- a/src/blob.c
+++ b/src/blob.c
@@ -449,7 +449,13 @@ int git_blob_filter(
 
 	if ((opts.flags & GIT_BLOB_FILTER_ATTRIBUTES_FROM_COMMIT) != 0) {
 		filter_opts.flags |= GIT_FILTER_ATTRIBUTES_FROM_COMMIT;
-		filter_opts.commit_id = opts.commit_id;
+
+#ifndef GIT_DEPRECATE_HARD
+		if (opts.commit_id)
+			git_oid_cpy(&filter_opts.attr_commit_id, opts.commit_id);
+		else
+#endif
+		git_oid_cpy(&filter_opts.attr_commit_id, &opts.attr_commit_id);
 	}
 
 	if (!(error = git_filter_list_load_ext(
diff --git a/src/filter.c b/src/filter.c
index dd7d2f7ec..73497cb30 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -446,7 +446,13 @@ static int filter_list_check_attributes(
 
 	if ((src->options.flags & GIT_FILTER_ATTRIBUTES_FROM_COMMIT) != 0) {
 		attr_opts.flags |= GIT_ATTR_CHECK_INCLUDE_COMMIT;
-		attr_opts.commit_id = src->options.commit_id;
+
+#ifndef GIT_DEPRECATE_HARD
+		if (src->options.commit_id)
+			git_oid_cpy(&attr_opts.attr_commit_id, src->options.commit_id);
+		else
+#endif
+		git_oid_cpy(&attr_opts.attr_commit_id, &src->options.attr_commit_id);
 	}
 
 	error = git_attr_get_many_with_session(
diff --git a/tests/filter/bare.c b/tests/filter/bare.c
index f8e34232f..8402638ce 100644
--- a/tests/filter/bare.c
+++ b/tests/filter/bare.c
@@ -137,13 +137,10 @@ void test_filter_bare__from_specific_commit_one(void)
 	git_blob_filter_options opts = GIT_BLOB_FILTER_OPTIONS_INIT;
 	git_blob *blob;
 	git_buf buf = { 0 };
-	git_oid commit_id;
-
-	cl_git_pass(git_oid_fromstr(&commit_id, "b8986fec0f7bde90f78ac72706e782d82f24f2f0"));
 
 	opts.flags |= GIT_BLOB_FILTER_NO_SYSTEM_ATTRIBUTES;
 	opts.flags |= GIT_BLOB_FILTER_ATTRIBUTES_FROM_COMMIT;
-	opts.commit_id = &commit_id;
+	cl_git_pass(git_oid_fromstr(&opts.attr_commit_id, "b8986fec0f7bde90f78ac72706e782d82f24f2f0"));
 
 	cl_git_pass(git_revparse_single(
 		(git_object **)&blob, g_repo, "055c872")); /* ident */
@@ -165,13 +162,10 @@ void test_filter_bare__from_specific_commit_with_no_attributes_file(void)
 	git_blob_filter_options opts = GIT_BLOB_FILTER_OPTIONS_INIT;
 	git_blob *blob;
 	git_buf buf = { 0 };
-	git_oid commit_id;
-
-	cl_git_pass(git_oid_fromstr(&commit_id, "5afb6a14a864e30787857dd92af837e8cdd2cb1b"));
 
 	opts.flags |= GIT_BLOB_FILTER_NO_SYSTEM_ATTRIBUTES;
 	opts.flags |= GIT_BLOB_FILTER_ATTRIBUTES_FROM_COMMIT;
-	opts.commit_id = &commit_id;
+	cl_git_pass(git_oid_fromstr(&opts.attr_commit_id, "5afb6a14a864e30787857dd92af837e8cdd2cb1b"));
 
 	cl_git_pass(git_revparse_single(
 		(git_object **)&blob, g_repo, "799770d")); /* all-lf */