add: modify add_files_to_cache() to avoid globals
The function add_files_to_cache() is used by all three of builtin/{add, checkout, commit}.c. That suggests this is common library code, and should be moved somewhere else, like read-cache.c. However, the function and its helpers made use of two global variables that made straight code movement difficult: * the_index * include_sparse The latter was perhaps more problematic since it was only accessible in builtin/add.c but was still affecting builtin/checkout.c and builtin/commit.c without this fact being very clear from the code. I'm not sure if the other two callers would want to add a `--sparse` flag similar to add.c to get non-default behavior, but exposing this dependence will help if we ever decide we do want to add such a flag. Modify add_files_to_cache() and its helpers to accept the necessary arguments instead of relying on globals. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren committed
May 16, 2023 at 06:33 UTC
50c37ee839f0c6842816dec0764d0592b5d048b2
4 files changed
+21
-10
builtin/add.c
+15
-7
@@ -37,6 +37,8 @@ static int include_sparse;
37
static const char *pathspec_from_file;
38
39
struct update_callback_data {
40
+ struct index_state *index;
41
+ int include_sparse;
42
int flags;
43
int add_errors;
44
};
@@ -100,7 +102,8 @@ static void update_callback(struct diff_queue_struct *q,
102
struct diff_filepair *p = q->queue[i];
103
const char *path = p->one->path;
104
103
- if (!include_sparse && !path_in_sparse_checkout(path, &the_index))
105
+ if (!data->include_sparse &&
106
+ !path_in_sparse_checkout(path, data->index))
107
continue;
108
109
switch (fix_unmerged_status(p, data)) {
@@ -108,7 +111,7 @@ static void update_callback(struct diff_queue_struct *q,
111
die(_("unexpected diff status %c"), p->status);
112
case DIFF_STATUS_MODIFIED:
113
case DIFF_STATUS_TYPE_CHANGED:
111
- if (add_file_to_index(&the_index, path, data->flags)) {
114
+ if (add_file_to_index(data->index, path, data->flags)) {
115
if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
116
die(_("updating files failed"));
117
data->add_errors++;
@@ -118,7 +121,7 @@ static void update_callback(struct diff_queue_struct *q,
121
if (data->flags & ADD_CACHE_IGNORE_REMOVAL)
122
break;
123
if (!(data->flags & ADD_CACHE_PRETEND))
121
- remove_file_from_index(&the_index, path);
124
+ remove_file_from_index(data->index, path);
125
if (data->flags & (ADD_CACHE_PRETEND|ADD_CACHE_VERBOSE))
126
printf(_("remove '%s'\n"), path);
127
break;
@@ -126,16 +129,19 @@ static void update_callback(struct diff_queue_struct *q,
129
}
130
}
131
129
-int add_files_to_cache(const char *prefix,
130
- const struct pathspec *pathspec, int flags)
132
+int add_files_to_cache(struct repository *repo, const char *prefix,
133
+ const struct pathspec *pathspec, int include_sparse,
134
+ int flags)
135
{
136
struct update_callback_data data;
137
struct rev_info rev;
138
139
memset(&data, 0, sizeof(data));
140
+ data.index = repo->index;
141
+ data.include_sparse = include_sparse;
142
data.flags = flags;
143
138
- repo_init_revisions(the_repository, &rev, prefix);
144
+ repo_init_revisions(repo, &rev, prefix);
145
setup_revisions(0, NULL, &rev, NULL);
146
if (pathspec)
147
copy_pathspec(&rev.prune_data, pathspec);
@@ -640,7 +646,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)
646
if (add_renormalize)
647
exit_status |= renormalize_tracked_files(&pathspec, flags);
648
else
643
- exit_status |= add_files_to_cache(prefix, &pathspec, flags);
649
+ exit_status |= add_files_to_cache(the_repository, prefix,
650
+ &pathspec, include_sparse,
651
+ flags);
652
653
if (add_new_files)
654
exit_status |= add_files(&dir, flags);
builtin/checkout.c
+1
-1
@@ -861,7 +861,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
861
* entries in the index.
862
*/
863
864
- add_files_to_cache(NULL, NULL, 0);
864
+ add_files_to_cache(the_repository, NULL, NULL, 0, 0);
865
init_merge_options(&o, the_repository);
866
o.verbosity = 0;
867
work = write_in_core_index_as_tree(the_repository);
builtin/commit.c
+2
-1
@@ -447,7 +447,8 @@ static const char *prepare_index(const char **argv, const char *prefix,
447
if (all || (also && pathspec.nr)) {
448
repo_hold_locked_index(the_repository, &index_lock,
449
LOCK_DIE_ON_ERROR);
450
- add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
450
+ add_files_to_cache(the_repository, also ? prefix : NULL,
451
+ &pathspec, 0, 0);
452
refresh_cache_or_die(refresh_flags);
453
cache_tree_update(&the_index, WRITE_TREE_SILENT);
454
if (write_locked_index(&the_index, &index_lock, 0))
cache.h
+3
-1
@@ -554,7 +554,9 @@ int cmp_cache_name_compare(const void *a_, const void *b_);
554
* return 0 if success, 1 - if addition of a file failed and
555
* ADD_FILES_IGNORE_ERRORS was specified in flags
556
*/
557
-int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags);
557
+int add_files_to_cache(struct repository *repo, const char *prefix,
558
+ const struct pathspec *pathspec, int include_sparse,
559
+ int flags);
560
561
/* diff.c */
562
extern int diff_auto_refresh_index;