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

Add a Path.toFile() converter on JVM #292

Open
jasonsparc opened this issue Apr 9, 2024 · 5 comments
Open

Add a Path.toFile() converter on JVM #292

jasonsparc opened this issue Apr 9, 2024 · 5 comments
Labels
enhancement fs integration An issue related to the integration with other libraries or platform-specific APIs

Comments

@jasonsparc
Copy link

…and perhaps also, a Path.toNioPath() on JVM.

Okio has a similar Path.toFile() method, along with a complementary Path.toNioPath().

While you could technically convert a Path into a string and then create a new java.io.File out of it, it feels like a waste when there's already a File object underneath that we could use. If kotlinx-io has a Path.toFile(), then Path.toNioPath() could also simply be implemented as path.toFile().toPath(), which would also return the same NIO Path instance every time.

Now of course, there's also the possibility that the underlying object implementing Path will not be a File anymore in the future. So the described performance win is irrelevant. However, I also think having such utility conversion methods would be convenient especially when trying to interoperate with JVM (or Android). It's useful for providing features currently present in the JVM but not in kotlinx-io, such as when implementing a custom FileSystem backed by JVM APIs, or when adding platform-dependent features. Also useful when you need a quick way to interop with some Java library that happens to require a File or NIO Path.

Parsing and manipulating paths is also one area where you're not touching the underlying file system to induce IO overhead, but when doing a lot of parsing and manipulation, you generally want that to be performant, so as to not waste precious CPU and battery life unnecessarily, especially when on Android.

@jasonsparc
Copy link
Author

I also would like to add that kotlinx-io's Path.toString() may not necessarily be compatible to File, for when the underlying object backing Path would change (in the future). So having a dedicated Path.toFile() conversion method would likely save us the pain of having to deal with any potential (future) incompatibilities.

@fzhinkin fzhinkin added enhancement fs integration An issue related to the integration with other libraries or platform-specific APIs labels Apr 9, 2024
@fzhinkin
Copy link
Collaborator

I totally agree that these functions would be quite handy, and for now everything should work fine.

However, thinking of a direction we're going evolve fs-API, there might be some issues.

Currently, kx-io's Path is always bound and interpreted as local file system's file path. Instead, a path should be bound to a FileSystem instance (and yes, currently there's only one implementation), the same way java.nio does it (it's not a final decision, but we're gravitating towards it).

In that case kotlinx.io.files.Path to java.io.File conversion should work only if a Path belongs to a local file system, in all other cases it should throw an exception.

For kotlinx.io.files.Path to java.nio.file.Path conversion we, potentially, can do better and try supporting it for other file system types (like, ZipFS). To make it work properly, a function responsible for conversion may need an instance of java.nio.file.FileSystem a result Java path should belong to. Or maybe it doesn't need an fs instance, I need to check how and if it should work (just as a reference, java.nio.file.Path::toFile fails for non-default filesystem).

@JakeWharton
Copy link

Instead, a path should be bound to a FileSystem instance (and yes, currently there's only one implementation), the same way java.nio does it (it's not a final decision, but we're gravitating towards it).

Huge +1 to this design. Using Okio's FS APIs I sorely miss the NIO design.

@jasonsparc
Copy link
Author

jasonsparc commented Apr 24, 2024

Using Okio's FS APIs I sorely miss the NIO design.

Perhaps kx-io's Path can be an interface, and then we could have a concrete implementation of it that wraps a File.

Say for example, the concrete implementation is a class named FilePath. Then with that, Path.toFile() can be implemented as an extension function that simply casts Path to FilePath in order to take the wrapped File. If the cast fails, then simply throw an exception.

Now, say Path could return the FileSystem it's associated with. When the cast to FilePath fails, we could additionally check if the associated FileSystem is the default or system one: if that additional check succeeds, then attempt to convert Path.toString() to File; throwing otherwise.


We could then mimic that design for other platforms, in case there's a much better platform-dependent object (similar to Java's File) that Path could wrap.

The downside to this is that, Path is now dependent on the underlying FileSystem that it's associated with: its string representation may now not be portable/convertible to other Path objects if the associated FileSystem is different. But I think that's actually a good thing.

Additionally, the default or system FileSystem may interpret Path differently depending on the platform and operating system – it's the "system" FileSystem after all. But that too could actually be a good thing, as it avoids issues such as square/okio#1460 from Okio. An OS may even interpret null characters in a Path as okay, yet some other OS may fail on such characters. Also, backslash (\) characters should be valid in directory and file names on Unix, in case there exist a set of files that has a really exotic reason (perhaps a supernatural reason) for having backslash characters in filenames – see for example, “.NET 8 breaking change: Backslash mapping in Unix file paths - .NET | Microsoft Learn

Suppose one wants a portable interpretation of Path, where it's guaranteed that a slash (/) is always interpreted as a file separator and all other characters are not (including backslash), then perhaps we could have a PortableFileSystem that may wrap a FileSystem object so that all Path objects are interpreted as described, but may throw if a character isn't valid according to the wrapped FileSystem object. That is, the only prominent difference between a PortableFileSystem(sfs: SystemFileSystem) and SystemFileSystem is that, PortableFileSystem may throw, while treating all file separators as slash characters, as if it's some kind of view/filter over the wrapped SystemFileSystem.

However, at this point, I'm getting off-topic, and I might even suggest a FileSystem.toPortable() as a convenient extension function returning a PortableFileSystem 😉 – also, I believe implementing that PortableFileSystem (perhaps accompanied by some PortablePath) would be very complicated and tricky, at the same level of complexity rivaling Okio's platform-independent Path objects. So maybe this last idea is not worth pursuing for now (at least).

@jasonsparc
Copy link
Author

jasonsparc commented Apr 24, 2024

P.S. That PortablePath could even have "X:/", "x:/" and "/" interpreted as all valid prefixes denoting an absolute Path. On Windows, the "/" may be interpreted to have the same drive letter as some base Path (which may default to the current working directory) that may have been passed to the PortableFileSystem constructor. On Unix, paths starting with "X:/" or "x:/" may be interpreted as pointing to a nonexistent location (and thus may throw), or perhaps simply stripped away when finally converting PortablePath to the wrapped FileSystem's Path object equivalent (the real equivalent of PortablePath).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement fs integration An issue related to the integration with other libraries or platform-specific APIs
Projects
None yet
Development

No branches or pull requests

3 participants