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

Improve file handling on Windows #3764

Merged
merged 12 commits into from
Oct 17, 2024

Conversation

sake92
Copy link
Contributor

@sake92 sake92 commented Oct 17, 2024

Changes:

  • replace new FileInputStream with Files.newInputStream
  • replace new FileOutputStream with Files.newOutputStream

Motivation:
#1939

https://bugs.openjdk.org/browse/JDK-6357433 (in a comment) says:

For jdk7, the workaround is to use new file system API. So instead of new FileInputStream(f) and new FileOutputStream(f), use f.toPath().newInputStream() and f.toPath().newOutputStream()

So seems like new NIO API does use, FILE_SHARE_DELETE, which is needed to delete files in unix-style.

@lihaoyi
Copy link
Member

lihaoyi commented Oct 17, 2024

Thanks @sake92! Might need to send a PR upstream to OS-Lib as well https://github.com/com-lihaoyi/os-lib/blob/f1bf225f8b110c4a561465717d8b13efcd9ee6d3/os/src/ZipOps.scala#L75

@lihaoyi
Copy link
Member

lihaoyi commented Oct 17, 2024

We also use java.util.jar.JarOutputStream in some places in Mill, I'm guessing that API predates NIO and may need to be updated as well

@sake92
Copy link
Contributor Author

sake92 commented Oct 17, 2024

Thanks @sake92! Might need to send a PR upstream to OS-Lib as well https://github.com/com-lihaoyi/os-lib/blob/f1bf225f8b110c4a561465717d8b13efcd9ee6d3/os/src/ZipOps.scala#L75

Sure thing, I'll update it there too.


We also use java.util.jar.JarOutputStream in some places in Mill, I'm guessing that API predates NIO and may need to be updated as well

JarOutputStream is not OS-dependent like the java.io input/output streams. I might be wrong tho.
https://docs.oracle.com/javase/8/docs/api///?java/util/jar/JarOutputStream.html says:

The JarOutputStream class is used to write the contents of a JAR file to any output stream. It extends the class java.util.zip.ZipOutputStream with support for writing an optional Manifest entry. The Manifest can be used to specify meta-information about the JAR file and its entries.

@lihaoyi lihaoyi merged commit 59e1bda into com-lihaoyi:main Oct 17, 2024
24 checks passed
lihaoyi pushed a commit to com-lihaoyi/os-lib that referenced this pull request Oct 17, 2024
@lefou lefou added this to the 0.12.0 milestone Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants