Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for Gradle 6.+ #349

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rpalcolea
Copy link

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
Copy link
Author

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

@deepy
Copy link

deepy commented Sep 20, 2019

This is related to #316

@dpeger
Copy link

dpeger commented Oct 10, 2019

@rpalcolea have you been able to resolve NodeJS with a current Gradle 6 build? See #351

@rpalcolea
Copy link
Author

rpalcolea commented Oct 10, 2019

Hey @dpeger, havent; try it out yet but this PR fixes that in 708f62b

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
Copy link
Author

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

@MorrisonCole
Copy link

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

@rpalcolea
Copy link
Author

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

@deepy
Copy link

deepy commented Nov 11, 2019

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
Copy link

alexvas commented Nov 19, 2019

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

@deepy
Copy link

deepy commented Nov 19, 2019

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

@alexvas
Copy link

alexvas commented Nov 19, 2019

@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
Copy link
Author

we do have projects using com.moowork.grunt

@wlsc
Copy link

wlsc commented Jan 27, 2020

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

@rpalcolea
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants