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

[Bug] Empty commits #64

Open
jbsarrodie opened this issue Oct 5, 2018 · 23 comments
Open

[Bug] Empty commits #64

jbsarrodie opened this issue Oct 5, 2018 · 23 comments
Labels

Comments

@jbsarrodie
Copy link
Member

Summary of our email discussion

I (and colleagues) faced a strange bug sometimes: we're asked to create a commit but in fact there are no changes and this lead to an empty commit. I don't know how to reproduce it on demand, but I can share a (private) repo that has such empty commit on its history.

Phil's answer
This can happen if the internal (Archi Ecore) model version number is updated [...] Or it might be a line-endings problem (CRLF vs LF). Can you check the commit in SmartGit and see if anything changed? But certainly git is seeing some difference otherwise it won't commit.

This was certainly not the internal model version number as it happens frequently enough. I first though it was line-ending problems but this also happens in a windows only context, so I'm not sure it's the root cause.

Some screenshots of empty commit seen through SmartGit have been exchanged through emails

It seems that jGit implementation is able to commit "nothing".

@jbsarrodie jbsarrodie added the bug label Oct 5, 2018
@Phillipus
Copy link
Member

Phillipus commented Oct 5, 2018

I have a case where I pulled from the "Let's Test" repo, it merged but the affected files were still in a "different" state needing to be committed again.

The line endings were the same and I can't see any difference in the files. And even SmartGit thinks there is a difference between the files but I can't see it.

@Phillipus
Copy link
Member

Phillipus commented Oct 5, 2018

I did a "Refresh" and it pulled and merged and it was all OK.

I then did another "Refresh" and this time some "changed" files were created by this action:

getRepository().exportModelToGraficoFiles();

So GraficoModelExporter#exportModel() is called at this point.

The files are exactly the same, including line endings.

@jbsarrodie
Copy link
Member Author

Interresting. If even SmartGit thinks there are some changes then there might be some but hidden. I'll try to reproduce and then compare files with an hex editor...

@Phillipus
Copy link
Member

This is really strange, I think it's line endings. Yours is LF (Linux) mine is CRLF (Windows). When I changed the line endings in the files manually to LF this solved the problem. So I discarded all files with the problem, and then the next time I did a Refresh it did not happen.

@Phillipus
Copy link
Member

Phillipus commented Oct 6, 2018

Repeating the same test case, some times not all files are affected. Some times it is 6 files, then the next time 3.

But it's definitely to do with line endings or encoding.

@Phillipus
Copy link
Member

  1. Refresh action. This pulls from Repo and a merge commit is automatically done. All files in SmartGit are OK.
  2. Another Refresh action. This exports again to grafico files. Now we see files with invisible diffs in SmartGit.
  3. Discard the changes on these files.
  4. Another Refresh action. All is OK from this point on, no invisible diffs.

@Phillipus
Copy link
Member

If I "Stage" the different files in SmartGit they disappear from the diff list and everything is OK from here on, even if I do another Refresh pull action.

@Phillipus
Copy link
Member

Phillipus commented Oct 6, 2018

There is an option to save the EMF Resource xml file with specific line endings.

Because the xml files that JB committed have UNIX "LF" line endings I added this to GraficoModelExporter:

resource.getDefaultSaveOptions().put(Resource.OPTION_LINE_DELIMITER, "\n");

And then after a Refresh there were no different files, so this shows that line endings is the issue.

So we should add the above line but also agree on what line endings to use. I propose Windows CRLF as it is more popular. LF might be best?

If we do this, there will initially be diffs until everyone is using this version and all commits use the same line endings.

@Phillipus
Copy link
Member

Alternatively, after each grafico export we could stage all files (Git "Add"). Staging files with these line ending diffs removes the line ending problem.

@Phillipus
Copy link
Member

I first though it was line-ending problems but this also happens in a windows only context

Hmmm. Then I don't know.

@Phillipus
Copy link
Member

I'm beginning to suspect that this can happen after a merge.

See https://bugs.eclipse.org/bugs/show_bug.cgi?id=499615

@Phillipus
Copy link
Member

I've updated JGit to 4.11 and I still see this problem and it seems to happen after a merge. Changing line endings manually of the affected files to match that in the git index clears the problem.

I think I have the solution, though. We need to ensure that line endings are the same for all users. And I suspect that JGit might use UNIX line endings internally.

At the moment all new repos have this set in the config file:

[core]
autocrlf = true

And GraficoModelExporter I think writes XML files with native line endings.

I tested this setting autocrlf to "input" and writing all XML files with UNIX line endings and I no longer got the problem.

@Phillipus
Copy link
Member

Setting autocrlf to "input" and writing all XML files with UNIX line endings seems to fix the problem.

I've made it so that new repos will have this setting, but existing ones will have to manually change the local config file to:

[core]
autocrlf = input

The only issue is that if you change this for an existing repo which previously used CRLF you'll get an initial diff of line endings as before. But aborting the changes and trying again seems to clear it.

@Phillipus
Copy link
Member

However, this change will affect existing users who either won't have the latest version or don't change their config files.

The only way round this I can see is to also do a "Stage" (git Add) at the time of the grafico export to clear the files.

@jbsarrodie
Copy link
Member Author

The only way round this I can see is to also do a "Stage" (git Add) at the time of the grafico export to clear the files.

Do you mean that for the moment we can commit without doing the equivalent of a git add ?

Is this git add enough to remove the issue (from your tests) ?

@Phillipus
Copy link
Member

I think there are a number of issues going on here.

If everyone was using the same line endings (UNIX) that would help. That's taken care of. But might not solve it on existing repos and commits until everyone is using the latest version.

Staging all files after a grafico export eliminates files that have different line endings if it's CRLF and LF.

So we need to do all of the above.

We'll just have to try it.

@stigbd
Copy link

stigbd commented Feb 7, 2019

Working in a hetergenous os-environment (osx, windows, linux, and even bash-on-ubuntu-on-windows) we are badly hit by this bug. My git cli says "nothing to commit, working tree clean", while Archi says "There are changes to commit". Reading through the comments above, I'm confident you will come up with a solution.
I will gladly contribute by testing any alpha-versions of the plugin, if need be.

@Phillipus
Copy link
Member

@stigbd
Copy link

stigbd commented Feb 7, 2019

My bad: I forgot to mention that I have been using the 0.4.0_alpha since 14th of january. I am still seeing this behaviour. On the other hand: my collaborators are not on 0.4.0_alpha. I guess that explains it.

Is there a recommended "Steps to Reproduce the Behaviour" related to this bug?

@jbsarrodie
Copy link
Member Author

I'm not sure that people using different versions of the plugin can cause this, but are you sure that everyone uses a version of Archi either below 4.3 or above 4.3, but not a mix. This is needed because internal XML format used by Archi change with 4.3, and Archi updates the model on load, leading to changes even if the user did nothing.

One simple way to see if this issue is still there: use a Git tool to check commit contents when you are forced to commit. If the bug is still there the commit will be empty (ie. no changed files). If the issue is related to people using different versions of Archi you'll see model.xml in this commit.

@stigbd
Copy link

stigbd commented Feb 7, 2019

Ok. I think we may be using a mix. I'll see to it that we are all above 4.3.

For inspecting the commit content I guess git diff COMMIT~ COMMIT will do it. Or is there another tool you recommend?

@jbsarrodie
Copy link
Member Author

Or is there another tool you recommend?

The simpliest option is to see commit content on your web-based git platform (GitHub, GitLab, BitBucket...)

On a workstation, I personaly often use SmartGit which is visual and easy to use (but not free for professional use after a try period). FWIW, I'm still looking for a good and opensource visual Git tool.

@Phillipus
Copy link
Member

SmartGIt is the best

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

3 participants