Navigation Menu

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

Fix chunk errors on Windows #2706 #2728

Merged
merged 2 commits into from Jun 16, 2017

Conversation

robander
Copy link
Member

Signed-off-by: Robert D Anderson robander@us.ibm.com

Addresses the issue reported in #2706.

Tracked down to an issue with Windows only: our isAbsolutePath method fails with some path styles, on Windows only. I haven't (yet) fixed that issue. Instead, the chunking method that used isAbsolutePath was updated so that it deals with files, and relies on the native File.isAbsolute() method. (Previously, URIs were converted to Strings, which were converted back to files.)

As an added benefit, this resolves long-standing issues with the gradlew test command that I hadn't taken time to figure out -- 14 of the existing chunk test cases failed with every Windows test, but worked on Mac / Linux. With this update, the gradlew test run is now clean on Windows.

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that changes existing functionality)

Checklist

Signed-off-by: Robert D Anderson <robander@us.ibm.com>
@robander robander added bug preprocess/chunking platform/windows Platform-specific issue that only appears on Windows labels Jun 15, 2017
@robander robander requested a review from jelovirt June 15, 2017 21:21
@robander robander added this to the 2.5.1 milestone Jun 15, 2017
Don't use URI path part as File constructor argument, as this will not work on Windows.
@@ -128,7 +128,7 @@ private void processChunk(final Element topicref, final URI outputFile) {
outputFileName = currentFile.resolve(hrefValue.getFragment() + FILE_EXTENSION_DITA);
} else {
// Find the first topic id in target file if any.
final String firstTopic = getFirstTopicId(currentFile.resolve(hrefValue).getPath());
final String firstTopic = getFirstTopicId(new File(currentFile.resolve(hrefValue).getPath()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work, because getPath() with return the path of the file in URI format.

@jelovirt jelovirt added the platform-dependent Label is obsolete, use specific platform label instead label Jun 16, 2017
@jelovirt
Copy link
Member

Fixed the issues I found.

@jelovirt jelovirt merged commit ea65e45 into dita-ot:hotfix/2.5.1 Jun 16, 2017
@robander robander deleted the hotfix/chunkerr branch June 16, 2017 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug platform/windows Platform-specific issue that only appears on Windows platform-dependent Label is obsolete, use specific platform label instead preprocess/chunking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants