Change unpack_trees' 'reset' flag into an enum
Traditionally, unpack_trees_options->reset was used to signal that it was okay to delete any untracked files in the way. This was used by `git read-tree --reset`, but then started appearing in other places as well. However, many of the other uses should not be deleting untracked files in the way. Change this value to an enum so that a value of 1 (i.e. "true") can be split into two: UNPACK_RESET_PROTECT_UNTRACKED, UNPACK_RESET_OVERWRITE_UNTRACKED In order to catch accidental misuses (i.e. where folks call it the way they traditionally used to), define the special enum value of UNPACK_RESET_INVALID = 1 which will trigger a BUG(). Modify existing callers so that read-tree --reset reset --hard checkout --force continue using the UNPACK_RESET_OVERWRITE_UNTRACKED logic, while other callers, including am checkout without --force stash (though currently dead code; reset always had a value of 0) numerous callers from rebase/sequencer to reset_head() will use the new UNPACK_RESET_PROTECT_UNTRACKED value. Also, note that it has been reported that 'git checkout <treeish> <pathspec>' currently also allows overwriting untracked files[1]. That case should also be fixed, but it does not use unpack_trees() and thus is outside the scope of the current changes. [1] https://lore.kernel.org/git/15dad590-087e-5a48-9238-5d2826950506@gmail.com/ Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren committed
Sep 27, 2021 at 16:33 UTC
480d3d6bf90a6ec5b8f02d672f1d4027a4889106
9 files changed
+39
-16
builtin/am.c
+2
-3
@@ -1918,9 +1918,8 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
1918
opts.dst_index = &the_index;
1919
opts.update = 1;
1920
opts.merge = 1;
1921
- opts.reset = reset;
1922
- if (!reset)
1923
- opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
1921
+ opts.reset = reset ? UNPACK_RESET_PROTECT_UNTRACKED : 0;
1922
+ opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
1923
opts.fn = twoway_merge;
1924
init_tree_desc(&t[0], head->buffer, head->size);
1925
init_tree_desc(&t[1], remote->buffer, remote->size);
builtin/checkout.c
+3
-2
@@ -646,9 +646,10 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
646
opts.head_idx = -1;
647
opts.update = worktree;
648
opts.skip_unmerged = !worktree;
649
- opts.reset = 1;
649
+ opts.reset = o->force ? UNPACK_RESET_OVERWRITE_UNTRACKED :
650
+ UNPACK_RESET_PROTECT_UNTRACKED;
651
+ opts.preserve_ignored = (!o->force && !o->overwrite_ignore);
652
opts.merge = 1;
651
- opts.preserve_ignored = 0;
653
opts.fn = oneway_merge;
654
opts.verbose_update = o->show_progress;
655
opts.src_index = &the_index;
builtin/read-tree.c
+3
@@ -166,6 +166,9 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
166
if (1 < opts.merge + opts.reset + prefix_set)
167
die("Which one? -m, --reset, or --prefix?");
168
169
+ if (opts.reset)
170
+ opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED;
171
+
172
/*
173
* NEEDSWORK
174
*
builtin/reset.c
+7
-2
@@ -71,9 +71,14 @@ static int reset_index(const char *ref, const struct object_id *oid, int reset_t
71
break;
72
case HARD:
73
opts.update = 1;
74
- /* fallthrough */
74
+ opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED;
75
+ break;
76
+ case MIXED:
77
+ opts.reset = UNPACK_RESET_PROTECT_UNTRACKED;
78
+ /* but opts.update=0, so working tree not updated */
79
+ break;
80
default:
76
- opts.reset = 1;
81
+ BUG("invalid reset_type passed to reset_index");
82
}
83
84
read_cache_unmerged();
builtin/stash.c
+2
-2
@@ -256,9 +256,9 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)
256
opts.src_index = &the_index;
257
opts.dst_index = &the_index;
258
opts.merge = 1;
259
- opts.reset = reset;
259
+ opts.reset = reset ? UNPACK_RESET_PROTECT_UNTRACKED : 0;
260
opts.update = update;
261
- if (update && !reset)
261
+ if (update)
262
opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
263
opts.fn = oneway_merge;
264
reset.c
+1
-1
@@ -59,7 +59,7 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action,
59
unpack_tree_opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
60
init_checkout_metadata(&unpack_tree_opts.meta, switch_to_branch, oid, NULL);
61
if (!detach_head)
62
- unpack_tree_opts.reset = 1;
62
+ unpack_tree_opts.reset = UNPACK_RESET_PROTECT_UNTRACKED;
63
64
if (repo_read_index_unmerged(r) < 0) {
65
ret = error(_("could not read index"));
t/t2500-untracked-overwriting.sh
+3
-3
@@ -92,7 +92,7 @@ test_setup_checkout_m () {
92
)
93
}
94
95
-test_expect_failure 'checkout -m does not nuke untracked file' '
95
+test_expect_success 'checkout -m does not nuke untracked file' '
96
test_setup_checkout_m &&
97
(
98
cd checkout &&
@@ -138,7 +138,7 @@ test_setup_sequencing () {
138
)
139
}
140
141
-test_expect_failure 'git rebase --abort and untracked files' '
141
+test_expect_success 'git rebase --abort and untracked files' '
142
test_setup_sequencing rebase_abort_and_untracked &&
143
(
144
cd sequencing_rebase_abort_and_untracked &&
@@ -155,7 +155,7 @@ test_expect_failure 'git rebase --abort and untracked files' '
155
)
156
'
157
158
-test_expect_failure 'git rebase fast forwarding and untracked files' '
158
+test_expect_success 'git rebase fast forwarding and untracked files' '
159
test_setup_sequencing rebase_fast_forward_and_untracked &&
160
(
161
cd sequencing_rebase_fast_forward_and_untracked &&
unpack-trees.c
+9
-1
@@ -1694,6 +1694,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
1694
int free_pattern_list = 0;
1695
struct dir_struct dir = DIR_INIT;
1696
1697
+ if (o->reset == UNPACK_RESET_INVALID)
1698
+ BUG("o->reset had a value of 1; should be UNPACK_TREES_*_UNTRACKED");
1699
+
1700
if (len > MAX_UNPACK_TREES)
1701
die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
1702
if (o->dir)
@@ -1708,6 +1711,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
1711
ensure_full_index(o->dst_index);
1712
}
1713
1714
+ if (o->reset == UNPACK_RESET_OVERWRITE_UNTRACKED &&
1715
+ o->preserve_ignored)
1716
+ BUG("UNPACK_RESET_OVERWRITE_UNTRACKED incompatible with preserved ignored files");
1717
+
1718
if (!o->preserve_ignored) {
1719
o->dir = &dir;
1720
o->dir->flags |= DIR_SHOW_IGNORED;
@@ -2231,7 +2238,8 @@ static int verify_absent_1(const struct cache_entry *ce,
2238
int len;
2239
struct stat st;
2240
2234
- if (o->index_only || o->reset || !o->update)
2241
+ if (o->index_only || !o->update ||
2242
+ o->reset == UNPACK_RESET_OVERWRITE_UNTRACKED)
2243
return 0;
2244
2245
len = check_leading_path(ce->name, ce_namelen(ce), 0);
unpack-trees.h
+9
-2
@@ -45,9 +45,15 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
45
*/
46
void clear_unpack_trees_porcelain(struct unpack_trees_options *opts);
47
48
+enum unpack_trees_reset_type {
49
+ UNPACK_RESET_NONE = 0, /* traditional "false" value; still valid */
50
+ UNPACK_RESET_INVALID = 1, /* "true" no longer valid; use below values */
51
+ UNPACK_RESET_PROTECT_UNTRACKED,
52
+ UNPACK_RESET_OVERWRITE_UNTRACKED
53
+};
54
+
55
struct unpack_trees_options {
49
- unsigned int reset,
50
- merge,
56
+ unsigned int merge,
57
update,
58
preserve_ignored,
59
clone,
@@ -65,6 +71,7 @@ struct unpack_trees_options {
71
exiting_early,
72
show_all_errors,
73
dry_run;
74
+ enum unpack_trees_reset_type reset;
75
const char *prefix;
76
int cache_bottom;
77
struct pathspec *pathspec;