From 90656858ce6ec0f4cba5ba5f8690ace9b83161d0 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 21 Sep 2021 11:28:39 -0400 Subject: filter: use a `git_oid` in filter options, not a pointer 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. --- include/git2/attr.h | 8 +++++++- include/git2/blob.h | 8 +++++++- include/git2/filter.h | 8 +++++++- src/attr.c | 25 +++++++++++++++++++------ src/attr_file.c | 34 ++++++++++++++++++++++------------ src/attr_file.h | 5 +++-- src/blob.c | 8 +++++++- src/filter.c | 8 +++++++- tests/filter/bare.c | 10 ++-------- 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 */ -- cgit v1.2.1