Skip to content

Improve file handling on Windows#3764

Merged
lihaoyi merged 12 commits intocom-lihaoyi:mainfrom
sake92:improve-windows-file-handling
Oct 17, 2024
Merged

Improve file handling on Windows#3764
lihaoyi merged 12 commits intocom-lihaoyi:mainfrom
sake92:improve-windows-file-handling

Conversation

@sake92
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

@sake92 sake92 mentioned this pull request Oct 17, 2024
Merged
@lihaoyi lihaoyi merged commit 59e1bda into com-lihaoyi:main Oct 17, 2024
lihaoyi pushed a commit to com-lihaoyi/os-lib that referenced this pull request Oct 17, 2024
@sake92
Use Files.newOutputStream instead of new java.io.FileOutputStream (#323)
af875ae 
As in com-lihaoyi/mill#3764
@lefou lefou added this to the 0.12.0 milestone Oct 17, 2024
@alexarchambault alexarchambault mentioned this pull request Oct 25, 2024
Merged
@alexarchambault alexarchambault mentioned this pull request Jan 10, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.12.0

Development

Successfully merging this pull request may close these issues.

3 participants

@sake92 @lihaoyi @lefou