cecom/pomutils

-Xignore-all-space not respected

borisbrodski opened this issue · 15 comments

First of all, thank you for this awesome project!

We are using it for a quite a bit. It works flowerless with the exception of just one issue:

If we merge with

$ git merge -Xignore-all-space other-branch

the pom.xml files, that get processed my the pomutils merge engine get conflicts,
if they contain differences in space/tab indentation.

Put it simple, the -Xignore-all-space git merge flag doesn't get respected for the files, on which pomutils was involved.

  • Are there a trick to avoid it?

I'm looking into implementing an additional switch, like --expandtab 4, that will convert all tabs into 4 spaces in both files before exiting.

  • Do you have better suggestions?
  • Are you interested in a push request?
cecom commented

Hi,

i had a quick look and it is not that easy. We only adjust the pom version value element, regardless of the position on the line.

Example:
branch1: <version>1.0</version>
branch2: ....<version>2.0</version> (where dots are spaces)

If you do a branch 2 --> branch1 merge, the result (before the real merge happens) will be:

  • if "our" strategie is used <version>1.0</version>
  • if "their" strategie is used <version>2.0</version>

If you do a branch 1 --> branch2 merge, the result (before the real merge happens) will be:

  • if "our" strategie is used ....<version>2.0</version> (where dots are spaces)
  • if "their" strategie is used ....<version>1.0</version> (where dots are spaces)

After we adjusted the value, we call the git merge driver git merge-file ... and this does not have any options like ignore-all-space.

I need to think about it a little bit more, if there is a solution....

Thank you for the quick response. I also couldn't find a simple solution to the missing --ignore-all-space switch. One complicated solution may be:

  • create a new temporary directory and git init there
  • Add base-file to a repo and commit it
  • Create branch1, add current-file and commit it
  • Create branch2 on the base commit, add other-file and commit it
  • Issue git merge branch1 -Xignore-all-space
  • Grab the merged file
  • Remove temporery directory

But if you would consider a simpler problem, like just confused tabs and spaces, the solution may be also much simpler. For example:

  • branch1: \t<version>1.0</version> (where \t is the tab)
  • branch2: ....<version>2.0</version> (where dots are spaces)

you may just come away with the simple TAB -> SPACE replacement. I already tested it by adding

  • --expandtab=<count-of-spaces>

option to the pomutils, that just replaces all TABs in

  • current
  • base
  • other

by 4 spaces (in my example). This solves my current problem. Of course, if TABs doesn't exactly match SPACES, this will fail as you have described.

cecom commented

The problem on your first solution is, that would happen for all poms. So if you have a multi module project with 100 of poms that could take a while, because it would be done for every single pom.

Your Second solution does not work, because i dont have access to the real pom file. I manipulate the file via the maven-version-plugin classes and they dont provide any access to the real file.

Update: For your second solution, i only want to update the line where the version "problem" exists. But you meant to replace all \t in the complete file, correct?

Yes, I replace all \t into spaces. This already works here. Do you find it wrong to replace all \t ?

And yes, the performance will be definitely the problem. On the other hand, solving conflicts manually is always longer, that any automated procedure. Also I would not use this first complex solution by default.

cecom commented

If there is no solution on your side (get a xml formatter for every developer so the pom.xml file is always correct formatted), then the way i would go would be:

  • add a property ala "preFormatXml" which defaults to "false"
  • if true, before we do anyhing with the pom, we format the complete pom with the formatter, so everything is alined after a template
  • after that we do our merge stuff.

That would solve \t and space issues. But im a litte bit unsure. There could also be some formatting changes in your pom, which you dont want...

Thank you!

This is working great so far. Instead of full format, I am just replacing tabs with spaces.

cecom commented

so you changed your developers format rules and the problem does not exist anymore? So we can close this issue?

No, I extended pomutils to simply replace all tabs with spaces before passing files to the git merge.

cecom commented

Ok, than send me a pull request and i will have a look. Plz add a test too. Have a look at my other tests, its easy to add them.

cecom commented

@borisbrodski are you working on it anymore? Or can i close this ticket?

Thank you for reminding me of this issue. I will add some tests and create a pull request.

Please, check the pull request :)

cecom commented

Hi Boris,

i had a look. What do you think, if i would change it to "Tabify" and "Untabify"? So we can get both worlds? The one who uses TABs can use the tabify and one who uses Spaces uses the Untabify. I would take your Code and change it a little bit :-)

Hi Cenom,

sure, go for the change. But please, accept pull request first and then make additional commit with your change. It allow me to merge back your change quickly (in my branch) and it will also preserve the authorship of the feature ;)

Thank you!

cecom commented

For sure. I will take your branch and work on it.