@matheusbsbs / matheus-bagliano / commits / e091228e17
Fork 0

sparse-checkout: update working directory in-process

The sparse-checkout builtin used 'git read-tree -mu HEAD' to update the skip-worktree bits in the index and to update the working directory. This extra process is overly complex, and prone to failure. It also requires that we write our changes to the sparse-checkout file before trying to update the index. Remove this extra process call by creating a direct call to unpack_trees() in the same way 'git read-tree -mu HEAD' does. In addition, provide an in-memory list of patterns so we can avoid reading from the sparse-checkout file. This allows us to test a proposed change to the file before writing to it. An earlier version of this patch included a bug when the 'set' command failed due to the "Sparse checkout leaves no entry on working directory" error. It would not rollback the index.lock file, so the replay of the old sparse-checkout specification would fail. A test in t1091 now covers that scenario. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Nov 21, 2019 at 22:04 UTC e091228e17e88b1bc16cb50d5c3aff10dc5119d1
5 files changed +105 -16
builtin/read-tree.c
+1 -1
@@ -185,7 +185,7 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
185
186 if (opts.reset || opts.merge || opts.prefix) {
187 if (read_cache_unmerged() && (opts.prefix || opts.merge))
188 - die("You need to resolve your current index first");
188 + die(_("You need to resolve your current index first"));
189 stage = opts.merge = 1;
190 }
191 resolve_undo_clear();
builtin/sparse-checkout.c
+71 -12
@@ -7,6 +7,11 @@
7 #include "run-command.h"
8 #include "strbuf.h"
9 #include "string-list.h"
10 +#include "cache.h"
11 +#include "cache-tree.h"
12 +#include "lockfile.h"
13 +#include "resolve-undo.h"
14 +#include "unpack-trees.h"
15
16 static char const * const builtin_sparse_checkout_usage[] = {
17 N_("git sparse-checkout (init|list|set|disable) <options>"),
@@ -60,18 +65,54 @@ static int sparse_checkout_list(int argc, const char **argv)
65 return 0;
66 }
67
63 -static int update_working_directory(void)
68 +static int update_working_directory(struct pattern_list *pl)
69 {
65 - struct argv_array argv = ARGV_ARRAY_INIT;
70 int result = 0;
67 - argv_array_pushl(&argv, "read-tree", "-m", "-u", "HEAD", NULL);
71 + struct unpack_trees_options o;
72 + struct lock_file lock_file = LOCK_INIT;
73 + struct object_id oid;
74 + struct tree *tree;
75 + struct tree_desc t;
76 + struct repository *r = the_repository;
77
69 - if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
70 - error(_("failed to update index with new sparse-checkout patterns"));
71 - result = 1;
72 - }
78 + if (repo_read_index_unmerged(r))
79 + die(_("you need to resolve your current index first"));
80 +
81 + if (get_oid("HEAD", &oid))
82 + return 0;
83 +
84 + tree = parse_tree_indirect(&oid);
85 + parse_tree(tree);
86 + init_tree_desc(&t, tree->buffer, tree->size);
87 +
88 + memset(&o, 0, sizeof(o));
89 + o.verbose_update = isatty(2);
90 + o.merge = 1;
91 + o.update = 1;
92 + o.fn = oneway_merge;
93 + o.head_idx = -1;
94 + o.src_index = r->index;
95 + o.dst_index = r->index;
96 + o.skip_sparse_checkout = 0;
97 + o.pl = pl;
98 + o.keep_pattern_list = !!pl;
99 +
100 + resolve_undo_clear_index(r->index);
101 + setup_work_tree();
102 +
103 + cache_tree_free(&r->index->cache_tree);
104 +
105 + repo_hold_locked_index(r, &lock_file, LOCK_DIE_ON_ERROR);
106 +
107 + core_apply_sparse_checkout = 1;
108 + result = unpack_trees(1, &t, &o);
109 +
110 + if (!result) {
111 + prime_cache_tree(r, r->index, tree);
112 + write_locked_index(r->index, &lock_file, COMMIT_LOCK);
113 + } else
114 + rollback_lock_file(&lock_file);
115
74 - argv_array_clear(&argv);
116 return result;
117 }
118
@@ -129,6 +170,15 @@ static int write_patterns_and_update(struct pattern_list *pl)
170 {
171 char *sparse_filename;
172 FILE *fp;
173 + int result;
174 +
175 + result = update_working_directory(pl);
176 +
177 + if (result) {
178 + clear_pattern_list(pl);
179 + update_working_directory(NULL);
180 + return result;
181 + }
182
183 sparse_filename = get_sparse_checkout_filename();
184 fp = fopen(sparse_filename, "w");
@@ -139,9 +189,11 @@ static int write_patterns_and_update(struct pattern_list *pl)
189 write_patterns_to_file(fp, pl);
190
191 fclose(fp);
192 +
193 free(sparse_filename);
194 + clear_pattern_list(pl);
195
144 - return update_working_directory();
196 + return 0;
197 }
198
199 enum sparse_checkout_mode {
@@ -199,7 +251,11 @@ static int sparse_checkout_init(int argc, const char **argv)
251 builtin_sparse_checkout_init_options,
252 builtin_sparse_checkout_init_usage, 0);
253
202 - mode = init_opts.cone_mode ? MODE_CONE_PATTERNS : MODE_ALL_PATTERNS;
254 + if (init_opts.cone_mode) {
255 + mode = MODE_CONE_PATTERNS;
256 + core_sparse_checkout_cone = 1;
257 + } else
258 + mode = MODE_ALL_PATTERNS;
259
260 if (set_config(mode))
261 return 1;
@@ -230,7 +286,8 @@ static int sparse_checkout_init(int argc, const char **argv)
286 }
287
288 reset_dir:
233 - return update_working_directory();
289 + core_apply_sparse_checkout = 1;
290 + return update_working_directory(NULL);
291 }
292
293 static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *path)
@@ -311,6 +368,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
368
369 hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
370 hashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);
371 + pl.use_cone_patterns = 1;
372
373 if (set_opts.use_stdin) {
374 while (!strbuf_getline(&line, stdin))
@@ -365,7 +423,8 @@ static int sparse_checkout_disable(int argc, const char **argv)
423 fprintf(fp, "/*\n");
424 fclose(fp);
425
368 - if (update_working_directory())
426 + core_apply_sparse_checkout = 1;
427 + if (update_working_directory(NULL))
428 die(_("error while refreshing working directory"));
429
430 unlink(sparse_filename);
t/t1091-sparse-checkout-builtin.sh
+28
@@ -248,4 +248,32 @@ test_expect_success 'cone mode: set with nested folders' '
248 test_cmp repo/.git/info/sparse-checkout expect
249 '
250
251 +test_expect_success 'revert to old sparse-checkout on bad update' '
252 + echo update >repo/deep/deeper2/a &&
253 + cp repo/.git/info/sparse-checkout expect &&
254 + test_must_fail git -C repo sparse-checkout set deep/deeper1 2>err &&
255 + test_i18ngrep "Cannot update sparse checkout" err &&
256 + test_cmp repo/.git/info/sparse-checkout expect &&
257 + ls repo/deep >dir &&
258 + cat >expect <<-EOF &&
259 + a
260 + deeper1
261 + deeper2
262 + EOF
263 + test_cmp dir expect
264 +'
265 +
266 +test_expect_success 'revert to old sparse-checkout on empty update' '
267 + git init empty-test &&
268 + (
269 + echo >file &&
270 + git add file &&
271 + git commit -m "test" &&
272 + test_must_fail git sparse-checkout set nothing 2>err &&
273 + test_i18ngrep "Sparse checkout leaves no entry on working directory" err &&
274 + test_i18ngrep ! ".git/index.lock" err &&
275 + git sparse-checkout set file
276 + )
277 +'
278 +
279 test_done
unpack-trees.c
+3 -2
@@ -1511,7 +1511,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
1511 memset(&pl, 0, sizeof(pl));
1512 if (!core_apply_sparse_checkout || !o->update)
1513 o->skip_sparse_checkout = 1;
1514 - if (!o->skip_sparse_checkout) {
1514 + if (!o->skip_sparse_checkout && !o->pl) {
1515 char *sparse = git_pathdup("info/sparse-checkout");
1516 pl.use_cone_patterns = core_sparse_checkout_cone;
1517 if (add_patterns_from_file_to_list(sparse, "", 0, &pl, NULL) < 0)
@@ -1684,7 +1684,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
1684
1685 done:
1686 trace_performance_leave("unpack_trees");
1687 - clear_pattern_list(&pl);
1687 + if (!o->keep_pattern_list)
1688 + clear_pattern_list(&pl);
1689 return ret;
1690
1691 return_failed:
unpack-trees.h
+2 -1
@@ -59,7 +59,8 @@ struct unpack_trees_options {
59 quiet,
60 exiting_early,
61 show_all_errors,
62 - dry_run;
62 + dry_run,
63 + keep_pattern_list;
64 const char *prefix;
65 int cache_bottom;
66 struct dir_struct *dir;