| Message ID | 20181209200449.16342-6-t.gummerer@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | introduce no-overlay and cached mode in git checkout | expand |
On Sun, Dec 9, 2018 at 9:05 PM Thomas Gummerer <t.gummerer@gmail.com> wrote: > @@ -302,15 +310,29 @@ static int checkout_paths(const struct checkout_opts *opts, > ce->ce_flags &= ~CE_MATCHED; > if (!opts->ignore_skipworktree && ce_skip_worktree(ce)) > continue; > - if (opts->source_tree && !(ce->ce_flags & CE_UPDATE)) > - /* > - * "git checkout tree-ish -- path", but this entry > - * is in the original index; it will not be checked > - * out to the working tree and it does not matter > - * if pathspec matched this entry. We will not do > - * anything to this entry at all. > - */ > - continue; > + if (opts->source_tree && !(ce->ce_flags & CE_UPDATE)) { > + if (!opts->overlay_mode && > + ce_path_match(&the_index, ce, &opts->pathspec, ps_matched)) { > + /* > + * "git checkout --no-overlay <tree-ish> -- path", > + * and the path is not in tree-ish, but is in > + * the current index, which means that it should > + * be removed. > + */ > + ce->ce_flags |= CE_MATCHED | CE_REMOVE | CE_WT_REMOVE; > + continue; > + } else { In non-overlay mode but when pathspec does not match, we come here too. > + /* > + * "git checkout tree-ish -- path", but this > + * entry is in the original index; it will not I think the missing key point in this comment block is "..is in the original index _and it's not in tree-ish_". In non-overlay mode, if pathspec does not match then it's safe to ignore too. But this logic starts too get to complex and hurt my brain. > + * be checked out to the working tree and it > + * does not matter if pathspec matched this > + * entry. We will not do anything to this entry > + * at all. > + */ > + continue; > + } > + } > /* > * Either this entry came from the tree-ish we are > * checking the paths out of, or we are checking out > @@ -1266,6 +1299,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) > "checkout", "control recursive updating of submodules", > PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater }, > OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")), > + OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode")), maybe add " (default)" to the help string. > OPT_END(), > }; >
On Sun, Dec 9, 2018 at 12:05 PM Thomas Gummerer <t.gummerer@gmail.com> wrote: > > Currently 'git checkout' is defined as an overlay operation, which > means that if in 'git checkout <tree-ish> -- [<pathspec>]' we have an > entry in the index that matches <pathspec>, but that doesn't exist in > <tree-ish>, that entry will not be removed from the index or the > working tree. > > Introduce a new --{,no-}overlay option, which allows using 'git > checkout' in non-overlay mode, thus removing files from the working > tree if they do not exist in <tree-ish> but match <pathspec>. > > Note that 'git checkout -p <tree-ish> -- [<pathspec>]' already works > this way, so no changes are needed for the patch mode. We disallow > 'git checkout --overlay -p' to avoid confusing users who would expect > to be able to force overlay mode in 'git checkout -p' this way. Whoa...that's interesting. To me, that argues even further that the traditional checkout behavior was wrong all along and the choice of --overlay vs. --no-overlay in the original implementation was a total oversight. I'm really tempted to say that --no-overlay should just be the default in checkout too...but maybe that's too high a hill to climb, at least for now. Making --overlap and -p incompatible is a reasonable first step. But you should probably add a comment to the -p option documentation that it implies --no-overlay. > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> > --- > builtin/checkout.c | 64 +++++++++++++++++++++++++++------- > t/t2025-checkout-no-overlay.sh | 47 +++++++++++++++++++++++++ > t/t9902-completion.sh | 1 + > 3 files changed, 99 insertions(+), 13 deletions(-) > create mode 100755 t/t2025-checkout-no-overlay.sh > > diff --git a/builtin/checkout.c b/builtin/checkout.c > index acdafc6e4c..0aef35bbc4 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -44,6 +44,7 @@ struct checkout_opts { > int ignore_skipworktree; > int ignore_other_worktrees; > int show_progress; > + int overlay_mode; > /* > * If new checkout options are added, skip_merge_working_tree > * should be updated accordingly. > @@ -132,7 +133,8 @@ static int skip_same_name(const struct cache_entry *ce, int pos) > return pos; > } > > -static int check_stage(int stage, const struct cache_entry *ce, int pos) > +static int check_stage(int stage, const struct cache_entry *ce, int pos, > + int overlay_mode) > { > while (pos < active_nr && > !strcmp(active_cache[pos]->name, ce->name)) { > @@ -140,6 +142,8 @@ static int check_stage(int stage, const struct cache_entry *ce, int pos) > return 0; > pos++; > } > + if (!overlay_mode) > + return 0; > if (stage == 2) > return error(_("path '%s' does not have our version"), ce->name); > else > @@ -165,7 +169,7 @@ static int check_stages(unsigned stages, const struct cache_entry *ce, int pos) > } > > static int checkout_stage(int stage, const struct cache_entry *ce, int pos, > - const struct checkout *state) > + const struct checkout *state, int overlay_mode) > { > while (pos < active_nr && > !strcmp(active_cache[pos]->name, ce->name)) { > @@ -173,6 +177,10 @@ static int checkout_stage(int stage, const struct cache_entry *ce, int pos, > return checkout_entry(active_cache[pos], state, NULL); > pos++; > } > + if (!overlay_mode) { > + unlink_entry(ce); > + return 0; > + } > if (stage == 2) > return error(_("path '%s' does not have our version"), ce->name); > else > @@ -302,15 +310,29 @@ static int checkout_paths(const struct checkout_opts *opts, > ce->ce_flags &= ~CE_MATCHED; > if (!opts->ignore_skipworktree && ce_skip_worktree(ce)) > continue; > - if (opts->source_tree && !(ce->ce_flags & CE_UPDATE)) > - /* > - * "git checkout tree-ish -- path", but this entry > - * is in the original index; it will not be checked > - * out to the working tree and it does not matter > - * if pathspec matched this entry. We will not do > - * anything to this entry at all. > - */ > - continue; > + if (opts->source_tree && !(ce->ce_flags & CE_UPDATE)) { > + if (!opts->overlay_mode && > + ce_path_match(&the_index, ce, &opts->pathspec, ps_matched)) { > + /* > + * "git checkout --no-overlay <tree-ish> -- path", > + * and the path is not in tree-ish, but is in > + * the current index, which means that it should > + * be removed. > + */ > + ce->ce_flags |= CE_MATCHED | CE_REMOVE | CE_WT_REMOVE; > + continue; > + } else { > + /* > + * "git checkout tree-ish -- path", but this > + * entry is in the original index; it will not > + * be checked out to the working tree and it > + * does not matter if pathspec matched this > + * entry. We will not do anything to this entry > + * at all. > + */ > + continue; > + } > + } > /* > * Either this entry came from the tree-ish we are > * checking the paths out of, or we are checking out > @@ -348,7 +370,7 @@ static int checkout_paths(const struct checkout_opts *opts, > if (opts->force) { > warning(_("path '%s' is unmerged"), ce->name); > } else if (opts->writeout_stage) { > - errs |= check_stage(opts->writeout_stage, ce, pos); > + errs |= check_stage(opts->writeout_stage, ce, pos, opts->overlay_mode); > } else if (opts->merge) { > errs |= check_stages((1<<2) | (1<<3), ce, pos); > } else { > @@ -375,12 +397,14 @@ static int checkout_paths(const struct checkout_opts *opts, > continue; > } > if (opts->writeout_stage) > - errs |= checkout_stage(opts->writeout_stage, ce, pos, &state); > + errs |= checkout_stage(opts->writeout_stage, ce, pos, &state, opts->overlay_mode); > else if (opts->merge) > errs |= checkout_merged(pos, &state); > pos = skip_same_name(ce, pos) - 1; > } > } > + remove_marked_cache_entries(&the_index, 1); > + remove_scheduled_dirs(); > errs |= finish_delayed_checkout(&state); > > if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) > @@ -542,6 +566,11 @@ static int skip_merge_working_tree(const struct checkout_opts *opts, > * opts->show_progress only impacts output so doesn't require a merge > */ > > + /* > + * opts->overlay_mode cannot be used with switching branches so is > + * not tested here > + */ > + > /* > * If we aren't creating a new branch any changes or updates will > * happen in the existing branch. Since that could only be updating > @@ -1178,6 +1207,10 @@ static int checkout_branch(struct checkout_opts *opts, > die(_("'%s' cannot be used with switching branches"), > "--patch"); > > + if (!opts->overlay_mode) > + die(_("'%s' cannot be used with switching branches"), > + "--no-overlay"); > + > if (opts->writeout_stage) > die(_("'%s' cannot be used with switching branches"), > "--ours/--theirs"); > @@ -1266,6 +1299,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) > "checkout", "control recursive updating of submodules", > PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater }, > OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")), > + OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode")), > OPT_END(), > }; > > @@ -1274,6 +1308,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) > opts.overwrite_ignore = 1; > opts.prefix = prefix; > opts.show_progress = -1; > + opts.overlay_mode = -1; > > git_config(git_checkout_config, &opts); > > @@ -1297,6 +1332,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) > if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1) > die(_("-b, -B and --orphan are mutually exclusive")); > > + if (opts.overlay_mode == 1 && opts.patch_mode) > + die(_("-p and --overlay are mutually exclusive")); > + > /* > * From here on, new_branch will contain the branch to be checked out, > * and new_branch_force and new_orphan_branch will tell us which one of > diff --git a/t/t2025-checkout-no-overlay.sh b/t/t2025-checkout-no-overlay.sh > new file mode 100755 > index 0000000000..3575321382 > --- /dev/null > +++ b/t/t2025-checkout-no-overlay.sh > @@ -0,0 +1,47 @@ > +#!/bin/sh > + > +test_description='checkout --no-overlay <tree-ish> -- <pathspec>' > + > +. ./test-lib.sh > + > +test_expect_success 'setup' ' > + git commit --allow-empty -m "initial" > +' > + > +test_expect_success 'checkout --no-overlay deletes files not in <tree>' ' > + >file && > + mkdir dir && > + >dir/file1 && > + git add file dir/file1 && > + git checkout --no-overlay HEAD -- file && > + test_path_is_missing file && > + test_path_is_file dir/file1 > +' > + > +test_expect_success 'checkout --no-overlay removing last file from directory' ' > + git checkout --no-overlay HEAD -- dir/file1 && > + test_path_is_missing dir > +' > + > +test_expect_success 'checkout -p --overlay is disallowed' ' > + test_must_fail git checkout -p --overlay HEAD 2>actual && > + test_i18ngrep "fatal: -p and --overlay are mutually exclusive" actual > +' > + > +test_expect_success '--no-overlay --theirs with M/D conflict deletes file' ' > + test_commit file1 file1 && > + test_commit file2 file2 && > + git rm --cached file1 && > + echo 1234 >file1 && > + F1=$(git rev-parse HEAD:file1) && > + F2=$(git rev-parse HEAD:file2) && > + { > + echo "100644 $F1 1 file1" && > + echo "100644 $F2 2 file1" > + } | git update-index --index-info && > + test_path_is_file file1 && > + git checkout --theirs --no-overlay -- file1 && > + test_path_is_missing file1 > +' > + > +test_done > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh > index 175f83d704..a3fd9a9630 100755 > --- a/t/t9902-completion.sh > +++ b/t/t9902-completion.sh > @@ -1436,6 +1436,7 @@ test_expect_success 'double dash "git checkout"' ' > --progress Z > --no-quiet Z > --no-... Z > + --overlay Z > EOF > ' > > -- > 2.20.0.405.gbc1bbc6f85 >
Elijah Newren <newren@gmail.com> writes: >> Note that 'git checkout -p <tree-ish> -- [<pathspec>]' already works >> this way, so no changes are needed for the patch mode. We disallow >> 'git checkout --overlay -p' to avoid confusing users who would expect >> to be able to force overlay mode in 'git checkout -p' this way. > > Whoa...that's interesting. To me, that argues even further that the > traditional checkout behavior was wrong all along and the choice of > --overlay vs. --no-overlay in the original implementation was a total > oversight. I'm really tempted to say that --no-overlay should just be > the default in checkout too...but maybe that's too high a hill to > climb, at least for now. You are exhibiting a typical reaction to a shiny new toy. The original "checkout paths out of the trees and/or the index to recover what was lost" is modeled after "cp -R .../else/where .", which is an overlay operation, and you wouldn't imagine complaining that "cp -R" is wrong not to remove things in the target, to be equivalent to "rm -fr where && cp -R .../else/where .". Each has its own uses. We just didn't have the other half of the pair. If anything, I would consider "checkout -p" that does not do overlay is a bug that needs to be fixed.
On Mon, Dec 10, 2018 at 7:07 PM Junio C Hamano <gitster@pobox.com> wrote: > > Elijah Newren <newren@gmail.com> writes: > > >> Note that 'git checkout -p <tree-ish> -- [<pathspec>]' already works > >> this way, so no changes are needed for the patch mode. We disallow > >> 'git checkout --overlay -p' to avoid confusing users who would expect > >> to be able to force overlay mode in 'git checkout -p' this way. > > > > Whoa...that's interesting. To me, that argues even further that the > > traditional checkout behavior was wrong all along and the choice of > > --overlay vs. --no-overlay in the original implementation was a total > > oversight. I'm really tempted to say that --no-overlay should just be > > the default in checkout too...but maybe that's too high a hill to > > climb, at least for now. > > You are exhibiting a typical reaction to a shiny new toy. > > The original "checkout paths out of the trees and/or the index to > recover what was lost" is modeled after "cp -R .../else/where .", > which is an overlay operation, and you wouldn't imagine complaining > that "cp -R" is wrong not to remove things in the target, to be > equivalent to "rm -fr where && cp -R .../else/where .". > > Each has its own uses. We just didn't have the other half of the > pair. Ah, modelled on cp -R. I think that rather than reacting "to a shiny new toy", I just had always had a different mental model AND failed to figure out what the original model was, leading me to always view it as buggy. Thanks for giving me the model I was missing. > If anything, I would consider "checkout -p" that does not do overlay > is a bug that needs to be fixed. Yeah, --no-overlay being the default for -p, and --overlay being the default otherwise is rather inconsistent. (Though I'm also fine with that being fixed by some future patch series.)
On 12/11, Junio C Hamano wrote: > Elijah Newren <newren@gmail.com> writes: > > >> Note that 'git checkout -p <tree-ish> -- [<pathspec>]' already works > >> this way, so no changes are needed for the patch mode. We disallow > >> 'git checkout --overlay -p' to avoid confusing users who would expect > >> to be able to force overlay mode in 'git checkout -p' this way. > > > > Whoa...that's interesting. To me, that argues even further that the > > traditional checkout behavior was wrong all along and the choice of > > --overlay vs. --no-overlay in the original implementation was a total > > oversight. I'm really tempted to say that --no-overlay should just be > > the default in checkout too...but maybe that's too high a hill to > > climb, at least for now. > > You are exhibiting a typical reaction to a shiny new toy. I wonder whether it's worth introducing a config option for this, so people could use this new mode by default if they wanted to, without having to type the extra argument? 'git checkout' is a plumbing command, but I see that some of the shell scripts in git.git are using it. Though they are only using it in the checkout branch mode, rather than the checkout paths mode. > The original "checkout paths out of the trees and/or the index to > recover what was lost" is modeled after "cp -R .../else/where .", > which is an overlay operation, and you wouldn't imagine complaining > that "cp -R" is wrong not to remove things in the target, to be > equivalent to "rm -fr where && cp -R .../else/where .". > > Each has its own uses. We just didn't have the other half of the > pair. > > If anything, I would consider "checkout -p" that does not do overlay > is a bug that needs to be fixed. >
On 12/10, Duy Nguyen wrote: > On Sun, Dec 9, 2018 at 9:05 PM Thomas Gummerer <t.gummerer@gmail.com> wrote: > > @@ -302,15 +310,29 @@ static int checkout_paths(const struct checkout_opts *opts, > > ce->ce_flags &= ~CE_MATCHED; > > if (!opts->ignore_skipworktree && ce_skip_worktree(ce)) > > continue; > > - if (opts->source_tree && !(ce->ce_flags & CE_UPDATE)) > > - /* > > - * "git checkout tree-ish -- path", but this entry > > - * is in the original index; it will not be checked > > - * out to the working tree and it does not matter > > - * if pathspec matched this entry. We will not do > > - * anything to this entry at all. > > - */ > > - continue; > > + if (opts->source_tree && !(ce->ce_flags & CE_UPDATE)) { > > + if (!opts->overlay_mode && > > + ce_path_match(&the_index, ce, &opts->pathspec, ps_matched)) { > > + /* > > + * "git checkout --no-overlay <tree-ish> -- path", > > + * and the path is not in tree-ish, but is in > > + * the current index, which means that it should > > + * be removed. > > + */ > > + ce->ce_flags |= CE_MATCHED | CE_REMOVE | CE_WT_REMOVE; > > + continue; > > + } else { > > In non-overlay mode but when pathspec does not match, we come here too. > > > + /* > > + * "git checkout tree-ish -- path", but this > > + * entry is in the original index; it will not > > I think the missing key point in this comment block is "..is in the > original index _and it's not in tree-ish_". In non-overlay mode, if > pathspec does not match then it's safe to ignore too. But this logic > starts too get to complex and hurt my brain. Yes, that would make it a bit easier to read. I took a while to try and refactor this to make it easier to read, but couldn't come up with anything much better unfortunately. I'll have another stab at simplifying the logic a bit for v2. > > + * be checked out to the working tree and it > > + * does not matter if pathspec matched this > > + * entry. We will not do anything to this entry > > + * at all. > > + */ > > + continue; > > + } > > + } > > /* > > * Either this entry came from the tree-ish we are > > * checking the paths out of, or we are checking out > > > @@ -1266,6 +1299,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) > > "checkout", "control recursive updating of submodules", > > PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater }, > > OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")), > > + OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode")), > > maybe add " (default)" to the help string. Makes sense, will add. > > OPT_END(), > > }; > > > -- > Duy
diff --git a/builtin/checkout.c b/builtin/checkout.c index acdafc6e4c..0aef35bbc4 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -44,6 +44,7 @@ struct checkout_opts { int ignore_skipworktree; int ignore_other_worktrees; int show_progress; + int overlay_mode; /* * If new checkout options are added, skip_merge_working_tree * should be updated accordingly. @@ -132,7 +133,8 @@ static int skip_same_name(const struct cache_entry *ce, int pos) return pos; } -static int check_stage(int stage, const struct cache_entry *ce, int pos) +static int check_stage(int stage, const struct cache_entry *ce, int pos, + int overlay_mode) { while (pos < active_nr && !strcmp(active_cache[pos]->name, ce->name)) { @@ -140,6 +142,8 @@ static int check_stage(int stage, const struct cache_entry *ce, int pos) return 0; pos++; } + if (!overlay_mode) + return 0; if (stage == 2) return error(_("path '%s' does not have our version"), ce->name); else @@ -165,7 +169,7 @@ static int check_stages(unsigned stages, const struct cache_entry *ce, int pos) } static int checkout_stage(int stage, const struct cache_entry *ce, int pos, - const struct checkout *state) + const struct checkout *state, int overlay_mode) { while (pos < active_nr && !strcmp(active_cache[pos]->name, ce->name)) { @@ -173,6 +177,10 @@ static int checkout_stage(int stage, const struct cache_entry *ce, int pos, return checkout_entry(active_cache[pos], state, NULL); pos++; } + if (!overlay_mode) { + unlink_entry(ce); + return 0; + } if (stage == 2) return error(_("path '%s' does not have our version"), ce->name); else @@ -302,15 +310,29 @@ static int checkout_paths(const struct checkout_opts *opts, ce->ce_flags &= ~CE_MATCHED; if (!opts->ignore_skipworktree && ce_skip_worktree(ce)) continue; - if (opts->source_tree && !(ce->ce_flags & CE_UPDATE)) - /* - * "git checkout tree-ish -- path", but this entry - * is in the original index; it will not be checked - * out to the working tree and it does not matter - * if pathspec matched this entry. We will not do - * anything to this entry at all. - */ - continue; + if (opts->source_tree && !(ce->ce_flags & CE_UPDATE)) { + if (!opts->overlay_mode && + ce_path_match(&the_index, ce, &opts->pathspec, ps_matched)) { + /* + * "git checkout --no-overlay <tree-ish> -- path", + * and the path is not in tree-ish, but is in + * the current index, which means that it should + * be removed. + */ + ce->ce_flags |= CE_MATCHED | CE_REMOVE | CE_WT_REMOVE; + continue; + } else { + /* + * "git checkout tree-ish -- path", but this + * entry is in the original index; it will not + * be checked out to the working tree and it + * does not matter if pathspec matched this + * entry. We will not do anything to this entry + * at all. + */ + continue; + } + } /* * Either this entry came from the tree-ish we are * checking the paths out of, or we are checking out @@ -348,7 +370,7 @@ static int checkout_paths(const struct checkout_opts *opts, if (opts->force) { warning(_("path '%s' is unmerged"), ce->name); } else if (opts->writeout_stage) { - errs |= check_stage(opts->writeout_stage, ce, pos); + errs |= check_stage(opts->writeout_stage, ce, pos, opts->overlay_mode); } else if (opts->merge) { errs |= check_stages((1<<2) | (1<<3), ce, pos); } else { @@ -375,12 +397,14 @@ static int checkout_paths(const struct checkout_opts *opts, continue; } if (opts->writeout_stage) - errs |= checkout_stage(opts->writeout_stage, ce, pos, &state); + errs |= checkout_stage(opts->writeout_stage, ce, pos, &state, opts->overlay_mode); else if (opts->merge) errs |= checkout_merged(pos, &state); pos = skip_same_name(ce, pos) - 1; } } + remove_marked_cache_entries(&the_index, 1); + remove_scheduled_dirs(); errs |= finish_delayed_checkout(&state); if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) @@ -542,6 +566,11 @@ static int skip_merge_working_tree(const struct checkout_opts *opts, * opts->show_progress only impacts output so doesn't require a merge */ + /* + * opts->overlay_mode cannot be used with switching branches so is + * not tested here + */ + /* * If we aren't creating a new branch any changes or updates will * happen in the existing branch. Since that could only be updating @@ -1178,6 +1207,10 @@ static int checkout_branch(struct checkout_opts *opts, die(_("'%s' cannot be used with switching branches"), "--patch"); + if (!opts->overlay_mode) + die(_("'%s' cannot be used with switching branches"), + "--no-overlay"); + if (opts->writeout_stage) die(_("'%s' cannot be used with switching branches"), "--ours/--theirs"); @@ -1266,6 +1299,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) "checkout", "control recursive updating of submodules", PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater }, OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")), + OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode")), OPT_END(), }; @@ -1274,6 +1308,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts.overwrite_ignore = 1; opts.prefix = prefix; opts.show_progress = -1; + opts.overlay_mode = -1; git_config(git_checkout_config, &opts); @@ -1297,6 +1332,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1) die(_("-b, -B and --orphan are mutually exclusive")); + if (opts.overlay_mode == 1 && opts.patch_mode) + die(_("-p and --overlay are mutually exclusive")); + /* * From here on, new_branch will contain the branch to be checked out, * and new_branch_force and new_orphan_branch will tell us which one of diff --git a/t/t2025-checkout-no-overlay.sh b/t/t2025-checkout-no-overlay.sh new file mode 100755 index 0000000000..3575321382 --- /dev/null +++ b/t/t2025-checkout-no-overlay.sh @@ -0,0 +1,47 @@ +#!/bin/sh + +test_description='checkout --no-overlay <tree-ish> -- <pathspec>' + +. ./test-lib.sh + +test_expect_success 'setup' ' + git commit --allow-empty -m "initial" +' + +test_expect_success 'checkout --no-overlay deletes files not in <tree>' ' + >file && + mkdir dir && + >dir/file1 && + git add file dir/file1 && + git checkout --no-overlay HEAD -- file && + test_path_is_missing file && + test_path_is_file dir/file1 +' + +test_expect_success 'checkout --no-overlay removing last file from directory' ' + git checkout --no-overlay HEAD -- dir/file1 && + test_path_is_missing dir +' + +test_expect_success 'checkout -p --overlay is disallowed' ' + test_must_fail git checkout -p --overlay HEAD 2>actual && + test_i18ngrep "fatal: -p and --overlay are mutually exclusive" actual +' + +test_expect_success '--no-overlay --theirs with M/D conflict deletes file' ' + test_commit file1 file1 && + test_commit file2 file2 && + git rm --cached file1 && + echo 1234 >file1 && + F1=$(git rev-parse HEAD:file1) && + F2=$(git rev-parse HEAD:file2) && + { + echo "100644 $F1 1 file1" && + echo "100644 $F2 2 file1" + } | git update-index --index-info && + test_path_is_file file1 && + git checkout --theirs --no-overlay -- file1 && + test_path_is_missing file1 +' + +test_done diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 175f83d704..a3fd9a9630 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -1436,6 +1436,7 @@ test_expect_success 'double dash "git checkout"' ' --progress Z --no-quiet Z --no-... Z + --overlay Z EOF '
Currently 'git checkout' is defined as an overlay operation, which means that if in 'git checkout <tree-ish> -- [<pathspec>]' we have an entry in the index that matches <pathspec>, but that doesn't exist in <tree-ish>, that entry will not be removed from the index or the working tree. Introduce a new --{,no-}overlay option, which allows using 'git checkout' in non-overlay mode, thus removing files from the working tree if they do not exist in <tree-ish> but match <pathspec>. Note that 'git checkout -p <tree-ish> -- [<pathspec>]' already works this way, so no changes are needed for the patch mode. We disallow 'git checkout --overlay -p' to avoid confusing users who would expect to be able to force overlay mode in 'git checkout -p' this way. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> --- builtin/checkout.c | 64 +++++++++++++++++++++++++++------- t/t2025-checkout-no-overlay.sh | 47 +++++++++++++++++++++++++ t/t9902-completion.sh | 1 + 3 files changed, 99 insertions(+), 13 deletions(-) create mode 100755 t/t2025-checkout-no-overlay.sh