From f951da8e2dae701f75b18d37c27cad8c5490bfc3 Mon Sep 17 00:00:00 2001 From: Rashin Arab <3806658+rasharab@users.noreply.github.com> Date: Wed, 15 Jan 2025 20:41:01 -0800 Subject: [PATCH 1/2] [BUG FIX] Fix path resolution for executables --- build.gradle.kts | 30 +++++++++++++++++++ .../com/github/gradle/node/exec/ExecRunner.kt | 21 ++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index cea9b7df..36124198 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -19,6 +19,7 @@ plugins { id("com.cinnober.gradle.semver-git") version "3.0.0" id("org.jetbrains.dokka") version "1.7.10" id("org.gradle.test-retry") version "1.5.0" + `maven-publish` } group = "com.github.node-gradle" @@ -59,6 +60,35 @@ dependencies { testImplementation("org.mock-server:mockserver-netty:5.15.0") } +publishing { + publications { + create("mavenJava") { + from(components["java"]) + groupId = "com.github.node-gradle" + artifactId = "gradle-node-plugin" + version = project.version.toString() + pom { + name.set("Gradle Node.js Plugin") + description.set("Gradle plugin for executing Node.js scripts. Supports npm, pnpm, Yarn, and Bun.") + url.set("https://github.com/node-gradle/gradle-node-plugin") + licenses { + license { + name.set("Apache License, Version 2.0") + url.set("http://www.apache.org/licenses/LICENSE-2.0.txt") + } + } + } + } + } + repositories { + maven { + name = "localMaven" + url = uri("${buildDir}/maven-repo") + } + } +} + + tasks.compileTestGroovy { // Should be // classpath += files(sourceSets.test.get().kotlin.classesDirectory) diff --git a/src/main/kotlin/com/github/gradle/node/exec/ExecRunner.kt b/src/main/kotlin/com/github/gradle/node/exec/ExecRunner.kt index 99d4ab9a..03466fcb 100644 --- a/src/main/kotlin/com/github/gradle/node/exec/ExecRunner.kt +++ b/src/main/kotlin/com/github/gradle/node/exec/ExecRunner.kt @@ -35,6 +35,24 @@ fun computeWorkingDir(nodeProjectDir: DirectoryProperty, execConfiguration: Exec return workingDir } +/** + * Helper function to find the best matching executable in the system PATH. + * + * @param executableName The name of the executable to search for. + * @return The best matching executable path as a String. + */ +fun findBestExecutableMatch(executableName: String): String { + val pathVariable = System.getenv("PATH") ?: return executableName + val paths = pathVariable.split(File.pathSeparator) + for (path in paths) { + val executableFile = File(path, executableName) + if (executableFile.exists() && executableFile.canExecute()) { + return executableFile.absolutePath + } + } + return executableName // Return the original executable if no match is found +} + /** * Basic execution runner that runs a given ExecConfiguration. * @@ -43,8 +61,9 @@ fun computeWorkingDir(nodeProjectDir: DirectoryProperty, execConfiguration: Exec */ class ExecRunner { fun execute(projectHelper: ProjectApiHelper, extension: NodeExtension, execConfiguration: ExecConfiguration): ExecResult { + val executablePath = findBestExecutableMatch(execConfiguration.executable) return projectHelper.exec { - executable = execConfiguration.executable + executable = executablePath args = execConfiguration.args environment = computeEnvironment(execConfiguration) isIgnoreExitValue = execConfiguration.ignoreExitValue From eb959c77e67897f9d421779abea4c67b8e674780 Mon Sep 17 00:00:00 2001 From: Rashin Arab <3806658+rasharab@users.noreply.github.com> Date: Thu, 16 Jan 2025 13:00:39 -0800 Subject: [PATCH 2/2] Address comments 1. Add abiility for tests to override environment behaviour 2. Fix tests. --- .../com/github/gradle/node/NodeExtension.kt | 4 + .../com/github/gradle/node/exec/ExecRunner.kt | 27 +---- .../gradle/node/variant/VariantComputer.kt | 34 +++++- .../node/npm/task/NpmInstallTaskTest.groovy | 2 + .../gradle/node/npm/task/NpmTaskTest.groovy | 113 ++++++++++++++++++ .../gradle/node/npm/task/NpxTaskTest.groovy | 1 + .../gradle/node/task/NodeTaskTest.groovy | 2 + .../node/variant/VariantComputerTest.groovy | 2 + 8 files changed, 159 insertions(+), 26 deletions(-) diff --git a/src/main/kotlin/com/github/gradle/node/NodeExtension.kt b/src/main/kotlin/com/github/gradle/node/NodeExtension.kt index 8fa9a68c..44a3a9b9 100644 --- a/src/main/kotlin/com/github/gradle/node/NodeExtension.kt +++ b/src/main/kotlin/com/github/gradle/node/NodeExtension.kt @@ -5,6 +5,7 @@ import com.github.gradle.node.util.Platform import org.gradle.api.Project import org.gradle.kotlin.dsl.create import org.gradle.kotlin.dsl.getByType +import org.gradle.kotlin.dsl.mapProperty import org.gradle.kotlin.dsl.property open class NodeExtension(project: Project) { @@ -181,6 +182,9 @@ open class NodeExtension(project: Project) { */ val resolvedPlatform = project.objects.property() + + val environment = project.objects.mapProperty().convention(System.getenv()) + init { distBaseUrl.set("https://nodejs.org/dist") } diff --git a/src/main/kotlin/com/github/gradle/node/exec/ExecRunner.kt b/src/main/kotlin/com/github/gradle/node/exec/ExecRunner.kt index 03466fcb..8d0bafc0 100644 --- a/src/main/kotlin/com/github/gradle/node/exec/ExecRunner.kt +++ b/src/main/kotlin/com/github/gradle/node/exec/ExecRunner.kt @@ -13,9 +13,10 @@ import java.io.File * * @param execConfiguration configuration to get environment variables from */ -fun computeEnvironment(execConfiguration: ExecConfiguration): Map { +fun computeEnvironment(extension: NodeExtension, execConfiguration: ExecConfiguration): Map { val execEnvironment = mutableMapOf() execEnvironment += System.getenv() + execEnvironment += extension.environment.getOrElse(emptyMap()) execEnvironment += execConfiguration.environment if (execConfiguration.additionalBinPaths.isNotEmpty()) { // Take care of Windows environments that may contain "Path" OR "PATH" - both existing @@ -34,25 +35,6 @@ fun computeWorkingDir(nodeProjectDir: DirectoryProperty, execConfiguration: Exec workingDir.mkdirs() return workingDir } - -/** - * Helper function to find the best matching executable in the system PATH. - * - * @param executableName The name of the executable to search for. - * @return The best matching executable path as a String. - */ -fun findBestExecutableMatch(executableName: String): String { - val pathVariable = System.getenv("PATH") ?: return executableName - val paths = pathVariable.split(File.pathSeparator) - for (path in paths) { - val executableFile = File(path, executableName) - if (executableFile.exists() && executableFile.canExecute()) { - return executableFile.absolutePath - } - } - return executableName // Return the original executable if no match is found -} - /** * Basic execution runner that runs a given ExecConfiguration. * @@ -61,11 +43,10 @@ fun findBestExecutableMatch(executableName: String): String { */ class ExecRunner { fun execute(projectHelper: ProjectApiHelper, extension: NodeExtension, execConfiguration: ExecConfiguration): ExecResult { - val executablePath = findBestExecutableMatch(execConfiguration.executable) return projectHelper.exec { - executable = executablePath + executable = execConfiguration.executable args = execConfiguration.args - environment = computeEnvironment(execConfiguration) + environment = computeEnvironment(extension, execConfiguration) isIgnoreExitValue = execConfiguration.ignoreExitValue workingDir = computeWorkingDir(extension.nodeProjectDir, execConfiguration) execConfiguration.execOverrides?.execute(this) diff --git a/src/main/kotlin/com/github/gradle/node/variant/VariantComputer.kt b/src/main/kotlin/com/github/gradle/node/variant/VariantComputer.kt index 49ed02cb..323bebee 100644 --- a/src/main/kotlin/com/github/gradle/node/variant/VariantComputer.kt +++ b/src/main/kotlin/com/github/gradle/node/variant/VariantComputer.kt @@ -8,19 +8,47 @@ import org.gradle.api.file.Directory import org.gradle.api.file.DirectoryProperty import org.gradle.api.provider.Property import org.gradle.api.provider.Provider +import java.io.File + +/** + * Helper function to find the best matching executable in the system PATH. + * + * @param executableName The name of the executable to search for. + * @return The best matching executable path as a String. + */ +fun findBestExecutableMatch( + executableName: String, + environmentProvider: Provider>, +): String { + val environment = environmentProvider.orNull ?: return executableName + val pathEnvironmentVariableName = if (environment["Path"] != null) "Path" else "PATH" + val pathVariable = environment[pathEnvironmentVariableName] ?: return executableName + val paths = pathVariable.split(File.pathSeparator) + for (path in paths) { + val executableFile = File(path, executableName) + if (executableFile.exists() && executableFile.canExecute()) { + return executableFile.absolutePath + } + } + return executableName // Return the original executable if no match is found +} /** * Get the expected node binary name, node.exe on Windows and node everywhere else. */ -fun computeNodeExec(nodeExtension: NodeExtension, nodeBinDirProvider: Provider): Provider { +fun computeNodeExec( + nodeExtension: NodeExtension, + nodeBinDirProvider: Provider, +): Provider { return zip(nodeExtension.download, nodeBinDirProvider).map { val (download, nodeBinDir) = it if (download) { val nodeCommand = if (nodeExtension.resolvedPlatform.get().isWindows()) "node.exe" else "node" nodeBinDir.dir(nodeCommand).asFile.absolutePath - } else "node" + } else findBestExecutableMatch("node", nodeExtension.environment) } } + fun computeNpmScriptFile(nodeDirProvider: Provider, command: String, isWindows: Boolean): Provider { return nodeDirProvider.map { nodeDir -> if (isWindows) nodeDir.dir("node_modules/npm/bin/$command-cli.js").asFile.path @@ -51,7 +79,7 @@ internal fun computeExec(nodeExtension: NodeExtension, binDirProvider: Provider< val command = if (nodeExtension.resolvedPlatform.get().isWindows()) { cfgCommand.mapIf({ it == unixCommand }) { windowsCommand } } else cfgCommand - if (download) binDir.dir(command).asFile.absolutePath else command + if (download) binDir.dir(command).asFile.absolutePath else findBestExecutableMatch(command, nodeExtension.environment) } } diff --git a/src/test/groovy/com/github/gradle/node/npm/task/NpmInstallTaskTest.groovy b/src/test/groovy/com/github/gradle/node/npm/task/NpmInstallTaskTest.groovy index 5142df68..ef28586d 100644 --- a/src/test/groovy/com/github/gradle/node/npm/task/NpmInstallTaskTest.groovy +++ b/src/test/groovy/com/github/gradle/node/npm/task/NpmInstallTaskTest.groovy @@ -13,6 +13,7 @@ class NpmInstallTaskTest extends AbstractTaskTest { def "exec npm install task with configured proxy"() { given: nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {})) + nodeExtension.environment.set([:]) GradleProxyHelper.setHttpsProxyHost("my-super-proxy.net") GradleProxyHelper.setHttpsProxyPort(11235) @@ -32,6 +33,7 @@ class NpmInstallTaskTest extends AbstractTaskTest { def "exec npm install task with configured proxy but disabled"() { given: nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {})) + nodeExtension.environment.set([:]) GradleProxyHelper.setHttpsProxyHost("my-super-proxy.net") GradleProxyHelper.setHttpsProxyPort(11235) nodeExtension.nodeProxySettings.set(ProxySettings.OFF) diff --git a/src/test/groovy/com/github/gradle/node/npm/task/NpmTaskTest.groovy b/src/test/groovy/com/github/gradle/node/npm/task/NpmTaskTest.groovy index c077d535..00e9b6aa 100644 --- a/src/test/groovy/com/github/gradle/node/npm/task/NpmTaskTest.groovy +++ b/src/test/groovy/com/github/gradle/node/npm/task/NpmTaskTest.groovy @@ -6,6 +6,8 @@ import com.github.gradle.node.npm.proxy.ProxySettings import com.github.gradle.node.task.AbstractTaskTest import com.github.gradle.node.util.PlatformHelperKt +import java.nio.file.Files + import static com.github.gradle.node.NodeExtension.DEFAULT_NODE_VERSION class NpmTaskTest extends AbstractTaskTest { @@ -13,9 +15,83 @@ class NpmTaskTest extends AbstractTaskTest { GradleProxyHelper.resetProxy() } + /** + * Creates a temporary directory with a dummy executable file. + * + * @param fileName The name of the dummy executable (e.g., "npm"). + * @param scriptContent (Optional) The content inside the executable. Defaults to a simple echo script. + * @return File object pointing to the dummy executable. + */ + File createTempDummyExecutable(String fileName, String scriptContent = "#!/bin/bash\necho 'Dummy executable ran with args: \$@'") { + // Create a temporary directory + File tempDir = Files.createTempDirectory("dummy-bin").toFile() + + // Create the dummy executable inside the temp directory + File executableFile = new File(tempDir, fileName) + + // Write the script content + executableFile.text = scriptContent + + // Set executable permissions + executableFile.setExecutable(true) + + return executableFile + } + + def "exec npm task with environment variable"() { + given: + def npmFile = createTempDummyExecutable("npm") + + nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {})) + nodeExtension.environment.set(["PATH": npmFile.parent]) + + def task = project.tasks.create('simple', NpmTask) + mockProjectApiHelperExec(task) + task.args.set(['a', 'b']) + task.environment.set(['a': '1']) + task.ignoreExitValue.set(true) + task.workingDir.set(projectDir) + + when: + project.evaluate() + task.exec() + + then: + 1 * execSpec.setIgnoreExitValue(true) + 1 * execSpec.setEnvironment({ it['a'] == '1' && containsPath(it) }) + 1 * execSpec.setExecutable(npmFile.absolutePath) + 1 * execSpec.setArgs(['a', 'b']) + } + + def "exec npm task with environment variable (windows)"() { + given: + def npmFile = createTempDummyExecutable("npm.cmd") + + nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Windows", "x86_64", {})) + nodeExtension.environment.set(["Path": npmFile.parent]) + + def task = project.tasks.create('simple', NpmTask) + mockProjectApiHelperExec(task) + task.args.set(['a', 'b']) + task.environment.set(['a': '1']) + task.ignoreExitValue.set(true) + task.workingDir.set(projectDir) + + when: + project.evaluate() + task.exec() + + then: + 1 * execSpec.setIgnoreExitValue(true) + 1 * execSpec.setEnvironment({ it['a'] == '1' && containsPath(it) }) + 1 * execSpec.setExecutable(npmFile.absolutePath) + 1 * execSpec.setArgs(['a', 'b']) + } + def "exec npm task"() { given: nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {})) + nodeExtension.environment.set([:]) def task = project.tasks.create('simple', NpmTask) mockProjectApiHelperExec(task) @@ -57,6 +133,39 @@ class NpmTaskTest extends AbstractTaskTest { 1 * execSpec.setArgs(['a', 'b']) } + def "exec npm task with path respects download"() { + given: + def npmFile = createTempDummyExecutable("npm") + + nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {})) + nodeExtension.environment.set(["PATH": npmFile.parent]) + nodeExtension.download.set(true) + + def nodeDir = projectDir.toPath().resolve(".gradle").resolve("nodejs") + .resolve("node-v${DEFAULT_NODE_VERSION}-linux-x64") + + def task = project.tasks.create('simple', NpmTask) + mockProjectApiHelperExec(task) + task.args.set(["run", "command"]) + + when: + project.evaluate() + task.exec() + + then: + 1 * execSpec.setIgnoreExitValue(false) + 1 * execSpec.setExecutable({ executable -> + def expectedNodePath = nodeDir.resolve("bin").resolve("node") + return fixAbsolutePath(executable) == expectedNodePath.toAbsolutePath().toString() + }) + 1 * execSpec.setArgs({ args -> + def command = nodeDir + .resolve("lib").resolve("node_modules").resolve("npm").resolve("bin") + .resolve("npm-cli.js").toAbsolutePath().toString() + return fixAbsolutePaths(args) == [command, "run", "command"] + }) + } + def "exec npm task (download)"() { given: nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {})) @@ -121,6 +230,8 @@ class NpmTaskTest extends AbstractTaskTest { def "exec npm task with configured proxy"() { given: nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {})) + nodeExtension.environment.set([:]) + GradleProxyHelper.setHttpsProxyHost("my-super-proxy.net") GradleProxyHelper.setHttpsProxyPort(11235) @@ -141,6 +252,8 @@ class NpmTaskTest extends AbstractTaskTest { def "exec npm task with configured proxy but disabled"() { given: nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {})) + nodeExtension.environment.set([:]) + GradleProxyHelper.setHttpsProxyHost("my-super-proxy.net") GradleProxyHelper.setHttpsProxyPort(11235) nodeExtension.nodeProxySettings.set(ProxySettings.OFF) diff --git a/src/test/groovy/com/github/gradle/node/npm/task/NpxTaskTest.groovy b/src/test/groovy/com/github/gradle/node/npm/task/NpxTaskTest.groovy index 53912700..4aa9fb53 100644 --- a/src/test/groovy/com/github/gradle/node/npm/task/NpxTaskTest.groovy +++ b/src/test/groovy/com/github/gradle/node/npm/task/NpxTaskTest.groovy @@ -10,6 +10,7 @@ class NpxTaskTest extends AbstractTaskTest { def "exec npx task"() { given: nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {})) + nodeExtension.environment.set([:]) def task = project.tasks.create('simple', NpxTask) mockProjectApiHelperExec(task) diff --git a/src/test/groovy/com/github/gradle/node/task/NodeTaskTest.groovy b/src/test/groovy/com/github/gradle/node/task/NodeTaskTest.groovy index 8899745f..45604c1f 100644 --- a/src/test/groovy/com/github/gradle/node/task/NodeTaskTest.groovy +++ b/src/test/groovy/com/github/gradle/node/task/NodeTaskTest.groovy @@ -18,6 +18,7 @@ class NodeTaskTest extends AbstractTaskTest { def "exec node task"() { given: nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {})) + nodeExtension.environment.set([:]) nodeExtension.download.set(false) def task = project.tasks.create('simple', NodeTask) @@ -93,6 +94,7 @@ class NodeTaskTest extends AbstractTaskTest { given: nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Windows", "x86_64", {})) nodeExtension.download.set(false) + nodeExtension.environment.set([:]) def task = project.tasks.create('simple', NodeTask) mockProjectApiHelperExec(task) diff --git a/src/test/groovy/com/github/gradle/node/variant/VariantComputerTest.groovy b/src/test/groovy/com/github/gradle/node/variant/VariantComputerTest.groovy index 2f0d5e93..970a3698 100644 --- a/src/test/groovy/com/github/gradle/node/variant/VariantComputerTest.groovy +++ b/src/test/groovy/com/github/gradle/node/variant/VariantComputerTest.groovy @@ -157,6 +157,7 @@ class VariantComputerTest extends Specification { nodeExtension.resolvedPlatform.set(platform) nodeExtension.download.set(download) nodeExtension.npmVersion.set(npmVersion) + nodeExtension.environment.set([:]) def variantComputer = new VariantComputer() @@ -208,6 +209,7 @@ class VariantComputerTest extends Specification { nodeExtension.resolvedPlatform.set(platform) nodeExtension.download.set(download) nodeExtension.npmVersion.set(npmVersion) + nodeExtension.environment.set([:]) def variantComputer = new VariantComputer()