xwiki-commons-tool-xar-plugin - Various changes due to issues building/running on Windows. #1220
Conversation
Using the file class to build paths results in the file separator changes, but then we compare the paths using regex's which assume the Unix style file separator. There was also some encoding issues presumably due to UTF-8 not always being the default encoding on Windows.
test I actually assume the patterns will use the unix line returns, so I shouldn't have put that in the pattern.
jameswartell
changed the title
| @@ -19,6 +19,13 @@ | |||
| */ | |||
| package org.xwiki.tool.xar; | |||
|
|
|||
| import static org.twdata.maven.mojoexecutor.MojoExecutor.configuration; | |||
There was a problem hiding this comment.
Import static are supposed to be at the end according to XWiki codestyle (I also doubt this change in needed for what this PR is about, so better avoid it in general).
There was a problem hiding this comment.
The eclipse formatter moved it. I needed to add this to the end in eclipse. I'll see about adding that to the section on setting up formatting in Eclipse.
There was a problem hiding this comment.
You might want to take a look at the various Eclipse related configurations on https://github.com/xwiki/xwiki-commons/tree/master/xwiki-commons-tools/xwiki-commons-tool-verification-resources/src/main/resources (and in this specific case import eclipse.importorder). But in general you should avoid auto format.
|
A lot of formatting changes, which makes it hard to see the real changes. Better keep PR clean from such unneeded (and invalid for some of them) changes. |
...-tool-xar/xwiki-commons-tool-xar-plugin/src/test/java/org/xwiki/tool/xar/FormatMojoTest.java
Outdated
Show resolved
Hide resolved
...-tool-xar/xwiki-commons-tool-xar-plugin/src/test/java/org/xwiki/tool/xar/FormatMojoTest.java
Outdated
Show resolved
Hide resolved
| InputStream is = Thread.currentThread().getContextClassLoader().getResourceAsStream("XWikiSyntaxLinks.it.xml"); | ||
| String expectedContent = IOUtils.toString(is, "UTF-8"); | ||
| // github clients may automatically convert the file to use windows line returns. |
There was a problem hiding this comment.
Hmm, might be better to deal with this kind of problems with https://github.com/xwiki/xwiki-commons/blob/master/.gitattributes.
There was a problem hiding this comment.
I dug in on this. It seemed intuitively like that might be a change with larger impacts.
I tried setting the line.seperator on the format object of the writer to the native to see that would be an easy way to get them to match, but it did not. It is because the comment nodes of the parsed version have new lines in them which don't match the windows format.
So I looked up if the SAX parser is misbehaving. It is not. The XML spec says: "To simplify the tasks of applications, the XML processor MUST behave as if it normalized all line breaks in external parsed entities (including the document entity) on input, before parsing, by translating all of the following to a single #xA character:"
https://www.w3.org/TR/xml11/#sec-line-ends
That convinced me. I do think simply making all xml conform to using Unix style line returns is the right thing to do. The parser is right to strip out carriage returns. I believe using the EOL=LF in the .gitattributes will get clients to forcibly normalize this both locally and when checking in.
...-tool-xar/xwiki-commons-tool-xar-plugin/src/test/java/org/xwiki/tool/xar/FormatMojoTest.java
Outdated
Show resolved
Hide resolved
.gitAttributes to force "/n" to be the line separator for xml on check-in and locally. Small optimization so we don't replace file.seperator when we don't even end up comparing it to a pattern.
I set up eclipse with xwiki formatter or it is happening by default. I didn't manually do those at all. I don't care about them either. |
|
Build of the xwiki-workflow-publication is now failing for me with this error when I try to use this as a snapshot.
|
Using the file class to build paths results in the file separator changes, but then we compare the paths using regex's which assume the Unix style file separator. There was also some encoding issues presumably due to UTF-8 not always being the default encoding on Windows.
Jira URL
No Jira
Changes
Description
before using a regex to look at a path on systems where "/" is not the File.separator replace the native with the Unix style file.separator.
Set the encoding to UTF-8 on every write/read in FormatMojoTest.
All the white space changes happened automatically using the xwiki formatter.
Clarifications
I discovered these issues trying to build the publication workflow application. The XAR XML files wouldn't validate because though some were set as visible technical pages, they didn't match the pattern given. I think we're thinking of these patterns to match paths like URL paths rather File paths, but the trouble comes in when we use the File class. I think this was the easiest way to fix it.
Then tests were failing due to encoding issues. I did add some debugging code to the tests to help pinpoint exactly where the first difference occurs on failure, because it was difficult to figure out what was going on at first.
There were also System.lineSeperator() differences in the test. I think my GitHub client may convert text files on download to the native (Windows), but this results in String differences on a compare when you read the expected value from a file, since no such conversion happens when we write the xml in memory.
Executed Tests
The tests were already failing, and it's a bit of a challenge to create the problem artificially. It would be good if builds were sometimes tested on Windows to find these issues.
Expected merging strategy