sageserpent-open / kineticMerge

Merge a heavily refactored codebase and stay sane.
MIT License
9 stars 1 forks source link

`MatchError` faults Kinetic Merge when merging a file deleted on one side and modified on another. #40

Closed sageserpent-open closed 1 month ago

sageserpent-open commented 1 month ago

This arose in connection with: https://github.com/sageserpent-open/kineticMerge/issues/39#issuecomment-2105935589.

While there is an existing test MainTest.conflictingModificationAndDeletionOfTheSameFile that passes, it is only covering modifications that insert content.

What happened on #39 with a real-world example was that a modification edited text on the opposite side of the merge from the deletion of the entire file - this breaks a pattern match that was treated as irrefutable when in reality it is not.

The job here is to reproduce the failure - probably best to write a new test and fix it.

Once this is done, the example merge in #39 can be checked as a good manual real-world test.

sageserpent-open commented 1 month ago

Incidentially, suppose the file is modified by deleting all of its content, but leaving an empty file behind.

What does Git do in this situation?

Should write a test to document this. The implementation will merge the two outcomes cleanly by deleting the file, and that seems reasonable.

sageserpent-open commented 1 month ago

Test added to reproduce bug and then fixed on: df31b317ebb5c919ec0a9c103690251b9921e398.

Should add a test to cover the previous comment and run Git to see what it does.

sageserpent-open commented 1 month ago

Manual testing as per #39 yields a merge with a conflict between our deleted file and their modified file. It's open to debate as to whether what's in the merged form of the file is sensible or not.

Rather than getting bogged down in the correctness of the merge for that manual test, let's finish off this ticket and revisit the manual merge once both #39 and #30 are delivered.

sageserpent-open commented 1 month ago

Git treats the merge of a file deleted on one branch with having its content emptied on another branch as a simple conflict between deletion and the existence of an empty file.

Inspection of the codebase of Kinetic Merge indicates that this should also be the outcome for this special case merge. A test was added in commit: 31725230153edb95af1597104ffb18d19e237db0 to verify this, and was checked by injecting faults into the two cases in the code that handle deletion versus modification merges.

With that, it is delivered.

sageserpent-open commented 1 month ago

This went out in release 1.2.0, Git commit SHA: 4d1935beef2b5923417904895884de8c2e749c6e.