| Message ID | 20190501101403.20294-2-phillip.wood123@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | read-tree: improve untracked file support | expand |
On Wed, May 1, 2019 at 5:14 PM Phillip Wood <phillip.wood123@gmail.com> wrote: > diff --git a/unpack-trees.h b/unpack-trees.h > index d344d7d296..732b262c4d 100644 > --- a/unpack-trees.h > +++ b/unpack-trees.h > @@ -41,9 +41,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_NO_RESET = 0, > + UNPACK_RESET_OVERWRITE_UNTRACKED, > + UNPACK_RESET_PROTECT_UNTRACKED > +}; > + > struct unpack_trees_options { > - unsigned int reset, > - merge, > + enum unpack_trees_reset_type reset; Can we add protected_untracked that can be used in combination with the current "reset"? This opens up (or raises the question about) the opportunity to protect untracked changes on non-reset updates. > + unsigned int merge, > update, > clone, > index_only, > -- > 2.21.0 >
On Wed, May 1, 2019 at 5:18 PM Duy Nguyen <pclouds@gmail.com> wrote: > > On Wed, May 1, 2019 at 5:14 PM Phillip Wood <phillip.wood123@gmail.com> wrote: > > diff --git a/unpack-trees.h b/unpack-trees.h > > index d344d7d296..732b262c4d 100644 > > --- a/unpack-trees.h > > +++ b/unpack-trees.h > > @@ -41,9 +41,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_NO_RESET = 0, > > + UNPACK_RESET_OVERWRITE_UNTRACKED, > > + UNPACK_RESET_PROTECT_UNTRACKED > > +}; > > + > > struct unpack_trees_options { > > - unsigned int reset, > > - merge, > > + enum unpack_trees_reset_type reset; > > Can we add protected_untracked that can be used in combination with > the current "reset"? > > This opens up (or raises the question about) the opportunity to > protect untracked changes on non-reset updates. Bahh. "protect_untracked" is already on for non-reset updates. Sorry for the noise.
On Wed, May 1, 2019 at 5:14 PM Phillip Wood <phillip.wood123@gmail.com> wrote: > > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > Currently there is no way to get git to discard changes to the > worktree without overwriting untracked files. `reset --hard`, > `checkout --force`, `checkout <rev> :/` and `read-tree --reset -u` "checkout <rev> :/" does not use unpack-trees, I think, so it does not belong to this group. > will all overwrite untracked files. unpack_trees() has known how to > avoid overwriting untracked files since commit fcc387db9b ("read-tree > -m -u: do not overwrite or remove untracked working tree files.", > 2006-05-17) in response to concerns about lost files [1] but those > protections do not apply to --reset. This patch adds an > --protect-untracked option for read-tree/unpack_trees() to apply th > same protections with --reset. Your enum makes me wonder if we should have --reset-tracked instead of --protect-untracked (say what you want to reset seems tiny bit easier to understand than "reset except untracked"). Which leads to another question, do we ever need --reset-untracked (i.e. overwriting untracked files but never ever touch tracked ones). I have one use case that --reset-untracked might make sense. Suppose you do "git reset HEAD^" where HEAD has some new files. The new files will be left alone in worktree of course, but switching back to HEAD after that, unpack-trees will cry murder because it would otherwise overwrite untracked files (that have the exact same content). --reset-untracked might be useful for that. I'm carrying a patch to detect identical file content though which addresses this very specific case. Maybe it's the only case that --reset-untracked may be useful. > It does not change the behavior of any > of the porcelain commands but paves the way for adding options or > changing the default for those in future. > > Note the actual change in unpack-trees.c is quite simple, the majority > of this patch is converting existing callers to use the new > unpack_trees_reset_type enum. This could be split in a separate patch, then the actual change would become small and more to the point. > [1] https://public-inbox.org/git/8aa486160605161500m1dd8428cj@mail.gmail.com/ > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > > Notes: > adding --protect-untracked to the invocation of > test_submodule_forced_switch() in t1013 fixes the known breakage of > tests 57 & 58 but breaks test 64 which relies on forcing. I'm not sure > that the expected results of 57 & 58 are correct if we're forcing. > > Documentation/git-read-tree.txt | 10 +++++++-- > builtin/am.c | 8 ++++--- > builtin/checkout.c | 2 +- > builtin/read-tree.c | 12 +++++++++++ > builtin/rebase.c | 2 +- > builtin/reset.c | 2 +- > builtin/stash.c | 7 ++++--- > t/lib-read-tree.sh | 11 ++++++++++ > t/t1005-read-tree-reset.sh | 37 +++++++++++++++++++++++++++++++-- > unpack-trees.c | 3 ++- > unpack-trees.h | 10 +++++++-- > 11 files changed, 88 insertions(+), 16 deletions(-) > > diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt > index d271842608..67864c6bbc 100644 > --- a/Documentation/git-read-tree.txt > +++ b/Documentation/git-read-tree.txt > @@ -40,12 +40,18 @@ OPTIONS > --reset:: > Same as -m, except that unmerged entries are discarded instead > of failing. When used with `-u`, updates leading to loss of > - working tree changes will not abort the operation. > + working tree changes will not abort the operation and > + untracked files will be overwritten. Use `--protect-untracked` > + to avoid overwriting untracked files. > > -u:: > After a successful merge, update the files in the work > tree with the result of the merge. > > +--protect-untracked:: > + Prevent `--reset` `-u` from overwriting untracked files. Requires > + `--reset` and `-u` (`-m` never overwrites untracked files). > + > -i:: > Usually a merge requires the index file as well as the > files in the working tree to be up to date with the > @@ -89,7 +95,7 @@ OPTIONS > existed in the original index file. > > --exclude-per-directory=<gitignore>:: > - When running the command with `-u` and `-m` options, the > + When using `-u` with `-m` or `--reset` `--protect-untracked` options, the > merge result may need to overwrite paths that are not > tracked in the current branch. The command usually > refuses to proceed with the merge to avoid losing such a > diff --git a/builtin/am.c b/builtin/am.c > index 912d9821b1..a92394b682 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -1854,7 +1854,8 @@ static void am_resolve(struct am_state *state) > * true, any unmerged entries will be discarded. Returns 0 on success, -1 on > * failure. > */ > -static int fast_forward_to(struct tree *head, struct tree *remote, int reset) > +static int fast_forward_to(struct tree *head, struct tree *remote, > + enum unpack_trees_reset_type reset) > { > struct lock_file lock_file = LOCK_INIT; > struct unpack_trees_options opts; > @@ -1942,7 +1943,8 @@ static int clean_index(const struct object_id *head, const struct object_id *rem > > read_cache_unmerged(); > > - if (fast_forward_to(head_tree, head_tree, 1)) > + if (fast_forward_to(head_tree, head_tree, > + UNPACK_RESET_OVERWRITE_UNTRACKED)) > return -1; > > if (write_cache_as_tree(&index, 0, NULL)) > @@ -1952,7 +1954,7 @@ static int clean_index(const struct object_id *head, const struct object_id *rem > if (!index_tree) > return error(_("Could not parse object '%s'."), oid_to_hex(&index)); > > - if (fast_forward_to(index_tree, remote_tree, 0)) > + if (fast_forward_to(index_tree, remote_tree, UNPACK_NO_RESET)) > return -1; > > if (merge_tree(remote_tree)) > diff --git a/builtin/checkout.c b/builtin/checkout.c > index ffa776c6e1..e9e70018f9 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -506,7 +506,7 @@ 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 = UNPACK_RESET_OVERWRITE_UNTRACKED; > opts.merge = 1; > opts.fn = oneway_merge; > opts.verbose_update = o->show_progress; > diff --git a/builtin/read-tree.c b/builtin/read-tree.c > index 5c9c082595..23735adde9 100644 > --- a/builtin/read-tree.c > +++ b/builtin/read-tree.c > @@ -114,6 +114,7 @@ static int git_read_tree_config(const char *var, const char *value, void *cb) > int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) > { > int i, stage = 0; > + int protect_untracked = -1; > struct object_id oid; > struct tree_desc t[MAX_UNPACK_TREES]; > struct unpack_trees_options opts; > @@ -140,6 +141,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) > PARSE_OPT_NONEG }, > OPT_BOOL('u', NULL, &opts.update, > N_("update working tree with merge result")), > + OPT_BOOL(0, "protect-untracked", &protect_untracked, > + N_("do not overwrite untracked files")), > { OPTION_CALLBACK, 0, "exclude-per-directory", &opts, > N_("gitignore"), > N_("allow explicitly ignored files to be overwritten"), > @@ -209,8 +212,17 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) > if ((opts.update || opts.index_only) && !opts.merge) > die("%s is meaningless without -m, --reset, or --prefix", > opts.update ? "-u" : "-i"); > + if (protect_untracked >= 0) { > + if (!opts.reset || !opts.update) > + die("-%s-protect-untracked requires --reset and -u", > + protect_untracked ? "" : "-no"); > + opts.reset = UNPACK_RESET_PROTECT_UNTRACKED; > + } > if ((opts.dir && !opts.update)) > die("--exclude-per-directory is meaningless unless -u"); > + if (opts.dir && opts.reset == UNPACK_RESET_OVERWRITE_UNTRACKED) > + warning("--exclude-per-directory without --preserve-untracked " > + "has no effect"); > if (opts.merge && !opts.index_only) > setup_work_tree(); > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 2e41ad5644..feb30a71f5 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -398,7 +398,7 @@ static int reset_head(struct object_id *oid, const char *action, > unpack_tree_opts.update = 1; > unpack_tree_opts.merge = 1; > if (!detach_head) > - unpack_tree_opts.reset = 1; > + unpack_tree_opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED; > > if (repo_read_index_unmerged(the_repository) < 0) { > ret = error(_("could not read index")); > diff --git a/builtin/reset.c b/builtin/reset.c > index 26ef9a7bd0..a39dd92fb2 100644 > --- a/builtin/reset.c > +++ b/builtin/reset.c > @@ -70,7 +70,7 @@ static int reset_index(const struct object_id *oid, int reset_type, int quiet) > opts.update = 1; > /* fallthrough */ > default: > - opts.reset = 1; > + opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED; > } > > read_cache_unmerged(); > diff --git a/builtin/stash.c b/builtin/stash.c > index 2a8e6d09b4..175d1b62d3 100644 > --- a/builtin/stash.c > +++ b/builtin/stash.c > @@ -227,7 +227,8 @@ static int clear_stash(int argc, const char **argv, const char *prefix) > return do_clear_stash(); > } > > -static int reset_tree(struct object_id *i_tree, int update, int reset) > +static int reset_tree(struct object_id *i_tree, int update, > + enum unpack_trees_reset_type reset) > { > int nr_trees = 1; > struct unpack_trees_options opts; > @@ -461,7 +462,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, > } > > if (has_index) { > - if (reset_tree(&index_tree, 0, 0)) > + if (reset_tree(&index_tree, 0, UNPACK_NO_RESET)) > return -1; > } else { > struct strbuf out = STRBUF_INIT; > @@ -471,7 +472,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, > return -1; > } > > - if (reset_tree(&c_tree, 0, 1)) { > + if (reset_tree(&c_tree, 0, UNPACK_RESET_OVERWRITE_UNTRACKED)) { > strbuf_release(&out); > return -1; > } > diff --git a/t/lib-read-tree.sh b/t/lib-read-tree.sh > index b95f485606..22f516d91f 100644 > --- a/t/lib-read-tree.sh > +++ b/t/lib-read-tree.sh > @@ -39,3 +39,14 @@ read_tree_u_must_fail () { > test_cmp pre-dry-run-wt post-dry-run-wt && > test_must_fail git read-tree "$@" > } > + > +read_tree_u_must_fail_save_err () { > + git ls-files -s >pre-dry-run && > + git diff-files -p >pre-dry-run-wt && > + test_must_fail git read-tree -n "$@" && > + git ls-files -s >post-dry-run && > + git diff-files -p >post-dry-run-wt && > + test_cmp pre-dry-run post-dry-run && > + test_cmp pre-dry-run-wt post-dry-run-wt && > + test_must_fail git read-tree "$@" 2>actual-err > +} > diff --git a/t/t1005-read-tree-reset.sh b/t/t1005-read-tree-reset.sh > index 83b09e1310..6c9dd6805b 100755 > --- a/t/t1005-read-tree-reset.sh > +++ b/t/t1005-read-tree-reset.sh > @@ -19,15 +19,48 @@ test_expect_success 'setup' ' > git add df && > echo content >new && > git add new && > - git commit -m two > + git commit -m two && > + git ls-files >expect-two > ' > > -test_expect_success 'reset should work' ' > +test_expect_success '--protect-untracked option sanity checks' ' > + read_tree_u_must_fail --reset --protect-untracked HEAD && > + read_tree_u_must_fail --reset --no-protect-untracked HEAD && > + read_tree_u_must_fail -m -u --protect-untracked HEAD && > + read_tree_u_must_fail -m -u --no-protect-untracked > +' > + > +test_expect_success 'reset should reset worktree' ' > + echo changed >df && > read_tree_u_must_succeed -u --reset HEAD^ && > git ls-files >actual && > test_cmp expect actual > ' > > +test_expect_success 'reset --protect-untracked protects untracked file' ' > + echo changed >new && > + read_tree_u_must_fail_save_err -u --reset --protect-untracked HEAD && > + echo "error: Untracked working tree file '\'new\'' would be overwritten by merge." >expected-err && > + test_cmp expected-err actual-err > +' > + > +test_expect_success 'reset --protect-untracked protects untracked directory' ' > + rm new && > + mkdir new && > + echo untracked >new/untracked && > + read_tree_u_must_fail_save_err -u --reset --protect-untracked HEAD && > + echo "error: Updating '\'new\'' would lose untracked files in it" >expected-err && > + test_cmp expected-err actual-err > +' > + > +test_expect_success 'reset --protect-untracked resets' ' > + rm -rf new && > + echo changed >df/file && > + read_tree_u_must_succeed -u --reset --protect-untracked HEAD && > + git ls-files >actual-two && > + test_cmp expect-two actual-two > +' > + > test_expect_success 'reset should remove remnants from a failed merge' ' > read_tree_u_must_succeed --reset -u HEAD && > git ls-files -s >expect && > diff --git a/unpack-trees.c b/unpack-trees.c > index 50189909b8..b1722730fe 100644 > --- a/unpack-trees.c > +++ b/unpack-trees.c > @@ -1917,7 +1917,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->reset == UNPACK_RESET_OVERWRITE_UNTRACKED || > + !o->update) > return 0; > > len = check_leading_path(ce->name, ce_namelen(ce)); > diff --git a/unpack-trees.h b/unpack-trees.h > index d344d7d296..732b262c4d 100644 > --- a/unpack-trees.h > +++ b/unpack-trees.h > @@ -41,9 +41,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_NO_RESET = 0, > + UNPACK_RESET_OVERWRITE_UNTRACKED, > + UNPACK_RESET_PROTECT_UNTRACKED > +}; > + > struct unpack_trees_options { > - unsigned int reset, > - merge, > + enum unpack_trees_reset_type reset; > + unsigned int merge, > update, > clone, > index_only, > -- > 2.21.0 >
On 02/05/2019 11:38, Duy Nguyen wrote: > On Wed, May 1, 2019 at 5:14 PM Phillip Wood <phillip.wood123@gmail.com> wrote: >> >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> Currently there is no way to get git to discard changes to the >> worktree without overwriting untracked files. `reset --hard`, >> `checkout --force`, `checkout <rev> :/` and `read-tree --reset -u` > > "checkout <rev> :/" does not use unpack-trees, I think, so it does not > belong to this group. You're right that it does not use unpack-trees, but I'm making the point that there is no way to discard changes to tracked files without overwriting untracked files whatever one uses. >> will all overwrite untracked files. unpack_trees() has known how to >> avoid overwriting untracked files since commit fcc387db9b ("read-tree >> -m -u: do not overwrite or remove untracked working tree files.", >> 2006-05-17) in response to concerns about lost files [1] but those >> protections do not apply to --reset. This patch adds an >> --protect-untracked option for read-tree/unpack_trees() to apply th >> same protections with --reset. > > Your enum makes me wonder if we should have --reset-tracked instead of > --protect-untracked (say what you want to reset seems tiny bit easier > to understand than "reset except untracked"). Which leads to another > question, do we ever need --reset-untracked (i.e. overwriting > untracked files but never ever touch tracked ones). > > I have one use case that --reset-untracked might make sense. Suppose > you do "git reset HEAD^" where HEAD has some new files. The new files > will be left alone in worktree of course, but switching back to HEAD > after that, unpack-trees will cry murder because it would otherwise > overwrite untracked files (that have the exact same content). > > --reset-untracked might be useful for that. I'm carrying a patch to > detect identical file content though which addresses this very > specific case. Maybe it's the only case that --reset-untracked may be > useful. That patch sounds useful (I played around with "hg update" the other day and I think it does the same). I think that's probably a common case, it also works if one does "git checkout <other-branch> file" then "git reset HEAD" followed "git checkout <other-branch>". --reset-untracked is just like --reset which we can't get rid of without breaking backwards compatibility. Having --reset-tracked is maybe better than having a --protect-tracked option that only works with --reset -u. >> It does not change the behavior of any >> of the porcelain commands but paves the way for adding options or >> changing the default for those in future. >> >> Note the actual change in unpack-trees.c is quite simple, the majority >> of this patch is converting existing callers to use the new >> unpack_trees_reset_type enum. > > This could be split in a separate patch, then the actual change would > become small and more to the point. Yes that would make it clearer. Best Wishes Phillip > >> [1] https://public-inbox.org/git/8aa486160605161500m1dd8428cj@mail.gmail.com/ >> >> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >> --- >> >> Notes: >> adding --protect-untracked to the invocation of >> test_submodule_forced_switch() in t1013 fixes the known breakage of >> tests 57 & 58 but breaks test 64 which relies on forcing. I'm not sure >> that the expected results of 57 & 58 are correct if we're forcing. >> >> Documentation/git-read-tree.txt | 10 +++++++-- >> builtin/am.c | 8 ++++--- >> builtin/checkout.c | 2 +- >> builtin/read-tree.c | 12 +++++++++++ >> builtin/rebase.c | 2 +- >> builtin/reset.c | 2 +- >> builtin/stash.c | 7 ++++--- >> t/lib-read-tree.sh | 11 ++++++++++ >> t/t1005-read-tree-reset.sh | 37 +++++++++++++++++++++++++++++++-- >> unpack-trees.c | 3 ++- >> unpack-trees.h | 10 +++++++-- >> 11 files changed, 88 insertions(+), 16 deletions(-) >> >> diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt >> index d271842608..67864c6bbc 100644 >> --- a/Documentation/git-read-tree.txt >> +++ b/Documentation/git-read-tree.txt >> @@ -40,12 +40,18 @@ OPTIONS >> --reset:: >> Same as -m, except that unmerged entries are discarded instead >> of failing. When used with `-u`, updates leading to loss of >> - working tree changes will not abort the operation. >> + working tree changes will not abort the operation and >> + untracked files will be overwritten. Use `--protect-untracked` >> + to avoid overwriting untracked files. >> >> -u:: >> After a successful merge, update the files in the work >> tree with the result of the merge. >> >> +--protect-untracked:: >> + Prevent `--reset` `-u` from overwriting untracked files. Requires >> + `--reset` and `-u` (`-m` never overwrites untracked files). >> + >> -i:: >> Usually a merge requires the index file as well as the >> files in the working tree to be up to date with the >> @@ -89,7 +95,7 @@ OPTIONS >> existed in the original index file. >> >> --exclude-per-directory=<gitignore>:: >> - When running the command with `-u` and `-m` options, the >> + When using `-u` with `-m` or `--reset` `--protect-untracked` options, the >> merge result may need to overwrite paths that are not >> tracked in the current branch. The command usually >> refuses to proceed with the merge to avoid losing such a >> diff --git a/builtin/am.c b/builtin/am.c >> index 912d9821b1..a92394b682 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -1854,7 +1854,8 @@ static void am_resolve(struct am_state *state) >> * true, any unmerged entries will be discarded. Returns 0 on success, -1 on >> * failure. >> */ >> -static int fast_forward_to(struct tree *head, struct tree *remote, int reset) >> +static int fast_forward_to(struct tree *head, struct tree *remote, >> + enum unpack_trees_reset_type reset) >> { >> struct lock_file lock_file = LOCK_INIT; >> struct unpack_trees_options opts; >> @@ -1942,7 +1943,8 @@ static int clean_index(const struct object_id *head, const struct object_id *rem >> >> read_cache_unmerged(); >> >> - if (fast_forward_to(head_tree, head_tree, 1)) >> + if (fast_forward_to(head_tree, head_tree, >> + UNPACK_RESET_OVERWRITE_UNTRACKED)) >> return -1; >> >> if (write_cache_as_tree(&index, 0, NULL)) >> @@ -1952,7 +1954,7 @@ static int clean_index(const struct object_id *head, const struct object_id *rem >> if (!index_tree) >> return error(_("Could not parse object '%s'."), oid_to_hex(&index)); >> >> - if (fast_forward_to(index_tree, remote_tree, 0)) >> + if (fast_forward_to(index_tree, remote_tree, UNPACK_NO_RESET)) >> return -1; >> >> if (merge_tree(remote_tree)) >> diff --git a/builtin/checkout.c b/builtin/checkout.c >> index ffa776c6e1..e9e70018f9 100644 >> --- a/builtin/checkout.c >> +++ b/builtin/checkout.c >> @@ -506,7 +506,7 @@ 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 = UNPACK_RESET_OVERWRITE_UNTRACKED; >> opts.merge = 1; >> opts.fn = oneway_merge; >> opts.verbose_update = o->show_progress; >> diff --git a/builtin/read-tree.c b/builtin/read-tree.c >> index 5c9c082595..23735adde9 100644 >> --- a/builtin/read-tree.c >> +++ b/builtin/read-tree.c >> @@ -114,6 +114,7 @@ static int git_read_tree_config(const char *var, const char *value, void *cb) >> int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) >> { >> int i, stage = 0; >> + int protect_untracked = -1; >> struct object_id oid; >> struct tree_desc t[MAX_UNPACK_TREES]; >> struct unpack_trees_options opts; >> @@ -140,6 +141,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) >> PARSE_OPT_NONEG }, >> OPT_BOOL('u', NULL, &opts.update, >> N_("update working tree with merge result")), >> + OPT_BOOL(0, "protect-untracked", &protect_untracked, >> + N_("do not overwrite untracked files")), >> { OPTION_CALLBACK, 0, "exclude-per-directory", &opts, >> N_("gitignore"), >> N_("allow explicitly ignored files to be overwritten"), >> @@ -209,8 +212,17 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) >> if ((opts.update || opts.index_only) && !opts.merge) >> die("%s is meaningless without -m, --reset, or --prefix", >> opts.update ? "-u" : "-i"); >> + if (protect_untracked >= 0) { >> + if (!opts.reset || !opts.update) >> + die("-%s-protect-untracked requires --reset and -u", >> + protect_untracked ? "" : "-no"); >> + opts.reset = UNPACK_RESET_PROTECT_UNTRACKED; >> + } >> if ((opts.dir && !opts.update)) >> die("--exclude-per-directory is meaningless unless -u"); >> + if (opts.dir && opts.reset == UNPACK_RESET_OVERWRITE_UNTRACKED) >> + warning("--exclude-per-directory without --preserve-untracked " >> + "has no effect"); >> if (opts.merge && !opts.index_only) >> setup_work_tree(); >> >> diff --git a/builtin/rebase.c b/builtin/rebase.c >> index 2e41ad5644..feb30a71f5 100644 >> --- a/builtin/rebase.c >> +++ b/builtin/rebase.c >> @@ -398,7 +398,7 @@ static int reset_head(struct object_id *oid, const char *action, >> unpack_tree_opts.update = 1; >> unpack_tree_opts.merge = 1; >> if (!detach_head) >> - unpack_tree_opts.reset = 1; >> + unpack_tree_opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED; >> >> if (repo_read_index_unmerged(the_repository) < 0) { >> ret = error(_("could not read index")); >> diff --git a/builtin/reset.c b/builtin/reset.c >> index 26ef9a7bd0..a39dd92fb2 100644 >> --- a/builtin/reset.c >> +++ b/builtin/reset.c >> @@ -70,7 +70,7 @@ static int reset_index(const struct object_id *oid, int reset_type, int quiet) >> opts.update = 1; >> /* fallthrough */ >> default: >> - opts.reset = 1; >> + opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED; >> } >> >> read_cache_unmerged(); >> diff --git a/builtin/stash.c b/builtin/stash.c >> index 2a8e6d09b4..175d1b62d3 100644 >> --- a/builtin/stash.c >> +++ b/builtin/stash.c >> @@ -227,7 +227,8 @@ static int clear_stash(int argc, const char **argv, const char *prefix) >> return do_clear_stash(); >> } >> >> -static int reset_tree(struct object_id *i_tree, int update, int reset) >> +static int reset_tree(struct object_id *i_tree, int update, >> + enum unpack_trees_reset_type reset) >> { >> int nr_trees = 1; >> struct unpack_trees_options opts; >> @@ -461,7 +462,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, >> } >> >> if (has_index) { >> - if (reset_tree(&index_tree, 0, 0)) >> + if (reset_tree(&index_tree, 0, UNPACK_NO_RESET)) >> return -1; >> } else { >> struct strbuf out = STRBUF_INIT; >> @@ -471,7 +472,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, >> return -1; >> } >> >> - if (reset_tree(&c_tree, 0, 1)) { >> + if (reset_tree(&c_tree, 0, UNPACK_RESET_OVERWRITE_UNTRACKED)) { >> strbuf_release(&out); >> return -1; >> } >> diff --git a/t/lib-read-tree.sh b/t/lib-read-tree.sh >> index b95f485606..22f516d91f 100644 >> --- a/t/lib-read-tree.sh >> +++ b/t/lib-read-tree.sh >> @@ -39,3 +39,14 @@ read_tree_u_must_fail () { >> test_cmp pre-dry-run-wt post-dry-run-wt && >> test_must_fail git read-tree "$@" >> } >> + >> +read_tree_u_must_fail_save_err () { >> + git ls-files -s >pre-dry-run && >> + git diff-files -p >pre-dry-run-wt && >> + test_must_fail git read-tree -n "$@" && >> + git ls-files -s >post-dry-run && >> + git diff-files -p >post-dry-run-wt && >> + test_cmp pre-dry-run post-dry-run && >> + test_cmp pre-dry-run-wt post-dry-run-wt && >> + test_must_fail git read-tree "$@" 2>actual-err >> +} >> diff --git a/t/t1005-read-tree-reset.sh b/t/t1005-read-tree-reset.sh >> index 83b09e1310..6c9dd6805b 100755 >> --- a/t/t1005-read-tree-reset.sh >> +++ b/t/t1005-read-tree-reset.sh >> @@ -19,15 +19,48 @@ test_expect_success 'setup' ' >> git add df && >> echo content >new && >> git add new && >> - git commit -m two >> + git commit -m two && >> + git ls-files >expect-two >> ' >> >> -test_expect_success 'reset should work' ' >> +test_expect_success '--protect-untracked option sanity checks' ' >> + read_tree_u_must_fail --reset --protect-untracked HEAD && >> + read_tree_u_must_fail --reset --no-protect-untracked HEAD && >> + read_tree_u_must_fail -m -u --protect-untracked HEAD && >> + read_tree_u_must_fail -m -u --no-protect-untracked >> +' >> + >> +test_expect_success 'reset should reset worktree' ' >> + echo changed >df && >> read_tree_u_must_succeed -u --reset HEAD^ && >> git ls-files >actual && >> test_cmp expect actual >> ' >> >> +test_expect_success 'reset --protect-untracked protects untracked file' ' >> + echo changed >new && >> + read_tree_u_must_fail_save_err -u --reset --protect-untracked HEAD && >> + echo "error: Untracked working tree file '\'new\'' would be overwritten by merge." >expected-err && >> + test_cmp expected-err actual-err >> +' >> + >> +test_expect_success 'reset --protect-untracked protects untracked directory' ' >> + rm new && >> + mkdir new && >> + echo untracked >new/untracked && >> + read_tree_u_must_fail_save_err -u --reset --protect-untracked HEAD && >> + echo "error: Updating '\'new\'' would lose untracked files in it" >expected-err && >> + test_cmp expected-err actual-err >> +' >> + >> +test_expect_success 'reset --protect-untracked resets' ' >> + rm -rf new && >> + echo changed >df/file && >> + read_tree_u_must_succeed -u --reset --protect-untracked HEAD && >> + git ls-files >actual-two && >> + test_cmp expect-two actual-two >> +' >> + >> test_expect_success 'reset should remove remnants from a failed merge' ' >> read_tree_u_must_succeed --reset -u HEAD && >> git ls-files -s >expect && >> diff --git a/unpack-trees.c b/unpack-trees.c >> index 50189909b8..b1722730fe 100644 >> --- a/unpack-trees.c >> +++ b/unpack-trees.c >> @@ -1917,7 +1917,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->reset == UNPACK_RESET_OVERWRITE_UNTRACKED || >> + !o->update) >> return 0; >> >> len = check_leading_path(ce->name, ce_namelen(ce)); >> diff --git a/unpack-trees.h b/unpack-trees.h >> index d344d7d296..732b262c4d 100644 >> --- a/unpack-trees.h >> +++ b/unpack-trees.h >> @@ -41,9 +41,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_NO_RESET = 0, >> + UNPACK_RESET_OVERWRITE_UNTRACKED, >> + UNPACK_RESET_PROTECT_UNTRACKED >> +}; >> + >> struct unpack_trees_options { >> - unsigned int reset, >> - merge, >> + enum unpack_trees_reset_type reset; >> + unsigned int merge, >> update, >> clone, >> index_only, >> -- >> 2.21.0 >> > >
diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt index d271842608..67864c6bbc 100644 --- a/Documentation/git-read-tree.txt +++ b/Documentation/git-read-tree.txt @@ -40,12 +40,18 @@ OPTIONS --reset:: Same as -m, except that unmerged entries are discarded instead of failing. When used with `-u`, updates leading to loss of - working tree changes will not abort the operation. + working tree changes will not abort the operation and + untracked files will be overwritten. Use `--protect-untracked` + to avoid overwriting untracked files. -u:: After a successful merge, update the files in the work tree with the result of the merge. +--protect-untracked:: + Prevent `--reset` `-u` from overwriting untracked files. Requires + `--reset` and `-u` (`-m` never overwrites untracked files). + -i:: Usually a merge requires the index file as well as the files in the working tree to be up to date with the @@ -89,7 +95,7 @@ OPTIONS existed in the original index file. --exclude-per-directory=<gitignore>:: - When running the command with `-u` and `-m` options, the + When using `-u` with `-m` or `--reset` `--protect-untracked` options, the merge result may need to overwrite paths that are not tracked in the current branch. The command usually refuses to proceed with the merge to avoid losing such a diff --git a/builtin/am.c b/builtin/am.c index 912d9821b1..a92394b682 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1854,7 +1854,8 @@ static void am_resolve(struct am_state *state) * true, any unmerged entries will be discarded. Returns 0 on success, -1 on * failure. */ -static int fast_forward_to(struct tree *head, struct tree *remote, int reset) +static int fast_forward_to(struct tree *head, struct tree *remote, + enum unpack_trees_reset_type reset) { struct lock_file lock_file = LOCK_INIT; struct unpack_trees_options opts; @@ -1942,7 +1943,8 @@ static int clean_index(const struct object_id *head, const struct object_id *rem read_cache_unmerged(); - if (fast_forward_to(head_tree, head_tree, 1)) + if (fast_forward_to(head_tree, head_tree, + UNPACK_RESET_OVERWRITE_UNTRACKED)) return -1; if (write_cache_as_tree(&index, 0, NULL)) @@ -1952,7 +1954,7 @@ static int clean_index(const struct object_id *head, const struct object_id *rem if (!index_tree) return error(_("Could not parse object '%s'."), oid_to_hex(&index)); - if (fast_forward_to(index_tree, remote_tree, 0)) + if (fast_forward_to(index_tree, remote_tree, UNPACK_NO_RESET)) return -1; if (merge_tree(remote_tree)) diff --git a/builtin/checkout.c b/builtin/checkout.c index ffa776c6e1..e9e70018f9 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -506,7 +506,7 @@ 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 = UNPACK_RESET_OVERWRITE_UNTRACKED; opts.merge = 1; opts.fn = oneway_merge; opts.verbose_update = o->show_progress; diff --git a/builtin/read-tree.c b/builtin/read-tree.c index 5c9c082595..23735adde9 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -114,6 +114,7 @@ static int git_read_tree_config(const char *var, const char *value, void *cb) int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) { int i, stage = 0; + int protect_untracked = -1; struct object_id oid; struct tree_desc t[MAX_UNPACK_TREES]; struct unpack_trees_options opts; @@ -140,6 +141,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) PARSE_OPT_NONEG }, OPT_BOOL('u', NULL, &opts.update, N_("update working tree with merge result")), + OPT_BOOL(0, "protect-untracked", &protect_untracked, + N_("do not overwrite untracked files")), { OPTION_CALLBACK, 0, "exclude-per-directory", &opts, N_("gitignore"), N_("allow explicitly ignored files to be overwritten"), @@ -209,8 +212,17 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) if ((opts.update || opts.index_only) && !opts.merge) die("%s is meaningless without -m, --reset, or --prefix", opts.update ? "-u" : "-i"); + if (protect_untracked >= 0) { + if (!opts.reset || !opts.update) + die("-%s-protect-untracked requires --reset and -u", + protect_untracked ? "" : "-no"); + opts.reset = UNPACK_RESET_PROTECT_UNTRACKED; + } if ((opts.dir && !opts.update)) die("--exclude-per-directory is meaningless unless -u"); + if (opts.dir && opts.reset == UNPACK_RESET_OVERWRITE_UNTRACKED) + warning("--exclude-per-directory without --preserve-untracked " + "has no effect"); if (opts.merge && !opts.index_only) setup_work_tree(); diff --git a/builtin/rebase.c b/builtin/rebase.c index 2e41ad5644..feb30a71f5 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -398,7 +398,7 @@ static int reset_head(struct object_id *oid, const char *action, unpack_tree_opts.update = 1; unpack_tree_opts.merge = 1; if (!detach_head) - unpack_tree_opts.reset = 1; + unpack_tree_opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED; if (repo_read_index_unmerged(the_repository) < 0) { ret = error(_("could not read index")); diff --git a/builtin/reset.c b/builtin/reset.c index 26ef9a7bd0..a39dd92fb2 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -70,7 +70,7 @@ static int reset_index(const struct object_id *oid, int reset_type, int quiet) opts.update = 1; /* fallthrough */ default: - opts.reset = 1; + opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED; } read_cache_unmerged(); diff --git a/builtin/stash.c b/builtin/stash.c index 2a8e6d09b4..175d1b62d3 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -227,7 +227,8 @@ static int clear_stash(int argc, const char **argv, const char *prefix) return do_clear_stash(); } -static int reset_tree(struct object_id *i_tree, int update, int reset) +static int reset_tree(struct object_id *i_tree, int update, + enum unpack_trees_reset_type reset) { int nr_trees = 1; struct unpack_trees_options opts; @@ -461,7 +462,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, } if (has_index) { - if (reset_tree(&index_tree, 0, 0)) + if (reset_tree(&index_tree, 0, UNPACK_NO_RESET)) return -1; } else { struct strbuf out = STRBUF_INIT; @@ -471,7 +472,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, return -1; } - if (reset_tree(&c_tree, 0, 1)) { + if (reset_tree(&c_tree, 0, UNPACK_RESET_OVERWRITE_UNTRACKED)) { strbuf_release(&out); return -1; } diff --git a/t/lib-read-tree.sh b/t/lib-read-tree.sh index b95f485606..22f516d91f 100644 --- a/t/lib-read-tree.sh +++ b/t/lib-read-tree.sh @@ -39,3 +39,14 @@ read_tree_u_must_fail () { test_cmp pre-dry-run-wt post-dry-run-wt && test_must_fail git read-tree "$@" } + +read_tree_u_must_fail_save_err () { + git ls-files -s >pre-dry-run && + git diff-files -p >pre-dry-run-wt && + test_must_fail git read-tree -n "$@" && + git ls-files -s >post-dry-run && + git diff-files -p >post-dry-run-wt && + test_cmp pre-dry-run post-dry-run && + test_cmp pre-dry-run-wt post-dry-run-wt && + test_must_fail git read-tree "$@" 2>actual-err +} diff --git a/t/t1005-read-tree-reset.sh b/t/t1005-read-tree-reset.sh index 83b09e1310..6c9dd6805b 100755 --- a/t/t1005-read-tree-reset.sh +++ b/t/t1005-read-tree-reset.sh @@ -19,15 +19,48 @@ test_expect_success 'setup' ' git add df && echo content >new && git add new && - git commit -m two + git commit -m two && + git ls-files >expect-two ' -test_expect_success 'reset should work' ' +test_expect_success '--protect-untracked option sanity checks' ' + read_tree_u_must_fail --reset --protect-untracked HEAD && + read_tree_u_must_fail --reset --no-protect-untracked HEAD && + read_tree_u_must_fail -m -u --protect-untracked HEAD && + read_tree_u_must_fail -m -u --no-protect-untracked +' + +test_expect_success 'reset should reset worktree' ' + echo changed >df && read_tree_u_must_succeed -u --reset HEAD^ && git ls-files >actual && test_cmp expect actual ' +test_expect_success 'reset --protect-untracked protects untracked file' ' + echo changed >new && + read_tree_u_must_fail_save_err -u --reset --protect-untracked HEAD && + echo "error: Untracked working tree file '\'new\'' would be overwritten by merge." >expected-err && + test_cmp expected-err actual-err +' + +test_expect_success 'reset --protect-untracked protects untracked directory' ' + rm new && + mkdir new && + echo untracked >new/untracked && + read_tree_u_must_fail_save_err -u --reset --protect-untracked HEAD && + echo "error: Updating '\'new\'' would lose untracked files in it" >expected-err && + test_cmp expected-err actual-err +' + +test_expect_success 'reset --protect-untracked resets' ' + rm -rf new && + echo changed >df/file && + read_tree_u_must_succeed -u --reset --protect-untracked HEAD && + git ls-files >actual-two && + test_cmp expect-two actual-two +' + test_expect_success 'reset should remove remnants from a failed merge' ' read_tree_u_must_succeed --reset -u HEAD && git ls-files -s >expect && diff --git a/unpack-trees.c b/unpack-trees.c index 50189909b8..b1722730fe 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1917,7 +1917,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->reset == UNPACK_RESET_OVERWRITE_UNTRACKED || + !o->update) return 0; len = check_leading_path(ce->name, ce_namelen(ce)); diff --git a/unpack-trees.h b/unpack-trees.h index d344d7d296..732b262c4d 100644 --- a/unpack-trees.h +++ b/unpack-trees.h @@ -41,9 +41,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_NO_RESET = 0, + UNPACK_RESET_OVERWRITE_UNTRACKED, + UNPACK_RESET_PROTECT_UNTRACKED +}; + struct unpack_trees_options { - unsigned int reset, - merge, + enum unpack_trees_reset_type reset; + unsigned int merge, update, clone, index_only,