sparse-checkout: avoid using internal API of unpack-trees

struct unpack_trees_options has the following field and comment: struct pattern_list *pl; /* for internal use */ Despite the internal-use comment, commit e091228e17 ("sparse-checkout: update working directory in-process", 2019-11-21) starting setting this field from an external caller. At the time, the only way around that would have been to modify unpack_trees() to take an extra pattern_list argument, and there's a lot of callers of that function. However, when we split update_sparsity() off as a separate function, with sparse-checkout being the sole caller, the need to update other callers went away. Fix this API problem by adding a pattern_list argument to update_sparsity() and stop setting the internal o.pl field directly. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Feb 27, 2023 at 15:28 UTC 1147c56ff70d5d1152601a73c9e76b9856ceebf6
3 files changed +14 -10
builtin/sparse-checkout.c
+1 -2
@@ -219,14 +219,13 @@ static int update_working_directory(struct pattern_list *pl)
219 o.dst_index = r->index;
220 index_state_init(&o.result, r);
221 o.skip_sparse_checkout = 0;
222 - o.pl = pl;
222
223 setup_work_tree();
224
225 repo_hold_locked_index(r, &lock_file, LOCK_DIE_ON_ERROR);
226
227 setup_unpack_trees_porcelain(&o, "sparse-checkout");
229 - result = update_sparsity(&o);
228 + result = update_sparsity(&o, pl);
229 clear_unpack_trees_porcelain(&o);
230
231 if (result == UPDATE_SPARSITY_WARNINGS)
unpack-trees.c
+11 -7
@@ -2091,10 +2091,10 @@ return_failed:
2091 *
2092 * CE_NEW_SKIP_WORKTREE is used internally.
2093 */
2094 -enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
2094 +enum update_sparsity_result update_sparsity(struct unpack_trees_options *o,
2095 + struct pattern_list *pl)
2096 {
2097 enum update_sparsity_result ret = UPDATE_SPARSITY_SUCCESS;
2097 - struct pattern_list pl;
2098 int i;
2099 unsigned old_show_all_errors;
2100 int free_pattern_list = 0;
@@ -2111,11 +2111,12 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
2111 trace_performance_enter();
2112
2113 /* If we weren't given patterns, use the recorded ones */
2114 - if (!o->pl) {
2115 - memset(&pl, 0, sizeof(pl));
2114 + if (!pl) {
2115 free_pattern_list = 1;
2117 - populate_from_existing_patterns(o, &pl);
2116 + pl = xcalloc(1, sizeof(*pl));
2117 + populate_from_existing_patterns(o, pl);
2118 }
2119 + o->pl = pl;
2120
2121 /* Expand sparse directories as needed */
2122 expand_index(o->src_index, o->pl);
@@ -2147,8 +2148,11 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o)
2148
2149 display_warning_msgs(o);
2150 o->show_all_errors = old_show_all_errors;
2150 - if (free_pattern_list)
2151 - clear_pattern_list(&pl);
2151 + if (free_pattern_list) {
2152 + clear_pattern_list(pl);
2153 + free(pl);
2154 + o->pl = NULL;
2155 + }
2156 trace_performance_leave("update_sparsity");
2157 return ret;
2158 }
unpack-trees.h
+2 -1
@@ -112,7 +112,8 @@ enum update_sparsity_result {
112 UPDATE_SPARSITY_WORKTREE_UPDATE_FAILURES = -2
113 };
114
115 -enum update_sparsity_result update_sparsity(struct unpack_trees_options *options);
115 +enum update_sparsity_result update_sparsity(struct unpack_trees_options *options,
116 + struct pattern_list *pl);
117
118 int verify_uptodate(const struct cache_entry *ce,
119 struct unpack_trees_options *o);