| Message ID | 100c0187bdfeef5c560ecd17160ed7c9a3032156.1614905738.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Complete merge-ort implementation...almost | expand |
On Fri, Mar 05 2021, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@gmail.com> > > When merge conflicts occur in paths removed by a sparse-checkout, we > need to unsparsify those paths (clear the SKIP_WORKTREE bit), and write > out the conflicted file to the working copy. In the very unlikely case > that someone manually put a file into the working copy at the location > of the SKIP_WORKTREE file, we need to avoid overwriting whatever edits > they have made and move that file to a different location first. > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > merge-ort.c | 43 +++++++++++++++++++++---------- > t/t6428-merge-conflicts-sparse.sh | 4 +-- > 2 files changed, 32 insertions(+), 15 deletions(-) > > diff --git a/merge-ort.c b/merge-ort.c > index a998f843a1da..37b69cbe0f9a 100644 > --- a/merge-ort.c > +++ b/merge-ort.c > @@ -3235,23 +3235,27 @@ static int checkout(struct merge_options *opt, > return ret; > } > > -static int record_conflicted_index_entries(struct merge_options *opt, > - struct index_state *index, > - struct strmap *paths, > - struct strmap *conflicted) > +static int record_conflicted_index_entries(struct merge_options *opt) > { > struct hashmap_iter iter; > struct strmap_entry *e; > + struct index_state *index = opt->repo->index; > + struct checkout state = CHECKOUT_INIT; > int errs = 0; > int original_cache_nr; > > - if (strmap_empty(conflicted)) > + if (strmap_empty(&opt->priv->conflicted)) > return 0; > > + /* If any entries have skip_worktree set, we'll have to check 'em out */ > + state.force = 1; > + state.quiet = 1; > + state.refresh_cache = 1; > + state.istate = index; > original_cache_nr = index->cache_nr; > > /* Put every entry from paths into plist, then sort */ > - strmap_for_each_entry(conflicted, &iter, e) { > + strmap_for_each_entry(&opt->priv->conflicted, &iter, e) { > const char *path = e->key; > struct conflict_info *ci = e->value; > int pos; > @@ -3292,9 +3296,23 @@ static int record_conflicted_index_entries(struct merge_options *opt, > * the higher order stages. Thus, we need override > * the CE_SKIP_WORKTREE bit and manually write those > * files to the working disk here. > - * > - * TODO: Implement this CE_SKIP_WORKTREE fixup. > */ > + if (ce_skip_worktree(ce)) { > + struct stat st; > + > + if (!lstat(path, &st)) { > + char *new_name = unique_path(&opt->priv->paths, > + path, > + "cruft"); > + > + path_msg(opt, path, 1, > + _("Note: %s not up to date and in way of checking out conflicted version; old copy renamed to %s"), > + path, new_name); I see this follows existing uses in merge-ort.c, but I wonder if this won't be quite unreadable on long paths, i.e.: <long x> renamed to <long x.new> As opposed to: We had to rename your thing: from: <long x> to: <long x.new>
On Mon, Mar 8, 2021 at 5:06 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > > On Fri, Mar 05 2021, Elijah Newren via GitGitGadget wrote: > > > From: Elijah Newren <newren@gmail.com> > > > > When merge conflicts occur in paths removed by a sparse-checkout, we > > need to unsparsify those paths (clear the SKIP_WORKTREE bit), and write > > out the conflicted file to the working copy. In the very unlikely case > > that someone manually put a file into the working copy at the location > > of the SKIP_WORKTREE file, we need to avoid overwriting whatever edits > > they have made and move that file to a different location first. > > > > Signed-off-by: Elijah Newren <newren@gmail.com> > > --- > > merge-ort.c | 43 +++++++++++++++++++++---------- > > t/t6428-merge-conflicts-sparse.sh | 4 +-- > > 2 files changed, 32 insertions(+), 15 deletions(-) > > > > diff --git a/merge-ort.c b/merge-ort.c > > index a998f843a1da..37b69cbe0f9a 100644 > > --- a/merge-ort.c > > +++ b/merge-ort.c > > @@ -3235,23 +3235,27 @@ static int checkout(struct merge_options *opt, > > return ret; > > } > > > > -static int record_conflicted_index_entries(struct merge_options *opt, > > - struct index_state *index, > > - struct strmap *paths, > > - struct strmap *conflicted) > > +static int record_conflicted_index_entries(struct merge_options *opt) > > { > > struct hashmap_iter iter; > > struct strmap_entry *e; > > + struct index_state *index = opt->repo->index; > > + struct checkout state = CHECKOUT_INIT; > > int errs = 0; > > int original_cache_nr; > > > > - if (strmap_empty(conflicted)) > > + if (strmap_empty(&opt->priv->conflicted)) > > return 0; > > > > + /* If any entries have skip_worktree set, we'll have to check 'em out */ > > + state.force = 1; > > + state.quiet = 1; > > + state.refresh_cache = 1; > > + state.istate = index; > > original_cache_nr = index->cache_nr; > > > > /* Put every entry from paths into plist, then sort */ > > - strmap_for_each_entry(conflicted, &iter, e) { > > + strmap_for_each_entry(&opt->priv->conflicted, &iter, e) { > > const char *path = e->key; > > struct conflict_info *ci = e->value; > > int pos; > > @@ -3292,9 +3296,23 @@ static int record_conflicted_index_entries(struct merge_options *opt, > > * the higher order stages. Thus, we need override > > * the CE_SKIP_WORKTREE bit and manually write those > > * files to the working disk here. > > - * > > - * TODO: Implement this CE_SKIP_WORKTREE fixup. > > */ > > + if (ce_skip_worktree(ce)) { > > + struct stat st; > > + > > + if (!lstat(path, &st)) { > > + char *new_name = unique_path(&opt->priv->paths, > > + path, > > + "cruft"); > > + > > + path_msg(opt, path, 1, > > + _("Note: %s not up to date and in way of checking out conflicted version; old copy renamed to %s"), > > + path, new_name); > > I see this follows existing uses in merge-ort.c, but I wonder if this > won't be quite unreadable on long paths, i.e.: > > <long x> renamed to <long x.new> > > As opposed to: > > We had to rename your thing: > from: <long x> > to: <long x.new> Makes sense, but it seems like something we'd want to do to a lot of messages rather than just this one. For now, especially given that I expect this particular message to be *very* rare, I think I'll leave this one as-is for now but we can address this idea in a subsequent series or as #leftoverbits.
diff --git a/merge-ort.c b/merge-ort.c index a998f843a1da..37b69cbe0f9a 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -3235,23 +3235,27 @@ static int checkout(struct merge_options *opt, return ret; } -static int record_conflicted_index_entries(struct merge_options *opt, - struct index_state *index, - struct strmap *paths, - struct strmap *conflicted) +static int record_conflicted_index_entries(struct merge_options *opt) { struct hashmap_iter iter; struct strmap_entry *e; + struct index_state *index = opt->repo->index; + struct checkout state = CHECKOUT_INIT; int errs = 0; int original_cache_nr; - if (strmap_empty(conflicted)) + if (strmap_empty(&opt->priv->conflicted)) return 0; + /* If any entries have skip_worktree set, we'll have to check 'em out */ + state.force = 1; + state.quiet = 1; + state.refresh_cache = 1; + state.istate = index; original_cache_nr = index->cache_nr; /* Put every entry from paths into plist, then sort */ - strmap_for_each_entry(conflicted, &iter, e) { + strmap_for_each_entry(&opt->priv->conflicted, &iter, e) { const char *path = e->key; struct conflict_info *ci = e->value; int pos; @@ -3292,9 +3296,23 @@ static int record_conflicted_index_entries(struct merge_options *opt, * the higher order stages. Thus, we need override * the CE_SKIP_WORKTREE bit and manually write those * files to the working disk here. - * - * TODO: Implement this CE_SKIP_WORKTREE fixup. */ + if (ce_skip_worktree(ce)) { + struct stat st; + + if (!lstat(path, &st)) { + char *new_name = unique_path(&opt->priv->paths, + path, + "cruft"); + + path_msg(opt, path, 1, + _("Note: %s not up to date and in way of checking out conflicted version; old copy renamed to %s"), + path, new_name); + errs |= rename(path, new_name); + free(new_name); + } + errs |= checkout_entry(ce, &state, NULL, NULL); + } /* * Mark this cache entry for removal and instead add @@ -3344,8 +3362,6 @@ void merge_switch_to_result(struct merge_options *opt, { assert(opt->priv == NULL); if (result->clean >= 0 && update_worktree_and_index) { - struct merge_options_internal *opti = result->priv; - trace2_region_enter("merge", "checkout", opt->repo); if (checkout(opt, head, result->tree)) { /* failure to function */ @@ -3355,13 +3371,14 @@ void merge_switch_to_result(struct merge_options *opt, trace2_region_leave("merge", "checkout", opt->repo); trace2_region_enter("merge", "record_conflicted", opt->repo); - if (record_conflicted_index_entries(opt, opt->repo->index, - &opti->paths, - &opti->conflicted)) { + opt->priv = result->priv; + if (record_conflicted_index_entries(opt)) { /* failure to function */ + opt->priv = NULL; result->clean = -1; return; } + opt->priv = NULL; trace2_region_leave("merge", "record_conflicted", opt->repo); } diff --git a/t/t6428-merge-conflicts-sparse.sh b/t/t6428-merge-conflicts-sparse.sh index 1bb52ff6f38c..7e8bf497f821 100755 --- a/t/t6428-merge-conflicts-sparse.sh +++ b/t/t6428-merge-conflicts-sparse.sh @@ -76,7 +76,7 @@ test_setup_numerals () { ) } -test_expect_merge_algorithm success failure 'conflicting entries written to worktree even if sparse' ' +test_expect_success 'conflicting entries written to worktree even if sparse' ' test_setup_numerals plain && ( cd numerals_plain && @@ -112,7 +112,7 @@ test_expect_merge_algorithm success failure 'conflicting entries written to work ) ' -test_expect_merge_algorithm failure failure 'present-despite-SKIP_WORKTREE handled reasonably' ' +test_expect_merge_algorithm failure success 'present-despite-SKIP_WORKTREE handled reasonably' ' test_setup_numerals in_the_way && ( cd numerals_in_the_way &&