Fix Windows build issues #121
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
And fix failing tests on Windows. This changes all of labkit-ui to eschew usage of the SingletonContext class in favor of passing the context between components as appropriate, creating new contexts in relevant situations, and disposing of contexts when they are finished being used in all cases except for demo code. It also escapes backslashes of Windows paths hardcoded into macros, to account for ImageJ macro using backslash as an escape character. Some of these changes change existing public API, so this commit increments the major/minor digit of the development version.
maarzt
approved these changes
Oct 17, 2024
| .replace('\'', '"') | ||
| .replace("INPUT_TIF", inputImage) | ||
| .replace("SEGMENTER_FILE", blobsModel) | ||
| .replace("OUTPUT_TIF", outputImage.getAbsolutePath().replaceAll("\\\\", "\\\\\\\\")); |
There was a problem hiding this comment.
I wonder if using apache-commons StringEscapteUtils.escapeJava(text) would be the even more correct solution here. But I cannot test it, and also wonder why the code replaces double backslashes. Windows would use single backslashes in paths right?
Suggested change
| .replace("OUTPUT_TIF", outputImage.getAbsolutePath().replaceAll("\\\\", "\\\\\\\\")); | |
| .replace("OUTPUT_TIF", StringEscapeUtils.escapeJava(outputImage.getAbsolutePath())); |
Anyway, this is the test code, not the production code. I'm happy if it runs reliably on Windows, Linux and Mac.
There was a problem hiding this comment.
wonder why the code replaces double backslashes.
Yeah, it's tricky. I should have written a comment about it, sorry... I have now pushed a commit that does so. ctrueden/labkit-ui@b82ab70
And explain why it needs to be so many backslashes. XD
This is probably the correct way to convert a string into a IJM string literal. I unfortunately couldn't find documentation how IJM escapes strings. Using the method is necessary because paths on window use backslashes and the need to be escaped, the tests fails otherwise.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch fixes issues with the unit tests on Windows as well as the recent ImgLib2 v7 update. It upgrades to the newly released
spim_dataversion 2.3.4, and also tidies up the SciJava context logic across the codebase to avoid theSingletonContextin favor of contexts with managed lifecycles (i.e. ensuringdispose()gets called at conclusion).It also escapes any backslashes hardcoded into macros (notably: Windows file paths), to account for ImageJ macro using backslash as an escape character.
With these changes, all tests on Windows now pass, and many cases of delayed test termination are now resolved with the explicit context management. I was disappointed to see that end-to-end test runtime from the CLI is not improved on Linux (03:59 min before, 04:10 min after, sample size 1).
Some of these changes modify existing public API, so this commit increments the major/minor digit of the development version.