-
Notifications
You must be signed in to change notification settings - Fork 575
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 path normalization for Windows OS #3215
base: master
Are you sure you want to change the base?
Fix path normalization for Windows OS #3215
Conversation
@DimaVilda Thanks for the PR. If existing tests are not affected, I'm happy to merge your fix. It looks like some tests on failing on Windows right now. |
hi @oldergod ! Lemme check it asap and in case I was wrong will fix it |
hi again there @oldergod private val fs = FakeFileSystem().apply {
if (Path.DIRECTORY_SEPARATOR == "\\") emulateWindows() else emulateUnix()
} the in emptyPackagedProtoMessage() test we get NPE: loader.initRoots(
sourcePath = listOf(Location.get(fs.workingDirectory.toString())), --> NPE
protoPath = listOf(Location.get(fs.workingDirectory.toString())),
) Cause: This issue comes from: private fun Buffer.startsWithVolumeLetterAndColon(slash: ByteString): Boolean {
if (slash != BACKSLASH) return false // This causes forward slashes to fail
...
} While "F:\".toPath() works correctly, "F:/".toPath() doesn't, even though both formats are valid on Windows. Since Windows accepts both slash types interchangeably, the path parsing should ideally handle both formats consistently. |
It doesn't look like it's part of the spec though that Windows supports or not forward slashes: https://retrocomputing.stackexchange.com/a/28348 @swankjesse did you look into it when making Okio's FileSystem? |
hey hi @oldergod and @swankjesse ! |
Issue:
The issue goes from kafka-ui thread kafbat/kafka-ui#261 which uses 'Location.get(...)' function from your library.
Example:
If in Location.get(...) we pass:
1st param base - C:\Users\username\IdeaProjects\kafka-ui\api\target\test-classes\protobuf-serde
2nd param path - language\language.proto
the result is - C:\Users\username\IdeaProjects\kafka-ui\api\target\test-classes*protobuf-serde/language\language.proto*
And then Linker.link(...) throws NPE:
Solution: