node-gradle / gradle-node-plugin

Gradle plugin for integrating NodeJS in your build. :rocket:
Apache License 2.0
601 stars 117 forks source link

Possibility to access the node executable path through the extension #235

Open Vampire opened 2 years ago

Vampire commented 2 years ago

I imagine a simple way to get the path to the node executable from the node extension. In my case I want to configure it for SonarQube, so a String / Provider<String> would be fine. But optimally it would probably be a RegularFileProperty.

So I imagine something like

property("sonar.nodejs.executable", node.executable.get())

instead of currently

property("sonar.nodejs.executable", VariantComputer()
    .let { variantComputer ->
        variantComputer
            .computeNodeDir(node)
            .let { variantComputer.computeNodeBinDir(it) }
            .let { variantComputer.computeNodeExec(node, it) }
            .get()
    }
)
rkrisztian commented 2 years ago

There is a better workaround possible with version 3, through the nodeSetup task (this is a Groovy example though):

property 'sonar.nodejs.executable', "${nodeSetup.nodeDir.get()}/bin/node"

Edit: This code is not Task Configuration Avoidance API compatible. If I'm not wrong, it should be put inside a tasks.named('sonarqube').configure {} block, and task sonarqube should depend on nodeSetup.

mattlong-finocomp commented 1 year ago

I'd like to second this request. I now have the following since 6.0.0:

def nodeDirProvider = node.resolvedNodeDir
def nodeBinDirProvider = variantComputer.computeNodeBinDir(nodeDirProvider, node.resolvedPlatform)
def nodeBinary = variantComputer.computeNodeExec(node, nodeBinDirProvider).get()

Ideally we'd be able to get resolvedNodeExec from the NodeExtension. Having this exposed on the extension would help users not have to make changes as you deprecate methods in VariantComputer.

Also the previous comment isn't platform aware and would fail on some operating systems as the path to the binary is platform dependent.

rkrisztian commented 1 year ago

Tested 7.0.0, and for me this is the code that works (in Groovy):

import com.github.gradle.node.variant.VariantComputer
import com.github.gradle.node.variant.VariantComputerKt
// (...)

        def nodeBinDirProvider = new VariantComputer().computeNodeBinDir(node.resolvedNodeDir, node.resolvedPlatform)
        def nodeBinary = VariantComputerKt.computeNodeExec(node, nodeBinDirProvider).get()
        property 'sonar.nodejs.executable', "${nodeBinDirProvider.get().asFile.toPath().resolve(nodeBinary)}"

Just using nodeBinary is not enough, as it's evaluated as just node on Linux.

I agree that we need a much more convenient API than these workarounds.

deepy commented 1 year ago

The configuration-cache changes exposed some sharp angles and there's been a bunch of changes I've had to do to accommodate them In hindsight, this should just have been stored on an extension right from 2.0.0, but right now there's some redesign that needs to be done in the way the tasks are setup.

With the way things are going right now, VariantComputer is going out the window, with the configuration-cache it's no longer possible to do what we intended with it in the first place. But this also means we need to find some good extension points for letting users affect the process, and that's what's holding me up. And I had hoped to side-step the entire issue by moving towards Shared Build Services but that has its own set of issues

rkrisztian commented 1 year ago

BTW, if download = false is set, the variant computer does not even want to reuse $PATH.

deepy commented 1 year ago

The current use of VariantComputer (iirc) is pretty much limited to: figuring out the paths of things if download = true and figuring out whether we want to call node or node.cmd in both cases We don't actually ever really look at the path though, we just let the OS handle that (which is generally fine except it also means we re-download node even if that specific version is already on the path)