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

Update publication artifact collection logic #306

Closed
wants to merge 5 commits into from

Conversation

ljacomet
Copy link

@ljacomet ljacomet commented Dec 23, 2019

This aligns the artifact collection logic with the one from the Artifactory plugin and enables deploying Gradle Module Metadata files to Bintray.

This PR replaces #230 by leveraging APIs introduced in Gradle 4.8, which becomes the minimum requirement.

Tests have been run with the default setup and a GRADLE_HOME set to version 6.0.1 to verify the upload of *.module files.

@ljacomet ljacomet changed the title Update publicaton artifact collection logic Update publication artifact collection logic Dec 23, 2019
@ljacomet
Copy link
Author

@eyalbe4 Any chance you can have a look at this PR?

logger.info "{} can only use maven publications - skipping {}.", path, publication.name
return []
}
def artifacts = publication.artifacts.findResults {
boolean signedArtifact = (it instanceof org.gradle.plugins.signing.Signature)
def signedExtension = signedArtifact ? it.toSignArtifact.getExtension() : null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question - With the new API, we no longer need this?
This code checks whether this is artifact is the signature file, and adds it to the list of uploaded artifact,s with the signature extension.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toSignArtifact was deprecated and removed, this PR would also fix #300

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eyalbe4 so, to answer your question: yes, we no longer need this.
In fact the current code is wrong as currently publishing signed artifacts does not work at all (#300 ).
Would be great to have this merged and released.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm as well that these changes still properly publish the signature files.

@ljacomet
Copy link
Author

ljacomet commented May 4, 2020

I'll try to take a look and resolve the conflicts in that PR later this week.

This enables the upload of Gradle Module Metadata when generated.
The APIs used require Gradle 4.8 at the minimum.

Fixes bintray#229
These are not required and even an error with recent Gradle versions.
@ljacomet
Copy link
Author

ljacomet commented May 5, 2020

@eyalbe4 I have updated the PR to the latest master. I ran tests again and all passed.

As a reminder, this PR uses API that appeared in Gradle 4.8 and thus will not work with a lower Gradle version.
As I also commented on other PRs in the JFrog ecosystem, there is no guarantee that a plugin built with version 5.6.2 will work with lower Gradle version, which is why my PR originally bumped Gradle only to 4.10. But I see that master has moved to 5.6.2 since then.

@SgtSilvio
Copy link

@eyalbe4 any plans when this will be merged?

@eyalbe4
Copy link
Contributor

eyalbe4 commented May 26, 2020

@ljacomet,
When running the tests after pulling the code of this pull request, some of the tests are failing with the following exception:

    Caused by: java.lang.NoClassDefFoundError: org/apache/maven/project/MavenProject
        at com.jfrog.bintray.gradle.Utils.readArtifactIdFromPom(Utils.groovy:46)
        at com.jfrog.bintray.gradle.Utils$readArtifactIdFromPom$1.call(Unknown Source)
        at com.jfrog.bintray.gradle.tasks.BintrayUploadTask.collectArtifacts(BintrayUploadTask.groovy:495)
        at com.jfrog.bintray.gradle.tasks.BintrayUploadTask$_bintrayUpload_closure1.doCall(BintrayUploadTask.groovy:202)
        at com.jfrog.bintray.gradle.tasks.BintrayUploadTask.bintrayUpload(BintrayUploadTask.groovy:198)
        at org.gradle.internal.reflect.JavaMethod.invoke(JavaMethod.java:103)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.doExecute(StandardTaskAction.java:49)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:42)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:28)
        at org.gradle.api.internal.AbstractTask$TaskActionWrapper.execute(AbstractTask.java:717)
        at org.gradle.api.internal.AbstractTask$TaskActionWrapper.execute(AbstractTask.java:684)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter$5.run(ExecuteActionsTaskExecuter.java:476)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:402)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:394)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$1.execute(DefaultBuildOperationExecutor.java:165)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:250)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:158)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:92)
        at org.gradle.internal.operations.DelegatingBuildOperationExecutor.run(DelegatingBuildOperationExecutor.java:31)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeAction(ExecuteActionsTaskExecuter.java:461)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:444)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.access$200(ExecuteActionsTaskExecuter.java:93)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter$TaskExecution.execute(ExecuteActionsTaskExecuter.java:237)
        at org.gradle.internal.execution.steps.ExecuteStep.lambda$execute$1(ExecuteStep.java:33)
        at org.gradle.internal.execution.steps.ExecuteStep.execute(ExecuteStep.java:33)
        at org.gradle.internal.execution.steps.ExecuteStep.execute(ExecuteStep.java:26)
        at org.gradle.internal.execution.steps.CleanupOutputsStep.execute(CleanupOutputsStep.java:58)
        at org.gradle.internal.execution.steps.CleanupOutputsStep.execute(CleanupOutputsStep.java:35)
        at org.gradle.internal.execution.steps.ResolveInputChangesStep.execute(ResolveInputChangesStep.java:48)
        at org.gradle.internal.execution.steps.ResolveInputChangesStep.execute(ResolveInputChangesStep.java:33)
        at org.gradle.internal.execution.steps.CancelExecutionStep.execute(CancelExecutionStep.java:39)
        at org.gradle.internal.execution.steps.TimeoutStep.executeWithoutTimeout(TimeoutStep.java:73)
        at org.gradle.internal.execution.steps.TimeoutStep.execute(TimeoutStep.java:54)
        at org.gradle.internal.execution.steps.CatchExceptionStep.execute(CatchExceptionStep.java:35)
        at org.gradle.internal.execution.steps.CreateOutputsStep.execute(CreateOutputsStep.java:51)
        at org.gradle.internal.execution.steps.SnapshotOutputsStep.execute(SnapshotOutputsStep.java:45)
        at org.gradle.internal.execution.steps.SnapshotOutputsStep.execute(SnapshotOutputsStep.java:31)
        at org.gradle.internal.execution.steps.CacheStep.executeWithoutCache(CacheStep.java:208)
        at org.gradle.internal.execution.steps.CacheStep.execute(CacheStep.java:70)
        at org.gradle.internal.execution.steps.CacheStep.execute(CacheStep.java:45)
        at org.gradle.internal.execution.steps.BroadcastChangingOutputsStep.execute(BroadcastChangingOutputsStep.java:49)
        at org.gradle.internal.execution.steps.StoreSnapshotsStep.execute(StoreSnapshotsStep.java:43)
        at org.gradle.internal.execution.steps.StoreSnapshotsStep.execute(StoreSnapshotsStep.java:32)
        at org.gradle.internal.execution.steps.RecordOutputsStep.execute(RecordOutputsStep.java:38)
        at org.gradle.internal.execution.steps.RecordOutputsStep.execute(RecordOutputsStep.java:24)
        at org.gradle.internal.execution.steps.SkipUpToDateStep.executeBecause(SkipUpToDateStep.java:96)
        at org.gradle.internal.execution.steps.SkipUpToDateStep.lambda$execute$0(SkipUpToDateStep.java:89)
        at org.gradle.internal.execution.steps.SkipUpToDateStep.execute(SkipUpToDateStep.java:54)
        at org.gradle.internal.execution.steps.SkipUpToDateStep.execute(SkipUpToDateStep.java:38)
        at org.gradle.internal.execution.steps.ResolveChangesStep.execute(ResolveChangesStep.java:76)
        at org.gradle.internal.execution.steps.ResolveChangesStep.execute(ResolveChangesStep.java:37)
        at org.gradle.internal.execution.steps.legacy.MarkSnapshottingInputsFinishedStep.execute(MarkSnapshottingInputsFinishedStep.java:36)
        at org.gradle.internal.execution.steps.legacy.MarkSnapshottingInputsFinishedStep.execute(MarkSnapshottingInputsFinishedStep.java:26)
        at org.gradle.internal.execution.steps.ResolveCachingStateStep.execute(ResolveCachingStateStep.java:90)
        at org.gradle.internal.execution.steps.ResolveCachingStateStep.execute(ResolveCachingStateStep.java:48)
        at org.gradle.internal.execution.steps.CaptureStateBeforeExecutionStep.execute(CaptureStateBeforeExecutionStep.java:69)
        at org.gradle.internal.execution.steps.CaptureStateBeforeExecutionStep.execute(CaptureStateBeforeExecutionStep.java:47)
        at org.gradle.internal.execution.impl.DefaultWorkExecutor.execute(DefaultWorkExecutor.java:33)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:140)
        ... 34 more

@ljacomet
Copy link
Author

ljacomet commented Jun 16, 2020

@eyalbe4 Indeed, my setup to run the project test was invalid and so I did not test what I wanted.
However the same issue is present on the main branch, so my changes are not the reason for the breakage.

I again confirmed with a minimal reproducer that this PR allows to upload Gradle Module Metadata when it is produced.

@eyalbe4
Copy link
Contributor

eyalbe4 commented Jun 16, 2020

@ljacomet,
I ran the tests on the master branch and also on this PR's branch. It loos like the tests are failing on this branch only.
Are you saying that you get the same error when running the tests on the master branch?

@ljacomet
Copy link
Author

Let me try again and describe the steps I am taking.

@ljacomet
Copy link
Author

At commit 67718c3, I then do the following:

  • rm -rf ~/.m2/repository/com/jfrog
  • export BINTRAY_USER=<my user>
  • export BINTRAY_KEY=<my key>
  • ./gradlew publishToMavenLocal
  • ./gradlew test

And the result is

Test Duration Result
[configurationWithSubModules]create package and version with configuration 6.805s failed
[configuration]create package and version with configuration 3.032s failed
[fileSpec]create debian package and version with fileSpec 19.202s passed
[fileSpec]create package and version with fileSpec 8.482s passed
[publicationWithJavaGradlePlugin]create package and version with publication 43.336s passed
[publication]create package and version with publication 42.629s passed
[publication]override 33.436s passed
debian package indexed 0.203s passed

@eyalbe4
Copy link
Contributor

eyalbe4 commented Jun 16, 2020

I suggest you use run the tests as described here. You should run them on the master branch, which means running the tests with the latest code.
Please let me know if this works for you.

@ljacomet
Copy link
Author

There is something I do not understand there.

  1. 67718c3 is current master
  2. AFAICT running the tests as documented in your link only runs them against released version of the plugin, not the one just built from source.

PluginSpecUtils.getGradleCommandPath uses the CLI to invoke Gradle for the test projects. All the embedded test projects have

buildscript {
    repositories {
        mavenLocal()
        jcenter()
    }
    dependencies {
        classpath "com.jfrog.bintray.gradle:gradle-bintray-plugin:1.8.5"
    }
}

which will only resolve the plugin from real repositories. There is no logic that consumes the plugin code from the project.
And there is no wiring of a task that would put the built plugin in the maven local repo as a dependency on build or test.

An explicit dependency on maven-core is required
to support configuration based projects.
Modern configuration are also used now.

These changes have been tested against Gradle
4.10.3, 5.6.2 and 6.5.
* Get rid of using clean
* Explain how to test a modified plugin
@ljacomet
Copy link
Author

I have pushed two new commits:

@eyalbe4 Could you take another look?

@CLAassistant
Copy link

CLAassistant commented Jun 18, 2020

CLA assistant check
All committers have signed the CLA.

@ljacomet
Copy link
Author

@eyalbe4 Any news on this? Is there something missing to get it included?

It would be great for Gradle builds using this plugin to have the ability to publish Gradle Module Metadata to Bintray.

implementation('org.apache.maven.resolver:maven-resolver-ant-tasks:1.2.0')
implementation('org.apache.maven:maven-core:3.0.5')

testImplementation('org.spockframework:spock-core:0.7-groovy-2.0') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be a good idea to update this dependency to a version that closely matches the Groovy version provided by gradleApi(), after all Spock is very sensitive to Groovy AST changes

@eyalbe4
Copy link
Contributor

eyalbe4 commented Jun 24, 2020

@ljacomet,

I double checked and this PR is definitely failing the tests with Caused by: java.lang.NoClassDefFoundError: org/apache/maven/project/MavenProject.
Here's what you need to do to run the tests with the new code:

  1. Change the version in the gradle,properties to a snapshot version of your choosing.
  2. Run gradle clean install to install the plugin with the new code and new version in your local maven repository.
  3. Replace the new version in the following build.gradle files, used by the various tests:
/Users/eyalb/dev/gradle-bintray-plugin/src/test/resources/gradle/projects/configuration/build.gradle
/Users/eyalb/dev/gradle-bintray-plugin/src/test/resources/gradle/projects/configurationWithSubModules/build.gradle
/Users/eyalb/dev/gradle-bintray-plugin/src/test/resources/gradle/projects/fileSpec/build.gradle
/Users/eyalb/dev/gradle-bintray-plugin/src/test/resources/gradle/projects/publication/build.gradle
/Users/eyalb/dev/gradle-bintray-plugin/src/test/resources/gradle/projects/publicationWithJavaGradlePlugin/build.gradle
  1. Run the tests as described in the README.

@ljacomet
Copy link
Author

@eyalbe4 Which git commit did you try?

The last version of this PR no longer fails any tests here. I just tried again with adding custom logging, which I then see in the test output. So 🤷‍♂️

I can also change the test setup so that neither my instructions nor yours are required anymore:

  • Make the version used in the test a param that is valued from the currentVersion property
  • Add a custom deploy task that uses a local repository under build
  • Replace mavenLocal() in the tests by this local repository
  • Add a dependency of the test task to the required deploy task

Then running tests with the modified code could simply be ./gradlew test

Optionally, there could even be another gradle property controlling the plugin version used in test so that you could test against any released plugin version.

But honestly before doing more work, I need to understand why we see different test results, on this PR or on master.

compileOnly('org.apache.maven:maven-project:2.0.11')
testRuntime('org.apache.maven:maven-project:2.0.11')
testCompile('org.spockframework:spock-core:0.7-groovy-2.0') {
implementation('org.apache.maven.resolver:maven-resolver-ant-tasks:1.2.0')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd exchange implementation for api (except for gradleApi()) as those dependencies are required for compilation & consumers that extend the plugin

@ljacomet
Copy link
Author

Given the sunsetting of Bintray/JCenter, this no longer matters.

@ljacomet ljacomet closed this Mar 16, 2021
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.

5 participants