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

[BUG FIX] Fix path resolution for executables #328

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
30 changes: 30 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -59,6 +60,35 @@ dependencies {
testImplementation("org.mock-server:mockserver-netty:5.15.0")
}

publishing {
publications {
create<MavenPublication>("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)
Expand Down
4 changes: 4 additions & 0 deletions src/main/kotlin/com/github/gradle/node/NodeExtension.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -181,6 +182,9 @@ open class NodeExtension(project: Project) {
*/
val resolvedPlatform = project.objects.property<Platform>()


val environment = project.objects.mapProperty<String, String>().convention(System.getenv())
Copy link
Author

@rasharab rasharab Jan 16, 2025

Choose a reason for hiding this comment

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

Primarily for test validation. Welcome other suggestions.


init {
distBaseUrl.set("https://nodejs.org/dist")
}
Expand Down
6 changes: 3 additions & 3 deletions src/main/kotlin/com/github/gradle/node/exec/ExecRunner.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import java.io.File
*
* @param execConfiguration configuration to get environment variables from
*/
fun computeEnvironment(execConfiguration: ExecConfiguration): Map<String, String> {
fun computeEnvironment(extension: NodeExtension, execConfiguration: ExecConfiguration): Map<String, String> {
val execEnvironment = mutableMapOf<String, String>()
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
Expand All @@ -34,7 +35,6 @@ fun computeWorkingDir(nodeProjectDir: DirectoryProperty, execConfiguration: Exec
workingDir.mkdirs()
return workingDir
}

/**
* Basic execution runner that runs a given ExecConfiguration.
*
Expand All @@ -46,7 +46,7 @@ class ExecRunner {
return projectHelper.exec {
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)
Expand Down
34 changes: 31 additions & 3 deletions src/main/kotlin/com/github/gradle/node/variant/VariantComputer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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<Map<String, String>>,
): 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<Directory>): Provider<String> {
fun computeNodeExec(
nodeExtension: NodeExtension,
nodeBinDirProvider: Provider<Directory>,
): Provider<String> {
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<Directory>, command: String, isWindows: Boolean): Provider<String> {
return nodeDirProvider.map { nodeDir ->
if (isWindows) nodeDir.dir("node_modules/npm/bin/$command-cli.js").asFile.path
Expand Down Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down
113 changes: 113 additions & 0 deletions src/test/groovy/com/github/gradle/node/npm/task/NpmTaskTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,92 @@ 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 {
def cleanup() {
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)
Expand Down Expand Up @@ -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", {}))
Expand Down Expand Up @@ -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)

Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down