Hi Elijah, On Fri, 19 Aug 2022, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@xxxxxxxxx> > > Commit 66b209b86a ("merge-ort: implement CE_SKIP_WORKTREE handling with > conflicted entries", 2021-03-20) added some code for merge-ort to handle > conflicted and skip_worktree entries in general. Included in this was > an ugly hack for dealing with present-despite-skipped entries and a > testcase (t6428.2) specific to that hack, since at that time users could > accidentally get files into that state when using a sparse checkout. > > However, with the merging of 82386b4496 ("Merge branch > 'en/present-despite-skipped'", 2022-03-09), that class of problems was > addressed globally and in a much cleaner way. As such, the > present-despite-skipped hack in merge-ort is no longer needed and can > simply be removed. > > No additional testcase is needed here; t6428.2 was written to test the > necessary functionality and is being kept. The fact that this test > continues to pass despite the code being removed shows that the extra > code is no longer necessary. > > Signed-off-by: Elijah Newren <newren@xxxxxxxxx> > --- > merge-ort: remove code obsoleted by other changes > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1302%2Fnewren%2Fnuke-present-despite-skipped-hack-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1302/newren/nuke-present-despite-skipped-hack-v1 > Pull-Request: https://github.com/git/git/pull/1302 > > merge-ort.c | 22 ++-------------------- > 1 file changed, 2 insertions(+), 20 deletions(-) Nice! Since I have been in the `merge-ort` code quite a bit as of late, I deem myself familiar enough with the code to dare offering my ACK. What is _particularly_ nice is that this patch removes an `lstat()` call that was a bit of a concern for me when using `merge-ort` in a worktree-less scenario. After some reasoning about the code, it turned out that it is not hit in that use case, nevertheless it is much easier to reason about `lstat()` calls that simply are not in the code. Thank you! Dscho > > diff --git a/merge-ort.c b/merge-ort.c > index 8b7de0fbd8e..a6a3ab839a0 100644 > --- a/merge-ort.c > +++ b/merge-ort.c > @@ -491,7 +491,6 @@ enum conflict_and_info_types { > CONFLICT_FILE_DIRECTORY, > CONFLICT_DISTINCT_MODES, > CONFLICT_MODIFY_DELETE, > - CONFLICT_PRESENT_DESPITE_SKIPPED, > > /* Regular rename */ > CONFLICT_RENAME_RENAME, /* same file renamed differently */ > @@ -536,8 +535,6 @@ static const char *type_short_descriptions[] = { > [CONFLICT_FILE_DIRECTORY] = "CONFLICT (file/directory)", > [CONFLICT_DISTINCT_MODES] = "CONFLICT (distinct modes)", > [CONFLICT_MODIFY_DELETE] = "CONFLICT (modify/delete)", > - [CONFLICT_PRESENT_DESPITE_SKIPPED] = > - "CONFLICT (upgrade your version of git)", > > /*** Regular rename ***/ > [CONFLICT_RENAME_RENAME] = "CONFLICT (rename/rename)", > @@ -748,8 +745,7 @@ static void path_msg(struct merge_options *opt, > /* Sanity checks */ > assert(omittable_hint == > !starts_with(type_short_descriptions[type], "CONFLICT") || > - type == CONFLICT_DIR_RENAME_SUGGESTED || > - type == CONFLICT_PRESENT_DESPITE_SKIPPED); > + type == CONFLICT_DIR_RENAME_SUGGESTED); > if (opt->record_conflict_msgs_as_headers && omittable_hint) > return; /* Do not record mere hints in headers */ > if (opt->priv->call_depth && opt->verbosity < 5) > @@ -4377,22 +4373,8 @@ static int record_conflicted_index_entries(struct merge_options *opt) > * the CE_SKIP_WORKTREE bit and manually write those > * files to the working disk here. > */ > - if (ce_skip_worktree(ce)) { > - struct stat st; > - > - if (!lstat(path, &st)) { > - char *new_name = unique_path(opt, > - path, > - "cruft"); > - > - path_msg(opt, CONFLICT_PRESENT_DESPITE_SKIPPED, 1, > - path, NULL, NULL, NULL, > - _("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); > - } > + if (ce_skip_worktree(ce)) > errs |= checkout_entry(ce, &state, NULL, NULL); > - } > > /* > * Mark this cache entry for removal and instead add > > base-commit: 6a475b71f8c4ce708d69fdc9317aefbde3769e25 > -- > gitgitgadget >