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>

diff --git a/builtin/am.c b/builtin/am.c
index 93beb661977d1eb32013f4fca0e33e578aa5f7d5..ec1c213cb3dd68b9461a128fbbccf1224476be03 100644 (file)
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1918,9 +1918,8 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
        opts.dst_index = &the_index;
        opts.update = 1;
        opts.merge = 1;
-       opts.reset = reset;
-       if (!reset)
-               opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
+       opts.reset = reset ? UNPACK_RESET_PROTECT_UNTRACKED : 0;
+       opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
        opts.fn = twoway_merge;
        init_tree_desc(&t[0], head->buffer, head->size);
        init_tree_desc(&t[1], remote->buffer, remote->size);

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5e7957dd068875335655813ca73e252a28a11dca..cbf73b8c9f65ae2fdd1d96265bd4972aeaa9bd68 100644 (file)
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -646,9 +646,10 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
        opts.head_idx = -1;
        opts.update = worktree;
        opts.skip_unmerged = !worktree;
-       opts.reset = 1;
+       opts.reset = o->force ? UNPACK_RESET_OVERWRITE_UNTRACKED :
+                               UNPACK_RESET_PROTECT_UNTRACKED;
+       opts.preserve_ignored = (!o->force && !o->overwrite_ignore);
        opts.merge = 1;
-       opts.preserve_ignored = 0;
        opts.fn = oneway_merge;
        opts.verbose_update = o->show_progress;
        opts.src_index = &the_index;

diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 443d206eca6f414dc63a2ffa19a8e81bc531503b..2109c4c9e5c1c747ea08c8e2152b610c5159ddbe 100644 (file)
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -166,6 +166,9 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
        if (1 < opts.merge + opts.reset + prefix_set)
                die("Which one? -m, --reset, or --prefix?");
 
+       if (opts.reset)
+               opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED;
+
        /*
         * NEEDSWORK
         *

diff --git a/builtin/reset.c b/builtin/reset.c
index 5df01cc42e000aa2e52392d92bf09c1f9ac4e8db..739359534947e4d0bb5578ee243e9c179fd1c36b 100644 (file)
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -71,9 +71,14 @@ static int reset_index(const char *ref, const struct object_id *oid, int reset_t
                break;
        case HARD:
                opts.update = 1;
-               /* fallthrough */
+               opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED;
+               break;
+       case MIXED:
+               opts.reset = UNPACK_RESET_PROTECT_UNTRACKED;
+               /* but opts.update=0, so working tree not updated */
+               break;
        default:
-               opts.reset = 1;
+               BUG("invalid reset_type passed to reset_index");
        }
 
        read_cache_unmerged();

diff --git a/builtin/stash.c b/builtin/stash.c
index d60cdaf32f5c65e8285501a34f59d976fd464890..0e3662a230c3abfa8cb4121f978040d37b8d761b 100644 (file)
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -256,9 +256,9 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)
        opts.src_index = &the_index;
        opts.dst_index = &the_index;
        opts.merge = 1;
-       opts.reset = reset;
+       opts.reset = reset ? UNPACK_RESET_PROTECT_UNTRACKED : 0;
        opts.update = update;
-       if (update && !reset)
+       if (update)
                opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
        opts.fn = oneway_merge;
 

diff --git a/reset.c b/reset.c
index f40a8ecf663542fff02b77328d9f261626fbf8b5..f214df3d96ca218a25c780b2dc79ca4ca76999de 100644 (file)
--- a/reset.c
+++ b/reset.c
@@ -59,7 +59,7 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action,
        unpack_tree_opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
        init_checkout_metadata(&unpack_tree_opts.meta, switch_to_branch, oid, NULL);
        if (!detach_head)
-               unpack_tree_opts.reset = 1;
+               unpack_tree_opts.reset = UNPACK_RESET_PROTECT_UNTRACKED;
 
        if (repo_read_index_unmerged(r) < 0) {
                ret = error(_("could not read index"));

diff --git a/t/t2500-untracked-overwriting.sh b/t/t2500-untracked-overwriting.sh
index 2412d121ea83d5d30b00ae1edc37345f06fdfc8b..18604360df8c770f7a1a14ce0b32db345f615b40 100755 (executable)
--- a/t/t2500-untracked-overwriting.sh
+++ b/t/t2500-untracked-overwriting.sh
@@ -92,7 +92,7 @@ test_setup_checkout_m () {
        )
 }
 
-test_expect_failure 'checkout -m does not nuke untracked file' '
+test_expect_success 'checkout -m does not nuke untracked file' '
        test_setup_checkout_m &&
        (
                cd checkout &&
@@ -138,7 +138,7 @@ test_setup_sequencing () {
        )
 }
 
-test_expect_failure 'git rebase --abort and untracked files' '
+test_expect_success 'git rebase --abort and untracked files' '
        test_setup_sequencing rebase_abort_and_untracked &&
        (
                cd sequencing_rebase_abort_and_untracked &&
@@ -155,7 +155,7 @@ test_expect_failure 'git rebase --abort and untracked files' '
        )
 '
 
-test_expect_failure 'git rebase fast forwarding and untracked files' '
+test_expect_success 'git rebase fast forwarding and untracked files' '
        test_setup_sequencing rebase_fast_forward_and_untracked &&
        (
                cd sequencing_rebase_fast_forward_and_untracked &&

diff --git a/unpack-trees.c b/unpack-trees.c
index 9ccb991084f2d7107c5e9101b9e1cfc249a77ec3..3fff5061569a37a6ffbab9d235a1d97e81c71ebb 100644 (file)
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1694,6 +1694,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
        int free_pattern_list = 0;
        struct dir_struct dir = DIR_INIT;
 
+       if (o->reset == UNPACK_RESET_INVALID)
+               BUG("o->reset had a value of 1; should be UNPACK_TREES_*_UNTRACKED");
+
        if (len > MAX_UNPACK_TREES)
                die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
        if (o->dir)
@@ -1708,6 +1711,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
                ensure_full_index(o->dst_index);
        }
 
+       if (o->reset == UNPACK_RESET_OVERWRITE_UNTRACKED &&
+           o->preserve_ignored)
+               BUG("UNPACK_RESET_OVERWRITE_UNTRACKED incompatible with preserved ignored files");
+
        if (!o->preserve_ignored) {
                o->dir = &dir;
                o->dir->flags |= DIR_SHOW_IGNORED;
@@ -2231,7 +2238,8 @@ static int verify_absent_1(const struct cache_entry *ce,
        int len;
        struct stat st;
 
-       if (o->index_only || o->reset || !o->update)
+       if (o->index_only || !o->update ||
+           o->reset == UNPACK_RESET_OVERWRITE_UNTRACKED)
                return 0;
 
        len = check_leading_path(ce->name, ce_namelen(ce), 0);

diff --git a/unpack-trees.h b/unpack-trees.h
index 61da25dafeed71883a8ca94ecec8b5d0b4ffcc7a..71ffb7eeb0c0d1df8539cdff4d6383116d4ff1b4 100644 (file)
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -45,9 +45,15 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
  */
 void clear_unpack_trees_porcelain(struct unpack_trees_options *opts);
 
+enum unpack_trees_reset_type {
+       UNPACK_RESET_NONE = 0,    /* traditional "false" value; still valid */
+       UNPACK_RESET_INVALID = 1, /* "true" no longer valid; use below values */
+       UNPACK_RESET_PROTECT_UNTRACKED,
+       UNPACK_RESET_OVERWRITE_UNTRACKED
+};
+
 struct unpack_trees_options {
-       unsigned int reset,
-                    merge,
+       unsigned int merge,
                     update,
                     preserve_ignored,
                     clone,
@@ -65,6 +71,7 @@ struct unpack_trees_options {
                     exiting_early,
                     show_all_errors,
                     dry_run;
+       enum unpack_trees_reset_type reset;
        const char *prefix;
        int cache_bottom;
        struct pathspec *pathspec;