Re: [PATCH 1/3] sparse-checkout: remove use of the_repository
From: Junio C Hamano <hidden>
Date: 2025-07-08 20:59:06
"Derrick Stolee via GitGitGadget" [off-list ref] writes:
From: Derrick Stolee <redacted> The logic for the 'git sparse-checkout' builtin uses the_repository all over the place, despite some use of a repository struct in different method parameters. Complete this removal of the_repository by using 'repo' when possible. In one place, there was already a local variable 'r' that was set to the_repository, so move that to a method parameter. We cannot remove the USE_THE_REPOSITORY_VARIABLE declaration as we are still using global constants for the state of the sparse-checkout. Signed-off-by: Derrick Stolee <redacted> --- builtin/sparse-checkout.c | 119 ++++++++++++++++++++------------------ 1 file changed, 63 insertions(+), 56 deletions(-)
OK. The damage is not too bad for a partial update ;-). As the file-scope static functions in builtin/sparse-checkout.c are not going to be called by anybody else, it does not really matter if they internally pass an extra parameter around or use the_repository since the end result is the same. But doing this may hopefully help those that may want to move some of these functions to a more library-ish part of the system outside builtin/ hierarchy.
quoted hunk
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 1bf01591b275..8b70d0c6a441 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c@@ -204,12 +204,12 @@ static void clean_tracked_sparse_directories(struct repository *r) ensure_full_index(r->index); } -static int update_working_directory(struct pattern_list *pl) +static int update_working_directory(struct repository *r, + struct pattern_list *pl) { enum update_sparsity_result result; struct unpack_trees_options o; struct lock_file lock_file = LOCK_INIT; - struct repository *r = the_repository; struct pattern_list *old_pl;
As this already used short-and-sweet 'r', we just follow suit to minimize the damage, which is fine.
quoted hunk
@@ -327,7 +327,8 @@ static void write_cone_to_file(FILE *fp, struct pattern_list *pl) string_list_clear(&sl, 0); } -static int write_patterns_and_update(struct pattern_list *pl) +static int write_patterns_and_update(struct repository *repo, + struct pattern_list *pl) { char *sparse_filename; FILE *fp;@@ -336,15 +337,15 @@ static int write_patterns_and_update(struct pattern_list *pl) sparse_filename = get_sparse_checkout_filename(); - if (safe_create_leading_directories(the_repository, sparse_filename)) + if (safe_create_leading_directories(repo, sparse_filename)) die(_("failed to create directory for sparse-checkout file")); hold_lock_file_for_update(&lk, sparse_filename, LOCK_DIE_ON_ERROR); - result = update_working_directory(pl); + result = update_working_directory(repo, pl); if (result) { rollback_lock_file(&lk); - update_working_directory(NULL); + update_working_directory(repo, NULL); goto out; }
But this introduces a new parameter. Both of two instances of repository struct used in the existing code in this function, other than references to struct repository *UNUSED, use "r", and with this patch, the name "repo" becomes more prevanent. We would probably want to rename "r" to "repo" for consistency in clean_tracked_sparse_repositories() and update_working_directory(), but that is better done later after the dust settles and the code around here becomes quiescent again, not as part of this topic as an extra churn. Looking good. Thanks. Will queue.