From f96f752f3a6e2b18e1bbd43b0ae93601bc258991 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Thu, 6 Mar 2025 09:35:45 -0800 Subject: [PATCH 01/11] feat(amazonq): launch lsp from resolved artifacts and allow user to override via config --- .../services/amazonq/lsp/AmazonQLspService.kt | 7 +++- .../jetbrains/settings/LspSettings.kt | 11 ++--- .../jetbrains/settings/LspSettingsTest.kt | 40 ++++++++++++------- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt index 501d93e78e8..d996b6aec0f 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt @@ -43,6 +43,7 @@ import software.aws.toolkits.core.utils.getLogger import software.aws.toolkits.core.utils.info import software.aws.toolkits.core.utils.warn import software.aws.toolkits.jetbrains.isDeveloperMode +import software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts.ArtifactManager import software.aws.toolkits.jetbrains.services.amazonq.lsp.auth.DefaultAuthCredentialsService import software.aws.toolkits.jetbrains.services.amazonq.lsp.encryption.JwtEncryptionManager import software.aws.toolkits.jetbrains.services.amazonq.lsp.model.createExtendedClientMetadata @@ -50,6 +51,7 @@ import software.aws.toolkits.jetbrains.services.amazonq.lsp.textdocument.TextDoc import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.WorkspaceFolderUtil.createWorkspaceFolders import software.aws.toolkits.jetbrains.services.amazonq.lsp.workspace.WorkspaceServiceHandler import software.aws.toolkits.jetbrains.services.telemetry.ClientMetadata +import software.aws.toolkits.jetbrains.settings.LspSettings import java.io.IOException import java.io.OutputStreamWriter import java.io.PipedInputStream @@ -236,8 +238,11 @@ private class AmazonQServerInstance(private val project: Project, private val cs } init { + // will cause slow service init, but maybe fine for now. will not block UI since fetch/extract will be under background progress + val artifact = runBlocking { ArtifactManager(project, manifestRange = null).fetchArtifact() }.toAbsolutePath() val cmd = GeneralCommandLine( - "amazon-q-lsp", + artifact.resolve("node").toString(), + LspSettings.getInstance().getArtifactPath() ?: artifact.resolve("aws-lsp-codewhisperer.js").toString(), "--stdio", "--set-credentials-encryption-key", ) diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/settings/LspSettings.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/settings/LspSettings.kt index 8ac9d527651..61060774e79 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/settings/LspSettings.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/settings/LspSettings.kt @@ -10,7 +10,7 @@ import com.intellij.openapi.components.Service import com.intellij.openapi.components.State import com.intellij.openapi.components.Storage import com.intellij.openapi.components.service -import com.intellij.util.xmlb.annotations.Property +import com.intellij.util.text.nullize @Service @State(name = "lspSettings", storages = [Storage("aws.xml", roamingType = RoamingType.DISABLED)]) @@ -26,11 +26,7 @@ class LspSettings : PersistentStateComponent { fun getArtifactPath() = state.artifactPath fun setArtifactPath(artifactPath: String?) { - if (artifactPath == null) { - state.artifactPath = "" - } else { - state.artifactPath = artifactPath - } + state.artifactPath = artifactPath.nullize(nullizeSpaces = true) } companion object { @@ -39,6 +35,5 @@ class LspSettings : PersistentStateComponent { } class LspConfiguration : BaseState() { - @get:Property - var artifactPath: String = "" + var artifactPath by string() } diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/settings/LspSettingsTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/settings/LspSettingsTest.kt index 3d1e240beb7..6b9d425ba3d 100644 --- a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/settings/LspSettingsTest.kt +++ b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/settings/LspSettingsTest.kt @@ -20,21 +20,27 @@ class LspSettingsTest { } @Test - fun `artifact path is empty by default`() { - assertThat(lspSettings.getArtifactPath()).isEmpty() + fun `artifact path is null by default`() { + assertThat(lspSettings.getArtifactPath()).isNull() } @Test fun `artifact path can be set`() { lspSettings.setArtifactPath("test\\lsp.js") - assertThat(lspSettings.getArtifactPath()).isNotEmpty() - assertThat(lspSettings.getArtifactPath()).isEqualTo("test\\lsp.js") + assertThat(lspSettings.getArtifactPath()) + .isEqualTo("test\\lsp.js") } @Test - fun `artifact path cannot be null`() { - lspSettings.setArtifactPath(null) - assertThat(lspSettings.getArtifactPath()).isEmpty() + fun `empty artifact path is null`() { + lspSettings.setArtifactPath("") + assertThat(lspSettings.getArtifactPath()).isNull() + } + + @Test + fun `blank artifact path is null`() { + lspSettings.setArtifactPath(" ") + assertThat(lspSettings.getArtifactPath()).isNull() } @Test @@ -42,7 +48,7 @@ class LspSettingsTest { val element = xmlElement( """ - + """.trimIndent() ) lspSettings.setArtifactPath("temp\\lsp.js") @@ -51,22 +57,26 @@ class LspSettingsTest { val actual = XMLOutputter().outputString(element) - val expected = "\n" + - "" + // language=XML + val expected = """ + + + """.trimIndent() - assertThat(actual).isEqualTo(expected) + assertThat(actual).isEqualToIgnoringWhitespace(expected) } @Test fun `deserialize empty settings to ensure backwards compatibility`() { val element = xmlElement( """ - - - """ + + + """ ) val actual = XmlSerializer.deserialize(element, LspConfiguration::class.java) - assertThat(actual.artifactPath).isEmpty() + assertThat(actual.artifactPath).isNull() } @Test From 44d8f5fcaf9ee88d5291c9fbcdeab1c573d153c3 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Thu, 6 Mar 2025 11:02:00 -0800 Subject: [PATCH 02/11] feat(amazonq): use zipfs to extract server.zip to automatically copy posix attributes attributes are lost when copying the file out of ZipFile --- .../amazonq/lsp/artifacts/LspUtils.kt | 25 ++--- .../amazonq/lsp/artifacts/LspUtilsTest.kt | 99 +++++++++++++++++++ .../aws/toolkits/core/utils/PathUtils.kt | 3 + 3 files changed, 115 insertions(+), 12 deletions(-) create mode 100644 plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt index 0c361fe0154..8490ff680cc 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt @@ -7,16 +7,17 @@ import com.intellij.openapi.util.SystemInfo import com.intellij.openapi.util.text.StringUtil import com.intellij.util.io.DigestUtil import com.intellij.util.system.CpuArch +import software.aws.toolkits.core.utils.ZIP_PROPERTY_POSIX import software.aws.toolkits.core.utils.createParentDirectories import software.aws.toolkits.core.utils.exists +import software.aws.toolkits.core.utils.hasPosixFilePermissions import java.io.FileNotFoundException -import java.io.FileOutputStream +import java.nio.file.FileSystems import java.nio.file.Files import java.nio.file.Path import java.nio.file.Paths import java.nio.file.StandardCopyOption import java.security.MessageDigest -import java.util.zip.ZipFile import kotlin.io.path.isDirectory import kotlin.io.path.listDirectoryEntries @@ -78,17 +79,17 @@ fun extractZipFile(zipFilePath: Path, destDir: Path) { } try { - ZipFile(zipFilePath.toFile()).use { zipFile -> - zipFile.entries() - .asSequence() - .filterNot { it.isDirectory } - .map { zipEntry -> - val destPath = destDir.resolve(zipEntry.name) + FileSystems.newFileSystem( + zipFilePath, + mapOf(ZIP_PROPERTY_POSIX to destDir.hasPosixFilePermissions()) + ).use { zipfs -> + Files.walk(zipfs.getPath("/")) + .filter { !it.isDirectory() } + .forEach { zipEntry -> + val destPath = Paths.get(destDir.toString(), zipEntry.toString()) destPath.createParentDirectories() - FileOutputStream(destPath.toFile()).use { targetFile -> - zipFile.getInputStream(zipEntry).copyTo(targetFile) - } - }.toList() + Files.copy(zipEntry, destPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES) + } } } catch (e: Exception) { throw LspException("Failed to extract zip file: ${e.message}", LspException.ErrorCode.UNZIP_FAILED, cause = e) diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt new file mode 100644 index 00000000000..b076565d1ac --- /dev/null +++ b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt @@ -0,0 +1,99 @@ +// Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts + +import com.intellij.testFramework.utils.io.createDirectory +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Assumptions.assumeTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import software.aws.toolkits.core.utils.ZIP_PROPERTY_POSIX +import software.aws.toolkits.core.utils.hasPosixFilePermissions +import software.aws.toolkits.core.utils.putNextEntry +import software.aws.toolkits.core.utils.test.assertPosixPermissions +import software.aws.toolkits.core.utils.writeText +import software.aws.toolkits.jetbrains.utils.satisfiesKt +import java.nio.file.FileSystems +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.StandardCopyOption +import java.nio.file.attribute.PosixFilePermissions +import java.util.zip.ZipOutputStream +import kotlin.io.path.isDirectory +import kotlin.io.path.isRegularFile +import kotlin.io.path.setPosixFilePermissions + +class LspUtilsTest { + @Test + fun `extractZipFile works`(@TempDir tempDir: Path) { + val source = tempDir.resolve("source").also { it.createDirectory() } + val target = tempDir.resolve("target").also { it.createDirectory() } + + source.resolve("file1").writeText("contents1") + source.resolve("file2").writeText("contents2") + source.resolve("file3").writeText("contents3") + + val sourceZip = tempDir.resolve("source.zip") + ZipOutputStream(Files.newOutputStream(sourceZip)).use { zip -> + Files.walk(source).filter { it.isRegularFile() }.forEach { + zip.putNextEntry(source.relativize(it).toString(), it) + } + + val precedingSlashFile = source.resolve("file4").also { it.writeText("contents4") } + zip.putNextEntry("/${source.relativize(precedingSlashFile)}", precedingSlashFile) + } + + extractZipFile(sourceZip, target) + + assertThat(target).satisfiesKt { + val files = Files.list(it).toList() + assertThat(files.size).isEqualTo(4) + assertThat(target.resolve("file1")).hasContent("contents1") + assertThat(target.resolve("file2")).hasContent("contents2") + assertThat(target.resolve("file3")).hasContent("contents3") + assertThat(target.resolve("file4")).hasContent("contents4") + } + } + + @Test + fun `extractZipFile respects posix`(@TempDir tempDir: Path) { + assumeTrue(tempDir.hasPosixFilePermissions()) + + val source = tempDir.resolve("source").also { it.createDirectory() } + val target = tempDir.resolve("target").also { it.createDirectory() } + + source.resolve("regularFile").also { + it.writeText("contents1") + it.setPosixFilePermissions(PosixFilePermissions.fromString("rw-r--r--")) + } + source.resolve("executableFile").also { + it.writeText("contents2") + it.setPosixFilePermissions(PosixFilePermissions.fromString("rwxr-xr-x")) + } + + val sourceZip = tempDir.resolve("source.zip") + FileSystems.newFileSystem( + sourceZip, + mapOf( + "create" to true, + ZIP_PROPERTY_POSIX to true, + ) + ).use { zipfs -> + Files.walk(source) + .filter { !it.isDirectory() } + .forEach { file -> + Files.copy(file, zipfs.getPath("/").resolve(source.relativize(file).toString()), StandardCopyOption.COPY_ATTRIBUTES) + } + } + + extractZipFile(sourceZip, target) + + assertThat(target).satisfiesKt { + val files = Files.list(it).toList() + assertThat(files.size).isEqualTo(2) + assertPosixPermissions(target.resolve("regularFile"), "rw-r--r--") + assertPosixPermissions(target.resolve("executableFile"), "rwxr-xr-x") + } + } +} diff --git a/plugins/core/core/src/software/aws/toolkits/core/utils/PathUtils.kt b/plugins/core/core/src/software/aws/toolkits/core/utils/PathUtils.kt index 9e209a1a32d..dc4755688b0 100644 --- a/plugins/core/core/src/software/aws/toolkits/core/utils/PathUtils.kt +++ b/plugins/core/core/src/software/aws/toolkits/core/utils/PathUtils.kt @@ -212,3 +212,6 @@ private fun tryOrLogShortException(log: Logger, block: () -> T) = try { log.warn { "${e::class.simpleName}: ${e.message}" } null } + +// https://github.com/corretto/corretto-21/blob/364eb35886643e504344136075f4a2442d6c0cb0/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java#L90C33-L90C78 +const val ZIP_PROPERTY_POSIX = "enablePosixFileAttributes" From 3b8d3ff1efa7454a210a66320a332904569646da Mon Sep 17 00:00:00 2001 From: Richard Li Date: Thu, 6 Mar 2025 11:08:28 -0800 Subject: [PATCH 03/11] windows --- .../jetbrains/services/amazonq/lsp/AmazonQLspService.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt index d996b6aec0f..3506dd053d8 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt @@ -18,6 +18,7 @@ import com.intellij.openapi.components.serviceIfCreated import com.intellij.openapi.project.Project import com.intellij.openapi.util.Disposer import com.intellij.openapi.util.Key +import com.intellij.openapi.util.SystemInfo import com.intellij.util.io.await import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Deferred @@ -240,8 +241,9 @@ private class AmazonQServerInstance(private val project: Project, private val cs init { // will cause slow service init, but maybe fine for now. will not block UI since fetch/extract will be under background progress val artifact = runBlocking { ArtifactManager(project, manifestRange = null).fetchArtifact() }.toAbsolutePath() + val node = if (SystemInfo.isWindows) "node.exe" else "node" val cmd = GeneralCommandLine( - artifact.resolve("node").toString(), + artifact.resolve(node).toString(), LspSettings.getInstance().getArtifactPath() ?: artifact.resolve("aws-lsp-codewhisperer.js").toString(), "--stdio", "--set-credentials-encryption-key", From 84d53ddd1070a471f04833ca8b71473c94c24321 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Thu, 6 Mar 2025 11:47:33 -0800 Subject: [PATCH 04/11] tst --- .../codewhisperer/settings/CodeWhispererConfigurable.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt index 91cff463cd0..598913d3a3e 100644 --- a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt +++ b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt @@ -72,8 +72,8 @@ class CodeWhispererConfigurable(private val project: Project) : textFieldWithBrowseButton(fileChooserDescriptor = fileChooserDescriptor) .bindText( - { LspSettings.getInstance().getArtifactPath() }, - { LspSettings.getInstance().setArtifactPath(it.takeIf { v -> v.isNotBlank() }) } + { LspSettings.getInstance().getArtifactPath() ?: "" }, + { LspSettings.getInstance().setArtifactPath(it) } ) .applyToComponent { emptyText.text = "Choose a file to upload" From 8cb5a34d5d53ac5d46ac002f7cdfe85ac0498d55 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Thu, 6 Mar 2025 11:48:49 -0800 Subject: [PATCH 05/11] text --- .../codewhisperer/settings/CodeWhispererConfigurable.kt | 2 +- .../software/aws/toolkits/resources/MessagesBundle.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt index 598913d3a3e..706d843e5f3 100644 --- a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt +++ b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt @@ -76,7 +76,7 @@ class CodeWhispererConfigurable(private val project: Project) : { LspSettings.getInstance().setArtifactPath(it) } ) .applyToComponent { - emptyText.text = "Choose a file to upload" + emptyText.text = message("executableCommon.auto_managed") } .resizableColumn() .align(Align.FILL) diff --git a/plugins/core/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties b/plugins/core/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties index 6fd97883b49..94bdea402bf 100644 --- a/plugins/core/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties +++ b/plugins/core/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties @@ -1256,7 +1256,7 @@ ecs.service.not_found=Service {0} not found in cluster {1} ecs.task_definition.json_schema_name=AWS ECS Task Definition ecs.task_definitions=Task Definitions environment.variables.dialog.title=Environment Variables -executableCommon.auto_managed=Managed by AWS Toolkit +executableCommon.auto_managed=Managed by AWS executableCommon.auto_resolved=Auto-detected: {0} executableCommon.cli_not_configured={0} executable not configured executableCommon.configurable.title=External Tools From b879ac23a1e970d969e8a5a8486045117fa04825 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Thu, 6 Mar 2025 12:13:17 -0800 Subject: [PATCH 06/11] lint --- .../codewhisperer/settings/CodeWhispererConfigurable.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt index 706d843e5f3..154f48d61c3 100644 --- a/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt +++ b/plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt @@ -72,7 +72,7 @@ class CodeWhispererConfigurable(private val project: Project) : textFieldWithBrowseButton(fileChooserDescriptor = fileChooserDescriptor) .bindText( - { LspSettings.getInstance().getArtifactPath() ?: "" }, + { LspSettings.getInstance().getArtifactPath().orEmpty() }, { LspSettings.getInstance().setArtifactPath(it) } ) .applyToComponent { From b855364e54c30a9ff24f39fe8c56d0d1e487af60 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Thu, 6 Mar 2025 17:56:29 -0800 Subject: [PATCH 07/11] windows jank --- .../jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt index 8490ff680cc..ef67277fe5d 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt @@ -12,6 +12,7 @@ import software.aws.toolkits.core.utils.createParentDirectories import software.aws.toolkits.core.utils.exists import software.aws.toolkits.core.utils.hasPosixFilePermissions import java.io.FileNotFoundException +import java.net.URI import java.nio.file.FileSystems import java.nio.file.Files import java.nio.file.Path @@ -80,7 +81,8 @@ fun extractZipFile(zipFilePath: Path, destDir: Path) { try { FileSystems.newFileSystem( - zipFilePath, + // jar prefix due to potentially ambiguous resolution to wrong fs impl for zipfs on windows + URI("jar:${zipFilePath.toUri()}"), mapOf(ZIP_PROPERTY_POSIX to destDir.hasPosixFilePermissions()) ).use { zipfs -> Files.walk(zipfs.getPath("/")) From a7bbeaa3833ffd86bffbf94b3126e313724b1259 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Fri, 7 Mar 2025 10:29:49 -0800 Subject: [PATCH 08/11] close streams --- .../amazonq/lsp/artifacts/LspUtils.kt | 16 ++++++------ .../amazonq/lsp/artifacts/LspUtilsTest.kt | 25 +++++++++++-------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt index ef67277fe5d..7724a8c2255 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt @@ -85,13 +85,15 @@ fun extractZipFile(zipFilePath: Path, destDir: Path) { URI("jar:${zipFilePath.toUri()}"), mapOf(ZIP_PROPERTY_POSIX to destDir.hasPosixFilePermissions()) ).use { zipfs -> - Files.walk(zipfs.getPath("/")) - .filter { !it.isDirectory() } - .forEach { zipEntry -> - val destPath = Paths.get(destDir.toString(), zipEntry.toString()) - destPath.createParentDirectories() - Files.copy(zipEntry, destPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES) - } + Files.walk(zipfs.getPath("/")).use { paths -> + paths + .filter { !it.isDirectory() } + .forEach { zipEntry -> + val destPath = Paths.get(destDir.toString(), zipEntry.toString()) + destPath.createParentDirectories() + Files.copy(zipEntry, destPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES) + } + } } } catch (e: Exception) { throw LspException("Failed to extract zip file: ${e.message}", LspException.ErrorCode.UNZIP_FAILED, cause = e) diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt index b076565d1ac..57dce7166cd 100644 --- a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt +++ b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt @@ -36,12 +36,15 @@ class LspUtilsTest { val sourceZip = tempDir.resolve("source.zip") ZipOutputStream(Files.newOutputStream(sourceZip)).use { zip -> - Files.walk(source).filter { it.isRegularFile() }.forEach { - zip.putNextEntry(source.relativize(it).toString(), it) + Files.walk(source).use { paths -> + paths + .filter { it.isRegularFile() } + .forEach { + zip.putNextEntry(source.relativize(it).toString(), it) + } + val precedingSlashFile = source.resolve("file4").also { it.writeText("contents4") } + zip.putNextEntry("/${source.relativize(precedingSlashFile)}", precedingSlashFile) } - - val precedingSlashFile = source.resolve("file4").also { it.writeText("contents4") } - zip.putNextEntry("/${source.relativize(precedingSlashFile)}", precedingSlashFile) } extractZipFile(sourceZip, target) @@ -80,11 +83,13 @@ class LspUtilsTest { ZIP_PROPERTY_POSIX to true, ) ).use { zipfs -> - Files.walk(source) - .filter { !it.isDirectory() } - .forEach { file -> - Files.copy(file, zipfs.getPath("/").resolve(source.relativize(file).toString()), StandardCopyOption.COPY_ATTRIBUTES) - } + Files.walk(source).use { paths -> + paths + .filter { it.isRegularFile() } + .forEach { file -> + Files.copy(file, zipfs.getPath("/").resolve(source.relativize(file).toString()), StandardCopyOption.COPY_ATTRIBUTES) + } + } } extractZipFile(sourceZip, target) From 687dc3feee002b93005f1fc9e5b807de5d7f5e43 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Fri, 7 Mar 2025 11:00:40 -0800 Subject: [PATCH 09/11] tst --- .../jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt index 57dce7166cd..ccbc0ae05d2 100644 --- a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt +++ b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt @@ -20,7 +20,6 @@ import java.nio.file.Path import java.nio.file.StandardCopyOption import java.nio.file.attribute.PosixFilePermissions import java.util.zip.ZipOutputStream -import kotlin.io.path.isDirectory import kotlin.io.path.isRegularFile import kotlin.io.path.setPosixFilePermissions @@ -57,6 +56,9 @@ class LspUtilsTest { assertThat(target.resolve("file3")).hasContent("contents3") assertThat(target.resolve("file4")).hasContent("contents4") } + + // windows hack + System.gc() } @Test From 370f2d69658ce0d8036093c742ea8c8533d38958 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Fri, 7 Mar 2025 11:06:55 -0800 Subject: [PATCH 10/11] more cleanup --- .../jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt | 5 +---- .../core/src/software/aws/toolkits/core/utils/ZipUtils.kt | 3 +-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt index ccbc0ae05d2..f38543688de 100644 --- a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt +++ b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt @@ -49,16 +49,13 @@ class LspUtilsTest { extractZipFile(sourceZip, target) assertThat(target).satisfiesKt { - val files = Files.list(it).toList() + val files = Files.list(it).use { stream -> stream.toList() } assertThat(files.size).isEqualTo(4) assertThat(target.resolve("file1")).hasContent("contents1") assertThat(target.resolve("file2")).hasContent("contents2") assertThat(target.resolve("file3")).hasContent("contents3") assertThat(target.resolve("file4")).hasContent("contents4") } - - // windows hack - System.gc() } @Test diff --git a/plugins/core/core/src/software/aws/toolkits/core/utils/ZipUtils.kt b/plugins/core/core/src/software/aws/toolkits/core/utils/ZipUtils.kt index a91ea7b9686..b1229f570ab 100644 --- a/plugins/core/core/src/software/aws/toolkits/core/utils/ZipUtils.kt +++ b/plugins/core/core/src/software/aws/toolkits/core/utils/ZipUtils.kt @@ -3,7 +3,6 @@ package software.aws.toolkits.core.utils -import java.io.BufferedInputStream import java.io.ByteArrayInputStream import java.io.IOException import java.io.InputStream @@ -17,7 +16,7 @@ import java.util.zip.ZipOutputStream */ fun ZipOutputStream.putNextEntry(entryName: String, file: Path) { try { - BufferedInputStream(Files.newInputStream(file)).use { inputStream -> + Files.newInputStream(file).buffered().use { inputStream -> putNextEntry(entryName, inputStream) } } catch (e: IOException) { From d3d19af4a1fb6cdd0e5b341579a6d899e0cc8c97 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Fri, 7 Mar 2025 11:07:20 -0800 Subject: [PATCH 11/11] more cleanup --- .../jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt index f38543688de..607b94f34bf 100644 --- a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt +++ b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt @@ -94,7 +94,7 @@ class LspUtilsTest { extractZipFile(sourceZip, target) assertThat(target).satisfiesKt { - val files = Files.list(it).toList() + val files = Files.list(it).use { stream -> stream.toList() } assertThat(files.size).isEqualTo(2) assertPosixPermissions(target.resolve("regularFile"), "rw-r--r--") assertPosixPermissions(target.resolve("executableFile"), "rwxr-xr-x")