merge-ort: remove code obsoleted by other changes
PATCH 1/1 merge-ort: remove code obsoleted by other changes
From: Elijah Newren <[email protected]>
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.
---
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(-)
merge-ort.c +2 -20
@@ -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
Signature · 1 line
gitgitgadget
Hi Elijah,
Quoted 24 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
Commit
66b209b86a("merge-ort: implementCE_SKIP_WORKTREEhandling with conflicted entries", 2021-03-20) added some code for merge-ort to handle conflicted andskip_worktreeentries 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 <[email protected]>---
merge-ort: remove code obsoleted by other changes
Quoted 6 lines
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/1302merge-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
Quoted 61 lines
merge-ort.c +2
-20@@ -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
Quoted 33 lines · Johannes Schindelin
Hi Elijah,
Quoted 28 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
Commit
66b209b86a("merge-ort: implementCE_SKIP_WORKTREEhandling with conflicted entries", 2021-03-20) added some code for merge-ort to handle conflicted andskip_worktreeentries 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 <[email protected]>---
merge-ort: remove code obsoleted by other changesPublished-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
Quoted 7 lines
Quoted 2 lines
merge-ort.c | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-)Nice!
Since I have been in the
merge-ortcode quite a bit as of late, I deem myself familiar enough with the code to dare offering my ACK.
Thanks for taking a look.
Quoted 5 lines
What is _particularly_ nice is that this patch removes an
lstat()call that was a bit of a concern for me when usingmerge-ortin 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 aboutlstat()calls that simply are not in the code.
Oh cool, glad there are other benefits too!