Support running Node tools in the Configuration Phase, and multiple tools per task by providing ValueSource implementations

[adam-enko]

Feature Request: Provide ValueSource implementations for running Node tools

Summary

Currently Gradle Plugin for Node has a 'task based' approach for running Node tools. One task can run one Node tool a single time. This is good for simple projects.

However, I would like to use Node tools in more advanced use cases:

1. Use Node tools before task execution, during the configuration phase.

   As part of Make npmInstall and npmSetup tasks cacheable #81 I proposed a way to make npm install cacheable, but this would require running npm in the configuration phase (to manually add dependencies to the cache). Using a ValueSource would make this trivial.

2. Support executing the same tool multiple times with different arguments or running different tools sequentially in the same task.

Proposal

Provide implementations of ValueSource so users can run node tools on-demand.

The existing task-based execution approach would still work as-is, but to avoid duplication it would be best to re-write the internal implementation to use a 'ValueSource first' approach. The existing tasks will have the same API surface (so there is no change for users), but internally they will use ValueSources.

We can break up the implementation into smaller steps by migrating each Node tool individually.

I'd be happy to make a PR.

[deepy]

Wouldn't the current FooExecRunners already work pretty well in a ValueSource?
The current tasks are pretty much just thin wrappers around the runners, so it probably wouldn't be hard to replace them with something else while keeping the tasks the same

I'm on a train and not able to take a closer look until later today, but the immediate question is what improvements a ValueSource brings?
And I'm generally positive in regards to the idea, and even if it all it provides is exactly the same as what the current runners do I think it'd be fine, but I'm trying to picture how it'd work (although I'm hoping it'll make writing tests a little bit easier)

Regardless of whether there's a ValueSource-first approach or not I think it makes perfect sense to add ValueSources that consumers can use
My station is coming up in a moment so unfortunately I didn't have enough time to finish this post 😅

[adam-enko]

Thanks for the tip! I did see the ExecRunners, but I assumed they were an internal utility, not intended to be used in buildscripts (because they have no docs, some things are deprecated, and setting them up is quite complicated).

I think you're right that the idea of using ValueSources is similar to the intention behind the ExecRunners - although I would really like to make the API much easier to create.

Your question about "what improvements does a ValueSource bring?" did stump me a bit, because I wasn't sure how to answers. So, I did some experimentation today, and I have some results.

The problem is the ExecRunners aren't compatible with configuration cache, and can't be run in the configuration phase.

I want to do something like this:

tasks.npmInstall {
    val npmInstallOfflineExecResult = nodeExtension.npmExec {
        command = listOf("install", "--dry-run", "--offline")
    }

    outputs.upToDateWhen {
        "missing dependencies" in npmInstallOfflineExecResult.get().standardOutput 
    }
}

I tried wrapping the ExecRunner in a provider {}, but Gradle complains about running exec {} in the configuration phase. The solution to that is a ValueSource, but this isn't possible at the moment:

I tried creating a ValueSource that 'wraps' NpmExecRunner, but I encountered two problems:

• Creating an NpmExecRunner requires an instance of NodeExtension, but this cannot be passed as a ValueSource parameter as it is not serializable.
• NpmExecRunner also requires types (DefaultProjectApiHelper,NpmExecRunner ) that must be created using an ObjectFactory, but an ObjectFactory is not injectable into a ValueSource.

[deepy]

Ah that'd do it, it's easy to simplify the construction of the ExecRunners, but types not accessible in ValueSources would make that problematic

Adding my notes before I go to sleep, but the immediate issue of running anything in the configuration phase is that it requires downloading node in the download = true scenario
Luckily we're already stuck doing that right now, as downloading node is done through a detached configuration, so I don't think there's anything preventing us from unpacking it during the configuration phase

I've been planning to look into using artifact transforms for the unzip/untar, and while I'm currently uncertain at how early they run, my impression is that they probably would work
And if those work, then I can't think of any additional roadblocks :-)
(and I'm still hoping that one day there'll be first-class support in gradle for toolchains that aren't java)

[adam-enko]

Adding my notes before I go to sleep, but the immediate issue of running anything in the configuration phase is that it requires downloading node in the download = true scenario Luckily we're already stuck doing that right now, as downloading node is done through a detached configuration, so I don't think there's anything preventing us from unpacking it during the configuration phase

Good point! We would have to account for that.

Although, when specifically regarding a npmInstall up-to-date check, if npm wasn't installed then it'd be acceptable if the ValueSource just returns 'false'.

Maybe it'd be worth having a requireNpmInstalled boolean flag in the proposed NPM ValueSource. If true: the ValueSource would try to install node, or fail. If false: return a 'null' result.

I've been planning to look into using artifact transforms for the unzip/untar, and while I'm currently uncertain at how early they run, my impression is that they probably would work And if those work, then I can't think of any additional roadblocks :-)

So, unfortunately artifact transforms have a pretty annoying drawback: they are sensitive to any changes in the buildscript. This means they will always be re-run, even for unrelated buildscript changes. gradle/gradle#30968

Fortunately, it's not such a big deal. Artifact transforms aren't so special, and it's quite easy to roll your own.

(and I'm still hoping that one day there'll be first-class support in gradle for toolchains that aren't java)

Me too!

I would recommend looking at the approach taken by an ex-Gradle dev https://github.com/jjohannes/gradle-demos/blob/main/toolchain-management/requirements.md. It uses some internal Gradle APIs (one for creating checksums, and another for dynamically defining repositories), but I don't think they're necessary.