Release version 3.0

[bsautel]

@deepy as far as I know, the 3.0 branch is ready to be deployed. All the tests are successful even if some of them are a little unstable (seems to come from GitHub Actions hosts or npm or yarn that sometimes fail and not from our code).

It remains a few things that I know you wanted to do, such as publishing to bintray or publishing the API docs.

I am wondering whether this is really blocking. The current production release does not have all that. That will provide some additional value for sure, but it will be for a future release (3.1 for instance?).

On the technical point of view, there is a GitHub Actions workflow that triggers on tag push and that we could test to release the version 2.2.4 if my memory is right, so I think we are ready.

What do you think about that?

Multiple users of the plugin asked us when this version will be released. And the current README file explains how to use the Kotlin rewrite version whereas it is not yet released. Not very good user experience.

Have a nice day!

[deepy]

This weekend I can merge the dokka gh-pages branch and tag 3.0.0-rc1
I had hoped to get further with configuration caching before that but time wasn't on my side

[bsautel]

Ok, nice. Thanks for your answer!

Since we automated the release process, if the configuration caching feature is ready in a few weeks, that's not a big deal, we will be able to release a 3.1 version. I don't have much time currently to have a look to this configuration caching topic.

Have a nice week-end and I'll have a look at the rc1 release.

[deepy]

Thank you. Your new plugin com.github.node-gradle.node has been submitted for approval by Gradle engineers. The request should be processed within the next few days, at which point you will be contacted via email.

I posted a question on the gradle community slack https://gradle-community.slack.com/archives/CA745PZHN/p1598133269008800

I can see the PENDING note next to the plugin if I login to the plugin portal so it's going appear correctly eventually

[bsautel]

Thanks for this work!

Hope it will be available soon! As far as I remember, the version 2.2.4 was published immediately, no?

Is because there was a publication policy change? Or because it is a major version number? 🤔

[deepy]

I think it's a bug with the plugin portal and that it doesn't recognize the plugin and I'm worried we might have the same issue if we try to publish anything from 2.x after 3.x has been approved

[bsautel]

Ok. Let's wait for a response, they say they will contact you in a few days. And maybe this will be faster in the Gradle community Slack. Hope this will be solved soon! 🤞

And do you know why the Build and Build Examples GitHub Actions failed with this error message?

An exception occurred applying plugin request [id: 'com.cinnober.gradle.semver-git', version: '3.0.0']
> Failed to apply plugin 'com.cinnober.gradle.semver-git'.
   > index is out of range 0..-1 (index = 0)

[deepy]

I can't reproduce it locally, I think something in server is happening on the tag, but as it builds fine with the tag locally I'm not sure where it fails, I've pushed another branch and that worked fine so not sure if we need to do anything

[bsautel]

I also can't reproduce it locally and indeed, it builds on the master branch. I probably has to do with the tag. The only other Gradle build that is triggered when pushing a tag is the Publish one and it triggers on new tag and not on new commit on any branch and this one works.

It would be nice if it could work but that's not too much annoing since we can push to master, wait for the build to be successful, and then push the tag, which will trigger the release.

[deepy]

rc1 available on the plugin portal now 🎉

[bsautel]

Nice!

I just upgraded a project in which I use the plugin and I noticed one issue: the group id of the plugin changed. It used to be com.github.node-gradle and is gradle.plugin.com.github.node-gradle now.

It is not visible if you load the plugin in the plugins block, but it is if you add the classpath dependency to the buildscript. It is the reason why I noticed it.

I don't know why, I don't believe we changed something related to that and noticed that there are many plugins published with this prefix. This may be due to a recent change in the Gradle plugin publish plugin? I'll investigate that.

If this is the new standard, I will update the documentation and add this to the breaking changes.

[deepy]

Looking at the buildscripts it seems like we dropped the mavenCoordinates part of the pluginBundle which would explain why this changed and why we had to get it re-approved, I'll push a fix after I verify it

[deepy]

I guess this makes for our quickest rc2 :^)

[bsautel]

Hum, it sounds like I deleted that, probably because it was redundant with the group and project name declaration. I should have read the docs before. Sorry, and thanks for the fix!

I'll test the rc2 once it will be published.

And this is probably the reason why the publication needed to be validated. The rc1 was published under a group that did not exist.

[deepy]

rc2 is out

[bsautel]

Thanks and I confirm you that it works as expected! Nice! 🎉

[bsautel]

I have been using this rc2 version since its release and it's working well for me. I think we can release it as a stable version, unless some issues were discovered by anybody?

I wrote a release checklist document to ensure we don't forget anything. @deepy if you think I forgot anything, please tell me or directly add it to the file.

[deepy]

Looks good to me!

Testing out the automatic proxy support in 3 I'm seeing some mixed results, namely that in reality most seem to only use HTTP proxies and as we currently put HTTPS_PROXY as https:// that autoconfigration is not working.

I have some additional checks to do in regards to this but it should be pretty quick and hopefully we can create the release today.

[bsautel]

Regarding the proxy support, I think I did not understand what you mean. We configure npm nearly the same way as Gradle is configured. If an HTTP proxy is set, we set a HTTP proxy, if an HTTPS proxy is set, we set an HTTPS proxy, if both are set, we set both.

I wrote nearly because there is at least one thing that we cannot reproduce. It is possible to ignore the proxy for a host name or a host name + a port in Gradle, and in npm we can only ignore the proxy for a host name, so we remove the ports from the Gradle proxy ignore configuration. We probably should explain that in the documentation.

[ntkoopman]

Proxy support is also not working for me because it's replacing the protocol here

https://github.com/node-gradle/gradle-node-plugin/blob/master/src/main/kotlin/com/github/gradle/node/npm/proxy/NpmProxy.kt#L25

[bsautel]

Could you show me your configuration please? Would you be using a HTTP URL for a HTTPS proxy or the opposite? If it is the case, is this supported by npm and yarn?

I added a small paragraph regarding the proxy in the usage page.

[deepy]

At work we use HTTP_PROXY and HTTPS_PROXY both pointing to a http:// url and the way that tools seem to handle this is by using the value of HTTPS_PROXY if they connect to a https:// address and HTTP_PROXY for http:// addresses.

As for Gradle we do

systemProp.http.proxyHost=proxy20.example.com
systemProp.http.proxyPort=8080
systemProp.https.proxyHost=proxy20.example.com
systemProp.https.proxyPort=8080

I've reached out to some friends who are also forced to use big enterprisey proxy setups but haven't got an answer yet.

But one thing I'm considering is that maybe we should check for the presence of NO_PROXY, HTTP_PROXY, and HTTPS_PROXY and not do any additional configuration if any of them are present.

[ntkoopman]

That is similar to our configuration.

systemProp.http.proxyUser={username}
systemProp.http.proxyPassword={password}
systemProp.https.proxyUser={username}
systemProp.https.proxyPassword={password}
systemProp.http.proxyHost=proxy.vic
systemProp.https.proxyHost=proxy.vic
systemProp.http.nonProxyHosts=localhost|127.*|[::1]|*.vic|*.corp|10.*|172.16.*|172.17.*|172.18.*|172.19.*|172.20.*|172.21.*|172.22.*|172.23.*|172.24.*|172.25.*|172.26.*|172.27.*|172.28.*|172.29.*|172.30.*|172.31.*|192.168.*|172.254.248.*|172.254.249.*|172.254.25*
systemProp.https.nonProxyHosts=localhost|127.*|[::1]|*.vic|*.corp|10.*|172.16.*|172.17.*|172.18.*|172.19.*|172.20.*|172.21.*|172.22.*|172.23.*|172.24.*|172.25.*|172.26.*|172.27.*|172.28.*|172.29.*|172.30.*|172.31.*|192.168.*|172.254.248.*|172.254.249.*|172.254.25*
systemProp.https.proxyPort=80

Environment variables are used in our default config because of the inconsistent support between tools. Note that e.g. curl only accepts the lowercase version:

export HTTP_PROXY="http://username:[email protected]:80"
export HTTPS_PROXY="$HTTP_PROXY"
export http_proxy="$HTTP_PROXY"
export https_proxy="$HTTP_PROXY"

[deepy]

@bsautel I think in addition to my previous comment we should probably default to http, it doesn't seem like many organisations use that.
(which probably doesn't matter when you're only using CONNECT and mitm-ing your users with your own CA)

[bsautel]

Thanks for all these precisions.

1. Indeed, @deepy, it is probably not relevant to configure the proxy when it is already configured globally thanks to the environment variable we define (and thus override and potentially break the existing configuration).
2. For now we only support HTTP proxy for HTTP requests and HTTPS proxy for HTTPS request, and that does not seem to be your case. But, according to the configuration example you showed us @deepy and @ntkoopman, how can we guess the protocol we should use? How can Gradle know whether it must use HTTP or HTTPS?
3. @ntkoopman you indicated that the problem is coming from the fact we remove the protocol. This is maybe a way to indicate which protocol to use, but in the two examples above, the protocol is not defined.
4. @deepy you mean that we should probably always connect to proxies using HTTP and never HTTPS?

Personally I don't know very well this topic, I have never had to use an HTTP proxy. I guess I am lucky! 😁

[deepy]

I don't know how Java picks the correct protocol, but I suspect it simply uses HTTP for both of them.

So for context, HTTP has a bunch of useful verbs like GET, HEAD, PUT, POST, DELETE. With HTTP proxies you're making a HTTP request with CONNECT, asking the server to create a connection to somewhere else for you.

In the case of big corporate proxies you usually force everyone to go through a proxy which creates fake SSL certificates signed by an internal CA and then pretend that the website presented you with that certificate instead of their real one, effectively doing a man-in-the-middle attack on their own employees to be able to access their traffic.
But since they're presenting a certificate of their own, the connection to the proxy doesn't technically need to be encrypted as the connection to whatever site you're connecting to through the proxy will be encrypted (by the proxy) later on.

So we should probably always use http:// for both HTTP_PROXY and HTTPS_PROXY

[deepy]

#120 contains fixes for npm, still needs some final things for yarn and also need to see that the tests pass.

[deepy]

#120 is ready for review, I'll start working on #119 tomorrow and then we can hopefully release the final RC soon 😅

[deepy]

3.0.0-rc3 is out with improved proxy handling, use http for both HTTP_PROXY and HTTPS_PROXY and don't configure the proxy settings if there's already proxy settings present in the environment variables.

There were a bit too many failing tests in #119 for me to get that in time for rc3, but now the proxy functionality should be done.

[bsautel]

Thanks @deepy! Really happy that the 3.0 final release is very close! Tell me if you need some help regarding the failing tests in #119.

[cdietrich]

do you have any idea why

script = project.file('node_modules/requirejs/bin/r.js')

is no longer working? and i need to call script.set() now

Cannot cast object '/xxxxxxxx/node_modules/requirejs/bin/r.js' with class 'java.io.File' to class 'org.gradle.api.file.RegularFileProperty'

[bsautel]

Yes, this version 3.0 contains multiple breaking changes that are explained in the changelog. The issue you are facing is explained in the second item of the breaking changes. It has to do with Gradle lazy properties.

[deepy]

I seem to remember there being something strange happening when we tried to add convenience methods for the kotlin users.

And I recently saw this appear in the gradle community slack, which sounds familiar gradle/gradle#13623

But I think at this point we have better test coverage, so it shouldn't be hard to give it another try.
And pushing it as another rc would work as well

[deepy]

Having used kotlin buildscripts more lately I think the .set() syntax seems to be the expected way to deal with extensions, or at least the plugins I've used required me to do that.

But with proxy support fixed and those other issues that popped up right after also fixed, I think we should probably release 3.0 now before I accidentally end up creating another RC that really could've been a patch.

There's some things left though:

• Add flag to disable task rule creation for 3.x #119 - We could probably do this pretty easy by moving the condition into the rule itself, e.g. don't create the task unless enableTaskRules = true
• Add support for Gradle configuration cache #111 - There's PR Configuration cache support #128 which I think might work, but since it breaks the non-integration tests I'm reluctant to do it, I really really wanted to get it done in time for 3.0
• Respect gradle offline mode #59 - It's a bit niche, but should hopefully be pretty simple and not break anything.
• Support musl - npm install on alpine is failing #51 - Since alpine linux is pretty popular in docker I imagine this would be good to work on at some point.

But I think we can probably save those for later and release 3.0 now, what do you think @bsautel?

[deepy]

Adding some chores I need to do before 3.0 can be released

• Check all the documentation (reminder to slef: enable spellchecker)
  • Document NodeExtension with KDoc
    • Document npmCommand
    • Document npxCommand
    • Document yarnCommand
    • Document cacheDir
  • Document Tasks with KDoc (we can probably skip @Internals)
    • Or at least the ones that the users might configure
    • NodeTask
    • NpmTask
    • NpxTask
    • YarnTask
  • Disable macOS spellchecker and make sure npm hasn't been replaced by nom
• Check if dokka documentation should be improved (e.g. do we need to add anything to classes?)
  ❌ Check if we can create a fancy pages like https://docs.gradle.org/6.7.1/dsl/org.gradle.api.Project.html with dokka
  • Updating dokka triggered a bug in the javadoc package-tree.html and package-overview.html missing in javadoc after upgrading to 1.4.20 Kotlin/dokka#1692
• Check if any dependencies should be updated
• Update changelog
• Add a link to this new version to the Release History in the README.md file and mark it as the current one
• Add the version and its changes to the CHANGELOG.md file
• Replace all the occurrences of the former latest version by this version in all the docs files
• Commit these changes and Push this Commit

[bsautel]

Hello @deepy and sorry, I was really busy those latest months and did not have any time to contribute to the project.

It sounds like we can indeed release the 3.0 version. None of the issues you are talking about seems to require a backward compatibility break, so we will be able to introduce them progressively in the future 3.x releases.

I'll try to have a look in the coming days to issue #111 and PR #128.

[deepy]

Hey @bsautel, no worries, time is the most valuable resource all of us have and while we appreciate all time we get since this is volunteer driven it's expected that life will happen. And life should come first, always take any time you need, there's no rush here :-)

To properly finish #111 we probably need to break backwards compatibility by moving the IvyRepository addition from task execution to plugin apply, which I think means we need to re-add afterEvaluate :-(

[bsautel]

I could have a look to #111. I could go one step further, but we still need to add some unit tests to ensure a better coverage of the configuration cache option.

Regarding the ivy repository, the problem is because the repository is added to the project at the beginning of the nodeSetup task, right? As far as I understand, using configuration cache prevents us from using the Project in tasks and thus from declaring the repository. We have to do it elsewhere, probably in the plugin code, but as it is conditional, we must wait for the configuration to be done, and thus use the afterEvaluate hook. Is this correct?

[deepy]

Awesome work! Yeah, that's the way I'm reading it, we're not allowed to touch the project object during task execution so afterEvaluate and then adding the IvyRepository in there would be the only way to get the node downloading to work.

Though from memory, with #128 we're about 3/4 of the way to fully(-ish?) support it!

[bsautel]

As far as I remember, we used to use the afterEvaluate hook to configure the tasks created by the plugin. This was not compatible with the lazy configuration mechanism, since the tasks are created lazily after the project initialization (so after the afterEvaluate hook), so we had to remove it. That does not mean that using afterEvaluate is always evil, and it sounds like declaring a repository is a valid use case of this hook.

@deepy, I fixed the unit tests after you moved the repository plugin declaration from the task to the plugin.

[deepy]

Yeah, I'd been avoiding afterEvaluate on account of that, but when I stepped back to think about it lazy task configuration shouldn't really depend on the project state (except the task configuration) so I think we might be fine here, I had to push the branch as my laptop was melting and giving me strange errors that seemed tied to my environment.

I'll see if I can fix the last test right now

[deepy]

Oh, no, that was a legitimate test failure and you fixed it 🌟

[bsautel]

@deepy do you want we change the proxy configuration boolean to an enum as you suggested in #122? This would be more flexible in the future if we need to make some adjustments.
I also would be in favor to upgrade the default Node.js version to the latest 14 which is now the LTS version.
And after that I think we will be able to finally release this great new version!

[deepy]

3.0.0-rc7 is now out, it has all the nice features and hopefully no configuration-cache bugs 🎉

[bsautel]

If my commit is satisfying to close #134 it sounds like we can release the 3.0.0 version.

[deepy]

3.0.0 is now released 🎉
virtual release party anyone :-)?

[Sineaggi]

Is this issue ready to be closed?