Merge pull request libgit2#6067 from libgit2/ethomson/filter_commit_id

filter: use a `git_oid` in filter options, not a pointer

Author: ethomson

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

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

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

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);

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;

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 {

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(

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(

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 */