[BUG FIX] Fix path resolution for executables #328
Conversation
| * @return The best matching executable path as a String. | ||
| */ | ||
| fun findBestExecutableMatch(executableName: String): String { | ||
| val pathVariable = System.getenv("PATH") ?: return executableName |
There was a problem hiding this comment.
iirc this won't work on windows where environment variables are case-insensitive (and default to Path)
| @@ -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) | |||
There was a problem hiding this comment.
From a quick glance it looks like this will break the download functionality entirely by only using already installed applications
There was a problem hiding this comment.
We will address these points. Thanks.
There was a problem hiding this comment.
Done with tests to validate behaviour (w/ download and without download)
| @@ -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()) | |||
There was a problem hiding this comment.
Primarily for test validation. Welcome other suggestions.
1. Add abiility for tests to override environment behaviour 2. Fix tests.
rasharab
force-pushed
the
FixGradleNode
branch
from
77c49cd to
eb959c7
Compare
Updated comments. I prematurely submitted this pr, apologies. |
There’s an issue with mac silicion+intellij, whereby gradle process executor is not respecting path environment variables.
This gets around that by using a fix for the plugin whereby we attempt to resolve the path manually.
#152
https://youtrack.jetbrains.com/issue/IDEA-334183
We hit this constantly with our development team and this fix resolved it for us.