Added support for Gradle 6.+

rpalcolea commented 4 years ago

Gradle 6 is around the corner and there are multiple deprecations that need to be addresses in order to work with it. This PR fixes the following:

Remove usage of maven plugin

The maven plugin has been deprecated. This is scheduled to be removed in Gradle 7.0. Please use the maven-publish plugin instead.

*Upgrade artifactory plugin to stop using non-existing Gradle's internal APIs

Caused by: java.lang.NoSuchMethodError: org.gradle.api.publish.maven.internal.publication.MavenPublicationInternal.getPublishableFiles()Lorg/gradle/api/file/FileCollection;
        at org.jfrog.gradle.plugin.artifactory.task.helper.TaskHelperPublications.checkDependsOnArtifactsToPublish(TaskHelperPublications.java:140)
        at org.jfrog.gradle.plugin.artifactory.task.ArtifactoryTask.checkDependsOnArtifactsToPublish(ArtifactoryTask.java:55)
        at org.jfrog.gradle.plugin.artifactory.task.BuildInfoBaseTask.projectsEvaluated(BuildInfoBaseTask.java:189)
        at org.jfrog.gradle.plugin.artifactory.task.BuildInfoBaseTask$projectsEvaluated.call(Unknown Source)
        at org.jfrog.gradle.plugin.artifactory.extractor.listener.ProjectsEvaluatedBuildListener$_afterEvaluate_closure1.doCall(ProjectsE

Move to implementation and testImplementation configurations

The compile configuration has been deprecated for dependency declaration. This will fail with an error in Gradle 7.0. Please use the implementation or api configuration instead.
        at build_ao06dlp02sqp83bf447p0wztv$_run_closure3.doCall(/Users/rperezalcolea/Projects/github/srs/gradle-node-plugin/build.gradle:31)
        (Run with --stacktrace to get the full stack trace of this deprecation warning.)
The testCompile configuration has been deprecated for dependency declaration. This will fail with an error in Gradle 7.0. Please use the testImplementation configuration instead.
        at build_ao06dlp02sqp83bf447p0wztv$_run_closure3.doCall(/Users/rperezalcolea/Projects/github/srs/gradle-node-plugin/build.gradle:32)
        (Run with --stacktrace to get the full stack trace of this deprecation warning.)

Annotate tasks properly. validateTaskProperties will fail builds on Gradle 6.0

Task property validation finished with warnings:
  - Warning: Type 'com.moowork.gradle.node.npm.NpmSetupTask': property 'args' is not annotated with an input or output annotation.
  - Warning: Type 'com.moowork.gradle.node.npm.NpmSetupTask': setter method 'setArgs()' should not be annotated with: @Internal
  - Warning: Type 'com.moowork.gradle.node.yarn.YarnSetupTask': property 'args' is not annotated with an input or output annotation.

Use patternLayout instead of layout 'pattern' which is removed on Gradle 6.0. Kept backwards compatibility here.

The IvyArtifactRepository.layout(String, Closure) method has been deprecated. This is scheduled to be removed in Gradle 6.0. Please use the IvyArtifactRepository.patternLayout(Action) method instead.
        at org.gradle.api.internal.artifacts.repositories.DefaultIvyArtifactRepository.layout(DefaultIvyArtifactRepository.java:302)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:326)
        at org.gradle.internal.extensibility.MixInClosurePropertiesAsMethodsDynamicObject.tryInvokeMethod(MixInClosurePropertiesAsMethodsDynamicObject.java:33)
        at com.moowork.gradle.node.task.SetupTask$_addRepository_closure5.doCall(SetupTask.groovy:164)

rpalcolea commented 4 years ago

Hi @srs , Let me know your thoughts on this one

deepy commented 4 years ago

This is related to #316

dpeger commented 4 years ago

@rpalcolea have you been able to resolve NodeJS with a current Gradle 6 build? See https://github.com/srs/gradle-node-plugin/issues/351

rpalcolea commented 4 years ago

Hey @dpeger, havent; try it out yet but this PR fixes that in https://github.com/srs/gradle-node-plugin/pull/349/commits/708f62b4fd1bf36d712aae6dbb15ad0186d2dda2

What happened between Gradle 5.x and 6.x is that artifact() is not part of the default metadataSources for Ivy/Maven repos:

https://github.com/gradle/gradle/blob/v5.6.2/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/DefaultIvyArtifactRepository.java#L433 vs https://github.com/gradle/gradle/blob/1c3b4cd7d648d68767c70e9deb62e77c1ecb5198/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/DefaultIvyArtifactRepository.java#L445

What that means is that the default repo created by SetupTask on this plugin will only pull ivy files that's why you get

Searched in the following locations:
       - https://nodejs.org/dist/v10.15.3/ivy.xml

Actually, the original behavior probably caused tons of 404s for the XML file as artifact was the second in the order.

If you look and how gradle configures their own javascript repos, all of the above might make sense: https://github.com/gradle/gradle/blob/b189979845c591d8c4a0032527383df0f6d679b2/subprojects/javascript/src/main/java/org/gradle/plugins/javascript/base/JavaScriptRepositoriesExtension.java#L53

The docs in https://docs.gradle.org/current/userguide/repository_types.html#sub:supported_metadata_sources are a little misleading:

The defaults for Ivy and Maven repositories change with Gradle 5.0. Before 5.0, artifact() was included in the defaults. Leading to some inefficiency when modules are missing completely. To restore this behavior, for example, for Maven central you can use mavenCentral { mavenPom(); artifact() }. In a similar way, you can opt into the new behavior in older Gradle versions using mavenCentral { mavenPom() }

But in reality, the breaking change happens in 6.x

This is totally unrelated to the patternLayout change

Btw, the fork of this already contains those fixes: https://github.com/node-gradle/gradle-node-plugin/blob/master/src/main/groovy/com/moowork/gradle/node/task/SetupTask.groovy

In our case, we would like to get this plugin fixed as it might be a taunting task to ask tens/hundreds of engineers to migrate to another plugin

rpalcolea commented 4 years ago

Hi @srs , any chance you could take a look on this PR? :)

MorrisonCole commented 4 years ago

With Gradle 6.0 now stable, the resolution issue mentioned above is now quite a major problem 👀

rpalcolea commented 4 years ago

agree, hopefully @srs can take a look at this soon

deepy commented 4 years ago

There hasn't been a commit or reaction in 8 months, if you're able to live without the Gulp and the Grunt parts of this I'd use the fork.

alexvas commented 4 years ago

Hi @rpalcolea, did you think about publishing your fork somewhere?

deepy commented 4 years ago

@alexvas do you need grunt/gulp in the plugin? Otherwise node-gradle/gradle-node-plugin

alexvas commented 4 years ago

@deepy thank you. Your plugin just works for me. It's version 2.2.0 works with Gradle 6.0.1 without any warning.

rpalcolea commented 4 years ago

we do have projects using com.moowork.grunt

wlsc commented 4 years ago

Can we merge this PR please? Support of Gradle 6.x is very needed.

rpalcolea commented 4 years ago

Hi @wlsc , it seems that this project is abandoned as we haven't heard back from plugin author in a while.

If you don't need gulp/grunt, I'd suggest to move to https://github.com/node-gradle/gradle-node-plugin since it is actively maintained these days and it's a fork from this

srs / gradle-node-plugin

Added support for Gradle 6.+ #349