Skip to content

Commit 9065685

Browse files
ethomsonethomson
committed
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.
1 parent ba01547 commit 9065685

File tree

9 files changed

+81
-33
lines changed

9 files changed

+81
-33
lines changed

‎include/git2/attr.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,17 @@ typedef struct {
147147
/** A combination of GIT_ATTR_CHECK flags */
148148
unsigned int flags;
149149

150+
#ifdef GIT_DEPRECATE_HARD
151+
void *reserved;
152+
#else
153+
git_oid *commit_id;
154+
#endif
155+
150156
/**
151157
* The commit to load attributes from, when
152158
* `GIT_ATTR_CHECK_INCLUDE_COMMIT` is specified.
153159
*/
154-
git_oid *commit_id;
160+
git_oid attr_commit_id;
155161
} git_attr_options;
156162

157163
#define GIT_ATTR_OPTIONS_VERSION 1

‎include/git2/blob.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,17 @@ typedef struct {
135135
/** Flags to control the filtering process, see `git_blob_filter_flag_t` above */
136136
uint32_t flags;
137137

138+
#ifdef GIT_DEPRECATE_HARD
139+
void *reserved;
140+
#else
141+
git_oid *commit_id;
142+
#endif
143+
138144
/**
139145
* The commit to load attributes from, when
140146
* `GIT_BLOB_FILTER_ATTRIBUTES_FROM_COMMIT` is specified.
141147
*/
142-
git_oid *commit_id;
148+
git_oid attr_commit_id;
143149
} git_blob_filter_options;
144150

145151
#define GIT_BLOB_FILTER_OPTIONS_VERSION 1

‎include/git2/filter.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,17 @@ typedef struct {
6666
/** See `git_filter_flag_t` above */
6767
uint32_t flags;
6868

69+
#ifdef GIT_DEPRECATE_HARD
70+
void *reserved;
71+
#else
72+
git_oid *commit_id;
73+
#endif
74+
6975
/**
7076
* The commit to load attributes from, when
7177
* `GIT_FILTER_ATTRIBUTES_FROM_COMMIT` is specified.
7278
*/
73-
git_oid *commit_id;
79+
git_oid attr_commit_id;
7480
} git_filter_options;
7581

7682
#define GIT_FILTER_OPTIONS_VERSION 1

‎src/attr.c‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ static int attr_setup(
382382
{
383383
git_buf system = GIT_BUF_INIT, info = GIT_BUF_INIT;
384384
git_attr_file_source index_source = { GIT_ATTR_FILE_SOURCE_INDEX, NULL, GIT_ATTR_FILE, NULL };
385-
git_attr_file_source head_source = { GIT_ATTR_FILE_SOURCE_COMMIT, NULL, GIT_ATTR_FILE, NULL };
385+
git_attr_file_source head_source = { GIT_ATTR_FILE_SOURCE_HEAD, NULL, GIT_ATTR_FILE, NULL };
386386
git_attr_file_source commit_source = { GIT_ATTR_FILE_SOURCE_COMMIT, NULL, GIT_ATTR_FILE, NULL };
387387
git_index *idx = NULL;
388388
const char *workdir;
@@ -432,7 +432,12 @@ static int attr_setup(
432432
goto out;
433433

434434
if ((opts && (opts->flags & GIT_ATTR_CHECK_INCLUDE_COMMIT) != 0)) {
435-
commit_source.commit_id = opts->commit_id;
435+
#ifndef GIT_DEPRECATE_HARD
436+
if (opts->commit_id)
437+
commit_source.commit_id = opts->commit_id;
438+
else
439+
#endif
440+
commit_source.commit_id = &opts->attr_commit_id;
436441

437442
if ((error = preload_attr_source(repo, attr_session, &commit_source)) < 0)
438443
goto out;
@@ -521,8 +526,10 @@ static int attr_decide_sources(
521526
break;
522527
}
523528

524-
if ((flags & GIT_ATTR_CHECK_INCLUDE_HEAD) != 0 ||
525-
(flags & GIT_ATTR_CHECK_INCLUDE_COMMIT) != 0)
529+
if ((flags & GIT_ATTR_CHECK_INCLUDE_HEAD) != 0)
530+
srcs[count++] = GIT_ATTR_FILE_SOURCE_HEAD;
531+
532+
if ((flags & GIT_ATTR_CHECK_INCLUDE_COMMIT) != 0)
526533
srcs[count++] = GIT_ATTR_FILE_SOURCE_COMMIT;
527534

528535
return count;
@@ -582,8 +589,14 @@ static int push_one_attr(void *ref, const char *path)
582589
for (i = 0; !error && i < n_src; ++i) {
583590
git_attr_file_source source = { src[i], path, GIT_ATTR_FILE };
584591

585-
if (src[i] == GIT_ATTR_FILE_SOURCE_COMMIT && info->opts)
586-
source.commit_id = info->opts->commit_id;
592+
if (src[i] == GIT_ATTR_FILE_SOURCE_COMMIT && info->opts) {
593+
#ifndef GIT_DEPRECATE_HARD
594+
if (info->opts->commit_id)
595+
source.commit_id = info->opts->commit_id;
596+
else
597+
#endif
598+
source.commit_id = &info->opts->attr_commit_id;
599+
}
587600

588601
error = push_attr_source(info->repo, info->attr_session, info->files,
589602
&source, allow_macros);

‎src/attr_file.c‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,9 @@ int git_attr_file__load(
163163

164164
break;
165165
}
166+
case GIT_ATTR_FILE_SOURCE_HEAD:
166167
case GIT_ATTR_FILE_SOURCE_COMMIT: {
167-
if (source->commit_id) {
168+
if (source->type == GIT_ATTR_FILE_SOURCE_COMMIT) {
168169
if ((error = git_commit_lookup(&commit, repo, source->commit_id)) < 0 ||
169170
(error = git_commit_tree(&tree, commit)) < 0)
170171
goto cleanup;
@@ -234,6 +235,8 @@ int git_attr_file__load(
234235
file->nonexistent = 1;
235236
else if (source->type == GIT_ATTR_FILE_SOURCE_INDEX)
236237
git_oid_cpy(&file->cache_data.oid, git_blob_id(blob));
238+
else if (source->type == GIT_ATTR_FILE_SOURCE_HEAD)
239+
git_oid_cpy(&file->cache_data.oid, git_tree_id(tree));
237240
else if (source->type == GIT_ATTR_FILE_SOURCE_COMMIT)
238241
git_oid_cpy(&file->cache_data.oid, git_tree_id(tree));
239242
else if (source->type == GIT_ATTR_FILE_SOURCE_FILE)
@@ -288,22 +291,29 @@ int git_attr_file__out_of_date(
288291
return (git_oid__cmp(&file->cache_data.oid, &id) != 0);
289292
}
290293

291-
case GIT_ATTR_FILE_SOURCE_COMMIT: {
294+
case GIT_ATTR_FILE_SOURCE_HEAD: {
292295
git_tree *tree = NULL;
293-
int error;
296+
int error = git_repository_head_tree(&tree, repo);
294297

295-
if (source->commit_id) {
296-
git_commit *commit = NULL;
298+
if (error < 0)
299+
return error;
297300

298-
if ((error = git_commit_lookup(&commit, repo, source->commit_id)) < 0)
299-
return error;
301+
error = (git_oid__cmp(&file->cache_data.oid, git_tree_id(tree)) != 0);
300302

301-
error = git_commit_tree(&tree, commit);
303+
git_tree_free(tree);
304+
return error;
305+
}
302306

303-
git_commit_free(commit);
304-
} else {
305-
error = git_repository_head_tree(&tree, repo);
306-
}
307+
case GIT_ATTR_FILE_SOURCE_COMMIT: {
308+
git_commit *commit = NULL;
309+
git_tree *tree = NULL;
310+
int error;
311+
312+
if ((error = git_commit_lookup(&commit, repo, source->commit_id)) < 0)
313+
return error;
314+
315+
error = git_commit_tree(&tree, commit);
316+
git_commit_free(commit);
307317

308318
if (error < 0)
309319
return error;

‎src/attr_file.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@ typedef enum {
4040
GIT_ATTR_FILE_SOURCE_MEMORY = 0,
4141
GIT_ATTR_FILE_SOURCE_FILE = 1,
4242
GIT_ATTR_FILE_SOURCE_INDEX = 2,
43-
GIT_ATTR_FILE_SOURCE_COMMIT = 3,
43+
GIT_ATTR_FILE_SOURCE_HEAD = 3,
44+
GIT_ATTR_FILE_SOURCE_COMMIT = 4,
4445

45-
GIT_ATTR_FILE_NUM_SOURCES = 4
46+
GIT_ATTR_FILE_NUM_SOURCES = 5
4647
} git_attr_file_source_t;
4748

4849
typedef struct {

‎src/blob.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,13 @@ int git_blob_filter(
449449

450450
if ((opts.flags & GIT_BLOB_FILTER_ATTRIBUTES_FROM_COMMIT) != 0) {
451451
filter_opts.flags |= GIT_FILTER_ATTRIBUTES_FROM_COMMIT;
452-
filter_opts.commit_id = opts.commit_id;
452+
453+
#ifndef GIT_DEPRECATE_HARD
454+
if (opts.commit_id)
455+
git_oid_cpy(&filter_opts.attr_commit_id, opts.commit_id);
456+
else
457+
#endif
458+
git_oid_cpy(&filter_opts.attr_commit_id, &opts.attr_commit_id);
453459
}
454460

455461
if (!(error = git_filter_list_load_ext(

‎src/filter.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,13 @@ static int filter_list_check_attributes(
446446

447447
if ((src->options.flags & GIT_FILTER_ATTRIBUTES_FROM_COMMIT) != 0) {
448448
attr_opts.flags |= GIT_ATTR_CHECK_INCLUDE_COMMIT;
449-
attr_opts.commit_id = src->options.commit_id;
449+
450+
#ifndef GIT_DEPRECATE_HARD
451+
if (src->options.commit_id)
452+
git_oid_cpy(&attr_opts.attr_commit_id, src->options.commit_id);
453+
else
454+
#endif
455+
git_oid_cpy(&attr_opts.attr_commit_id, &src->options.attr_commit_id);
450456
}
451457

452458
error = git_attr_get_many_with_session(

‎tests/filter/bare.c‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,10 @@ void test_filter_bare__from_specific_commit_one(void)
137137
git_blob_filter_options opts = GIT_BLOB_FILTER_OPTIONS_INIT;
138138
git_blob *blob;
139139
git_buf buf = { 0 };
140-
git_oid commit_id;
141-
142-
cl_git_pass(git_oid_fromstr(&commit_id, "b8986fec0f7bde90f78ac72706e782d82f24f2f0"));
143140

144141
opts.flags |= GIT_BLOB_FILTER_NO_SYSTEM_ATTRIBUTES;
145142
opts.flags |= GIT_BLOB_FILTER_ATTRIBUTES_FROM_COMMIT;
146-
opts.commit_id = &commit_id;
143+
cl_git_pass(git_oid_fromstr(&opts.attr_commit_id, "b8986fec0f7bde90f78ac72706e782d82f24f2f0"));
147144

148145
cl_git_pass(git_revparse_single(
149146
(git_object **)&blob, g_repo, "055c872")); /* ident */
@@ -165,13 +162,10 @@ void test_filter_bare__from_specific_commit_with_no_attributes_file(void)
165162
git_blob_filter_options opts = GIT_BLOB_FILTER_OPTIONS_INIT;
166163
git_blob *blob;
167164
git_buf buf = { 0 };
168-
git_oid commit_id;
169-
170-
cl_git_pass(git_oid_fromstr(&commit_id, "5afb6a14a864e30787857dd92af837e8cdd2cb1b"));
171165

172166
opts.flags |= GIT_BLOB_FILTER_NO_SYSTEM_ATTRIBUTES;
173167
opts.flags |= GIT_BLOB_FILTER_ATTRIBUTES_FROM_COMMIT;
174-
opts.commit_id = &commit_id;
168+
cl_git_pass(git_oid_fromstr(&opts.attr_commit_id, "5afb6a14a864e30787857dd92af837e8cdd2cb1b"));
175169

176170
cl_git_pass(git_revparse_single(
177171
(git_object **)&blob, g_repo, "799770d")); /* all-lf */

0 commit comments

Comments
 (0)