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

Get openZip working on KotlinNative #1439

Merged
merged 3 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
22 changes: 22 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import org.jetbrains.dokka.gradle.DokkaTask
import org.jetbrains.kotlin.gradle.targets.js.nodejs.NodeJsRootExtension
import org.jetbrains.kotlin.gradle.targets.js.nodejs.NodeJsRootPlugin
import org.jetbrains.kotlin.gradle.targets.js.npm.tasks.KotlinNpmInstallTask
import org.jetbrains.kotlin.gradle.targets.js.testing.KotlinJsTest
import org.jetbrains.kotlin.gradle.targets.jvm.tasks.KotlinJvmTest
import org.jetbrains.kotlin.gradle.targets.native.tasks.KotlinNativeTest
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile

plugins {
Expand Down Expand Up @@ -246,3 +249,22 @@ plugins.withType<NodeJsRootPlugin> {
args += "--ignore-engines"
}
}

/**
* Set the `OKIO_ROOT` environment variable for tests to access it.
* https://publicobject.com/2023/04/16/read-a-project-file-in-a-kotlin-multiplatform-test/
*/
allprojects {
tasks.withType<KotlinJvmTest>().configureEach {
environment("OKIO_ROOT", rootDir)
}

tasks.withType<KotlinNativeTest>().configureEach {
environment("SIMCTL_CHILD_OKIO_ROOT", rootDir)
environment("OKIO_ROOT", rootDir)
}

tasks.withType<KotlinJsTest>().configureEach {
environment("OKIO_ROOT", rootDir.toString())
}
}
6 changes: 6 additions & 0 deletions okio-testing-support/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ kotlin {
}
}

val zlibMain by creating {
dependsOn(commonMain)
}

if (kmpJsEnabled) {
val jsMain by getting {
dependsOn(nonWasmMain)
Expand All @@ -36,6 +40,7 @@ kotlin {

val jvmMain by getting {
dependsOn(nonWasmMain)
dependsOn(zlibMain)
dependencies {
// On the JVM the kotlin-test library resolves to one of three implementations based on
// which testing framework is in use. JUnit is used downstream, but Gradle can't know that
Expand All @@ -48,6 +53,7 @@ kotlin {
createSourceSet("nativeMain", children = nativeTargets)
.also { nativeMain ->
nativeMain.dependsOn(nonWasmMain)
nativeMain.dependsOn(zlibMain)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import kotlin.random.Random
import kotlin.test.assertEquals
import kotlin.time.Duration
import okio.ByteString.Companion.toByteString
import okio.Path.Companion.toPath

fun Char.repeat(count: Int): String {
return toString().repeat(count)
Expand Down Expand Up @@ -90,3 +91,7 @@ expect val FileSystem.allowSymlinks: Boolean
expect val FileSystem.allowReadsWhileWriting: Boolean

expect var FileSystem.workingDirectory: Path

expect fun getEnv(name: String): String?

val okioRoot = getEnv("OKIO_ROOT")!!.toPath()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minimal zip is 22 bytes. Look at this chunker at a whopping 41!!!

Binary file not shown.
3 changes: 3 additions & 0 deletions okio-testing-support/src/jsMain/kotlin/okio/TestingJs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@ actual fun isBrowser(): Boolean {
}

actual fun isWasm() = false

actual fun getEnv(name: String): String? =
js("globalThis.process.env[name]") as String?
4 changes: 4 additions & 0 deletions okio-testing-support/src/jvmMain/kotlin/okio/TestingJvm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
*/
package okio

actual val SYSTEM_FILE_SYSTEM = FileSystem.SYSTEM

actual fun isBrowser() = false

actual fun isWasm() = false

actual fun getEnv(name: String): String? = System.getenv(name)
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@
*/
package okio

import kotlinx.cinterop.ExperimentalForeignApi
import kotlinx.cinterop.toKString
import platform.posix.getenv

actual val SYSTEM_FILE_SYSTEM = FileSystem.SYSTEM

actual fun isBrowser() = false

actual fun isWasm() = false

@OptIn(ExperimentalForeignApi::class)
actual fun getEnv(name: String): String? = getenv(name)?.toKString()
2 changes: 2 additions & 0 deletions okio-testing-support/src/wasmMain/kotlin/okio/TestingWasm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,5 @@ actual val FileSystem.allowReadsWhileWriting: Boolean
actual var FileSystem.workingDirectory: Path
get() = error("unexpected call")
set(_) = error("unexpected call")

actual fun getEnv(name: String): String? = error("unexpected call")
18 changes: 18 additions & 0 deletions okio-testing-support/src/zlibMain/kotlin/okio/TestingZlib.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright (C) 2024 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package okio

expect val SYSTEM_FILE_SYSTEM: FileSystem
3 changes: 3 additions & 0 deletions okio/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ kotlin {

val zlibTest by creating {
dependsOn(commonTest)
dependencies {
implementation(libs.test.assertk)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I will fix this wasm problem tomorrow (although I don't control when releases happen).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was playing with it. I ran into this:

* What went wrong:
Execution failed for task ':assertk:wasmWasiNodeTest'.
> command '/Users/jwilson/.gradle/nodejs/node-v21.0.0-v8-canary202309143a48826a08-darwin-arm64/bin/node' exited with errors (exit code: 1)

Totally unclear what I did wrong. Trying to borrow config from Okio wasn’t helping much!

}
}

val jvmMain by getting {
Expand Down
3 changes: 0 additions & 3 deletions okio/src/jvmMain/kotlin/okio/JvmOkio.kt
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,6 @@ fun Sink.hashingSink(digest: MessageDigest): HashingSink = HashingSink(this, dig
*/
fun Source.hashingSource(digest: MessageDigest): HashingSource = HashingSource(this, digest)

@Throws(IOException::class)
fun FileSystem.openZip(zipPath: Path): FileSystem = okio.internal.openZip(zipPath, this)

fun ClassLoader.asResourceFileSystem(): FileSystem = ResourceFileSystem(this, indexEagerly = true)

/**
Expand Down
2 changes: 1 addition & 1 deletion okio/src/nativeMain/kotlin/okio/internal/-ZlibNative.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ internal actual val DEFAULT_COMPRESSION: Int = platform.zlib.Z_DEFAULT_COMPRESSI

/**
* Roll our own date math because Kotlin doesn't include a built-in date math API, and the
* kotlinx.datetime library doesn't offer a stable at this time.
* kotlinx.datetime library doesn't offer a stable release at this time.
*
* Also, we don't necessarily want to take on that dependency for Okio.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
*/
package okio

import java.io.FileNotFoundException
import java.util.zip.Inflater
import okio.Path.Companion.toPath
import okio.internal.COMPRESSION_METHOD_STORED
import okio.internal.FixedLengthSource
Expand Down
26 changes: 26 additions & 0 deletions okio/src/zlibMain/kotlin/okio/ZlibOkio.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright (C) 2014 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

@file:JvmMultifileClass
@file:JvmName("Okio")

package okio

import kotlin.jvm.JvmMultifileClass
import kotlin.jvm.JvmName

@Throws(IOException::class)
fun FileSystem.openZip(zipPath: Path): FileSystem = okio.internal.openZip(zipPath, this)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should really make this public (with a little design tweak for OkHttp). We're long-past the rule of three on it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can do. Follow-up!

File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import okio.Path
import okio.Path.Companion.toPath
import okio.ZipFileSystem
import okio.buffer
import okio.use

private const val LOCAL_FILE_HEADER_SIGNATURE = 0x4034b50
private const val CENTRAL_FILE_HEADER_SIGNATURE = 0x2014b50
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,30 @@
*/
package okio

import assertk.assertThat
import assertk.assertions.containsExactly
import assertk.assertions.containsExactlyInAnyOrder
import assertk.assertions.isEmpty
import assertk.assertions.isEqualTo
import assertk.assertions.isFalse
import assertk.assertions.isGreaterThan
import assertk.assertions.isLessThan
import assertk.assertions.isNotNull
import assertk.assertions.isNull
import assertk.assertions.isTrue
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertFailsWith
import kotlinx.datetime.Instant
import okio.ByteString.Companion.decodeHex
import okio.ByteString.Companion.encodeUtf8
import okio.Path.Companion.toPath
import org.assertj.core.api.Assertions.assertThat
import org.junit.Before
import org.junit.Test

class ZipFileSystemTest {
private val fileSystem = FileSystem.SYSTEM
private var base = "../okio-testing-support/src/commonMain/resources/okio/zipfilesystem".toPath()
private val fileSystem = SYSTEM_FILE_SYSTEM
private var base = okioRoot / "okio-testing-support/src/commonMain/resources/okio/zipfilesystem"

@Before
@BeforeTest
fun setUp() {
fileSystem.createDirectory(base)
}
Expand Down Expand Up @@ -83,7 +93,7 @@ class ZipFileSystemTest {
.isEqualTo("Another file!")

assertThat(zipFileSystem.list("/".toPath()))
.hasSameElementsAs(listOf("/hello.txt".toPath(), "/directory".toPath()))
.containsExactlyInAnyOrder("/hello.txt".toPath(), "/directory".toPath())
assertThat(zipFileSystem.list("/directory".toPath()))
.containsExactly("/directory/subdirectory".toPath())
assertThat(zipFileSystem.list("/directory/subdirectory".toPath()))
Expand Down Expand Up @@ -112,7 +122,7 @@ class ZipFileSystemTest {
fun zipWithDeflate() {
val content = "Android\n".repeat(1000)
val zipPath = base / "zipWithDeflate.zip"
assertThat(fileSystem.metadata(zipPath).size).isLessThan(content.length.toLong())
assertThat(fileSystem.metadata(zipPath).size).isNotNull().isLessThan(content.length.toLong())
val zipFileSystem = fileSystem.openZip(zipPath)

assertThat(zipFileSystem.read("a.txt".toPath()) { readUtf8() })
Expand All @@ -138,7 +148,7 @@ class ZipFileSystemTest {
fun zipWithStore() {
val content = "Android\n".repeat(1000)
val zipPath = base / "zipWithStore.zip"
assertThat(fileSystem.metadata(zipPath).size).isGreaterThan(content.length.toLong())
assertThat(fileSystem.metadata(zipPath).size).isNotNull().isGreaterThan(content.length.toLong())
val zipFileSystem = fileSystem.openZip(zipPath)

assertThat(zipFileSystem.read("a.txt".toPath()) { readUtf8() })
Expand Down Expand Up @@ -524,8 +534,8 @@ class ZipFileSystemTest {
* `META-INF/kotlin-gradle-statistics.kotlin_module`.
*
* We used to crash on duplicates, but they are common in practice so now we prefer the last
* entry. This behavior is consistent with both [java.util.zip.ZipFile] and
* [java.nio.file.FileSystem].
* entry. This behavior is consistent with both `java.util.zip.ZipFile` and
* `java.nio.file.FileSystem`.
*
* ```
* echo "This is the first hello.txt" > hello.txt
Expand Down
Loading