Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(add): Merge deletions support #1836

Merged
merged 15 commits into from
Oct 31, 2023
Merged

feat(add): Merge deletions support #1836

merged 15 commits into from
Oct 31, 2023

Conversation

DanilKazanov
Copy link
Contributor

@DanilKazanov DanilKazanov commented Oct 30, 2023

This PR adds merge deletions support to isomorhic-git. if a file that was deleted in one branch and modified in another branch, MergeConflictError is returned with the data.
For example, I have repo with two branches: master and develop, and file a. Then I change file a in master and delete file a in branch develop.

I merge develop in master (delete by them):

With native git:

  • Native git status:
$ git status
On branch master
You have unmerged paths.
  (fix conflicts and run "git commit")
  (use "git merge --abort" to abort the merge)

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
        deleted by them: a

no changes added to commit (use "git add" and/or "git commit -a")
  • Native git status --short:
$ git status --short
UD a
  • isomorphic-git statusMatrix:
    [ 'a', 1, 1, 3 ]

With isomorphic-git:

  • Native git status:
$ git status
On branch master
Unmerged paths:
  (use "git restore --staged <file>..." to unstage)
  (use "git add/rm <file>..." as appropriate to mark resolution)
        deleted by them: a

no changes added to commit (use "git add" and/or "git commit -a")
  • Native git status --short:
$ git status --short
UD a
  • isomorphic-git statusMatrix:
    [ 'a', 1, 1, 3 ]

As you can see, they are the same.

I merge master in develop (delete by us):

With native git:

  • Native git status:
$ git status
On branch develop
You have unmerged paths.
  (fix conflicts and run "git commit")
  (use "git merge --abort" to abort the merge)

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
        deleted by us:   a

no changes added to commit (use "git add" and/or "git commit -a")
  • Native git status --short:
$ git status --short
DU a
  • isomorphic-git statusMatrix:
    [ 'a', 0, 2, 3 ]

With isomorphic-git:

  • Native git status:
$ git status
On branch develop
Unmerged paths:
  (use "git restore --staged <file>..." to unstage)
  (use "git add/rm <file>..." as appropriate to mark resolution)
        deleted by us:   a

no changes added to commit (use "git add" and/or "git commit -a")
  • Native git status --short:
$ git status --short
DU a
  • isomorphic-git statusMatrix:
    [ 'a', 0, 2, 3 ]

As you can see, they are the same too.

Also, if file a has been deleted in both branches, it is simply not a conflict and it is resolved automatically (removed from the index).

The MergeConflictError error now has this data: { filepaths: string[], bothModified: string[], deleteByUs: string[], deleteByTheirs: string[] }. Also, I added more branches in test-merge fixture repository.

From my experience, there is only one variant of conficts left: when the same file is added in different branches. But, I don't really understand how to solve this, because diff3 algorithm needs 3 files, so it needs a base file as well. The native git somehow knows how to build a base file from two files, or it uses some other diff algorithm. I had ideas to implement via diff-lines (already in the isomorphic-git dependencies), but I can't be sure how native this is. I'd have to change mergeDriver, since it won't return the base file in such cases.

And another difference I've noticed is the renaming. The native git can realize that a file has been renamed in one branch and in another, and will create a conlifct. (For example, I do mv a a2 in master and mv a a3 in develop. Native-git in merge will create a conflict, isomorphic-git will not. For isomorphic, it is deleting in two branches, adding a2 and a3

@DanilKazanov
Copy link
Contributor Author

I have no idea why tests are failing. First time I cloned isomorpgic-git, run npm i, ran the tests, they failed.

@jcubic
Copy link
Contributor

jcubic commented Oct 30, 2023

Thanks for the PR. Can you fix the lint errors?

I'm not sure if the tests work locally, there were some fixed done but I'm not sure if they work. The CI/CD failed on lint errors. If tests fail you can use this PR to fix the tests.

@DanilKazanov
Copy link
Contributor Author

I fixed link erorrs

@jcubic
Copy link
Contributor

jcubic commented Oct 30, 2023

now tests are failing, what exactly is .rejects in unit tests?

@DanilKazanov
Copy link
Contributor Author

DanilKazanov commented Oct 30, 2023

now tests are failing, what exactly is .rejects in unit tests?

https://jestjs.io/docs/asynchronous#asyncawait

If rejects broke tests for some reason, I can try revert my rejects refactor in test-merge file and do standart way, something like (error.code).toBe(Errors.MergeConflictError.code).

@DanilKazanov
Copy link
Contributor Author

the tests seemed to pass

@jcubic
Copy link
Contributor

jcubic commented Oct 31, 2023

AFAIK the proper API in jest to test if something throws is:

expect(() => call()).toThrow() see docs: https://jestjs.io/docs/expect#tothrowerror

@DanilKazanov
Copy link
Contributor Author

AFAIK the proper API in jest to test if something throws is:

expect(() => call()).toThrow() see docs: https://jestjs.io/docs/expect#tothrowerror

I did the same thing as the other tests in test-merge.js.

@jcubic
Copy link
Contributor

jcubic commented Oct 31, 2023

That's odd.

@jcubic jcubic merged commit 90ea0e3 into isomorphic-git:main Oct 31, 2023
4 checks passed
@jcubic
Copy link
Contributor

jcubic commented Oct 31, 2023

Ok looks good. One more thing can you document the feature of mergeConflict Error? I hope that the website can be built, we have this legacy Azure Pipeline that gives a lot of issues.

@isomorphic-git-bot
Copy link
Member

🎉 This PR is included in version 1.25.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@DanilKazanov DanilKazanov deleted the merge-deletions-support branch November 1, 2023 09:08
@DanilKazanov DanilKazanov restored the merge-deletions-support branch November 1, 2023 09:08
@DanilKazanov DanilKazanov deleted the merge-deletions-support branch November 1, 2023 09:08
@DanilKazanov
Copy link
Contributor Author

DanilKazanov commented Nov 1, 2023

I aldready tried to change docs: isomorphic-git/isomorphic-git.github.io#94, but it looks like website isn't being updated.
About the MergeConflictError, I should write docs here https://isomorphic-git.org/docs/en/merge#manually-resolving-merge-conflicts?

@jcubic
Copy link
Contributor

jcubic commented Nov 2, 2023

I've sent a message to the original author on LinkedIN, I hope he will reply there. Last time I sent him DM on twitter he didn't replied, and it seems that website is the only one thing that we don't have access to. And we don't know how to build the website.

@billiegoose
Copy link
Member

Hiya @jcubic - sorry I don't have a Twitter (er, I mean X) account anymore. I'll add you on LinkedIn

The docs are in the main repo (this one) in the website folder. The build process is here The Azure pipeline that publishes to npm also updates the main website. The nps website.publish script runs ./scripts/deploy-gh-pages.cjs which (in a rather meta fashion) uses isomorphic-git to push the subdirectory with the built website to the GitHub Pages repo.

To test the docs locally, you also run nps website (it uses the presences of the CI environment variable to either run the docusaurus dev server or build and push).

@jcubic
Copy link
Contributor

jcubic commented Nov 7, 2023

@billiegoose so normal release do the the update of website? We thought that the website was on isomorphic-git.github.io repo. What is this repo for then?

@billiegoose
Copy link
Member

@billiegoose so normal release do the the update of website?

Yup.

We thought that the website was on isomorphic-git.github.io repo. What is this repo for then?

Free hosting from GitHub Pages. It's not meant to be edited by humans, it's just a destination. See all the commits on the main branch?

https://github.com/isomorphic-git/isomorphic-git.github.io/commits/main

They're all made by isomorphic-git-bot. The commit messages match the commit messages from the main branch of this repo.

I guess the "Edit" links docusaurus makes are a little awkward since they point to that repo instead of the website folder of this repo.

@billiegoose
Copy link
Member

@jcubic #1840

modesty pushed a commit to modesty/isomorphic-git that referenced this pull request Apr 9, 2024
* init

* add fixture

* add tests

* refactor test errors

* add new bracnhes in merge fixture rep

* fix grammar

* add "delete poth" in fixture rep

* add "delete by both"

* fix test "delete by both"

* add data in MergeConflictError

* change docs

* docs: add @DanilKazanov as a contributor

* revert change docs for some reason

* fix-eslint

* Revert "refactor test errors"

This reverts commit 4e627f7.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants