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

Merge with conflict leads to "theirs" work to be lost (erased by "mine") #130

Closed
jbsarrodie opened this issue Mar 25, 2020 · 7 comments
Closed
Assignees
Labels

Comments

@jbsarrodie
Copy link
Member

Logging this for my own investigation

I very recently had two colleagues who faced a big issue with Archi 4.6 and coArchi 0.5.3.

In both cases, the publish their changes while their last publish had been done 1 or 2 months before. A one can imagine, they had some conflicts to solve. In both case they choose "theirs" to be sure to keep everything done by the rest of the team.

The result is a commit history not showing the usual merge commit labeled "Merge branch 'master' of ...", but instead it shows their lastest "real" commit (the one with a manually entered commit message) as a merge commit.

The impact is that in fact despite having chosen "theirs" in teh conflict solver, the model now contains their own (old) version and not a merge of their version and the current one on the git server.

I don't understand how this could happen, so I'm investigating....

@jbsarrodie jbsarrodie added the bug label Mar 25, 2020
@jbsarrodie jbsarrodie self-assigned this Mar 25, 2020
@jbsarrodie
Copy link
Member Author

jbsarrodie commented Mar 26, 2020

I can now reproduce the bug:

Create a new model with only one view inside one sub-folder, and another folder. Then publish it (using this one https://collaboration.archimatetool.com/jbsarrodie/Issue_130):
image

From now on, we have two users (A and B) who work on this same version of the model)

User A updates moves the sub-folder "Some Folder" into "Folder 2", and create new views (but doesn't change the existing one) and publish it:
image

User B does a lot of changes to the original view, and add a new folder and a new view. Then publish:
image

A conflict is raised:
image

This happens because there have been so many changes to the view that git is not able to see this as a change on the same file (updated and moved), so git thinks it is a new view and thus thinks User A deleted the original one. Under the hood, I think git has no issue tracking the folder.xml file, so it is moved without any issue. Leading (if I choose "Mine") after the merge to two "Some Folder" folders, one with the original version of the view and folder.xml, the other with the new version of the view and no folder.xml.

Let's continue (keep "Mine"). As predicted, we have an error message about the missing folder.xml:
image

Let's click on "OK".

Everything seems normal and model is still opened in Archi. If we look at the change history, it clearly shows that the merge did not occured:
image

But now, let's try to publish again (that's what most user would do in this case). I'm asked to commit but there's no reasons as nothing changed. Let's enter a commit message and continue...
image

Publication is now done:
image

We could think everything is back to normal, but it is not. This last commit created is in fact a merge commit:
image

This has a nasty side effect of publishing the local version (not the one resulting of the previous, failed, merge), and thus erasing other people's work (if you look at previous screenshot, you'll see that "Folder 3" -created by User A- is missing).

@jbsarrodie
Copy link
Member Author

I can reproduce this bug also with coArchi 0.6.0. The only slight difference is that after the error message about folder.xml, the model is closed. But other than that, impact is the same at the end: User A loses all his/her work.

@jbsarrodie
Copy link
Member Author

jbsarrodie commented Mar 31, 2020

@Phillipus I've found the origin of this bug: RefreshModelAction was not reseting the repository to local state in this very specific case: we have had a conflict, conflict has been solved, merge is ok from a git point of view, but resulting model is corrupted and cannot be loaded by GraficoModelImporter.

I've commited a fix in branch issue130. Can you have a look and tell me if you think there is any side effect please?

@Phillipus
Copy link
Member

Your change calls loader.loadModel() in both cases in the if/else clause. Would it make sense to only call it once (as in the existing code) but surround that with a try/catch block and throw the ex exception?

@jbsarrodie
Copy link
Member Author

Would it make sense to only call it once (as in the existing code) but surround that with a try/catch block and throw the ex exception?

The issue if that I can't call resetToLocal() outside the inner block

@Phillipus
Copy link
Member

The issue if that I can't call resetToLocal() outside the inner block

OK, I see that.

And should it re-throw the exception? Or quietly absorb it and return?

@jbsarrodie
Copy link
Member Author

And should it re-throw the exception? Or quietly absorb it and return?

We have to notify the user that something wrong happened, so forwarding the exception seems fine.

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

No branches or pull requests

2 participants