Page MenuHomePhabricator

merge: store ACTION_KEEP_ABSENT when we are keeping the file absent locally
ClosedPublic

Authored by pulkit on Aug 24 2020, 10:14 AM.

Details

Summary

If a file is not present on the local side, and it's unchanged between other
merge parent and ancestor, we don't use any action, neither we had a if-else
branch for that condition.

This leads to bid-merge missing that there is a such action possible which can
be performed. As test changes demonstrate, we now choose the locally deleted
side instead of choosing the remote one.

This whole logic is not acurrate as we should prompt user on what to do
when this kind of criss-cross merge is in play.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Aug 24 2020, 10:14 AM

Using ACTION_KEEP for a file that does not exist seems a bti scary. Did we not refactor the merge code to allow to express this ? (Do not touch working copy and dirstate, but something happened?)

Using ACTION_KEEP for a file that does not exist seems a bti scary. Did we not refactor the merge code to allow to express this ? (Do not touch working copy and dirstate, but something happened?)

ACTION_KEEP is used for the same purpose: Do not touch working copy and dirstate, but something happened.

We can have a more dedicated ACTION_KEEP_DELETED

pulkit updated this revision to Diff 22473.Aug 27 2020, 5:38 AM

Having a dedicated action would seems cleaner.

pulkit retitled this revision from merge: store ACTION_KEEP when we are keeping the file deleted locally to merge: store ACTION_KEEP_DELETED when we are keeping the file deleted locally.
pulkit edited the summary of this revision. (Show Details)
pulkit updated this revision to Diff 22498.
pulkit added a comment.Sep 1 2020, 8:13 AM

Having a dedicated action would seems cleaner.

Added a dedicated action ACTION_KEEP_DELETED and used it.

pulkit retitled this revision from merge: store ACTION_KEEP_DELETED when we are keeping the file deleted locally to merge: store ACTION_KEEP_ABSENT when we are keeping the file absent locally.Sep 2 2020, 7:53 AM
pulkit updated this revision to Diff 22509.

So right now the bid merge still behave in a bad way (delete the file instead of conflicting, but at least not it consistently does so regardless of where the merge is started?

If so, please update the changeset description to point this out.

I am also pondering if we should have a summary of the current issues within the test file, so that we can clean up while fixing this.

marmoute requested changes to this revision.Sep 11 2020, 5:40 AM
This revision now requires changes to proceed.Sep 11 2020, 5:40 AM
This revision was not accepted when it landed; it landed in state Needs Revision.
This revision was automatically updated to reflect the committed changes.