bintray / gradle-bintray-plugin

Apache License 2.0
1.28k stars 197 forks source link

Update publication artifact collection logic #306

Closed ljacomet closed 3 years ago

ljacomet commented 4 years ago

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 commented 4 years ago

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

ljacomet commented 4 years ago

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

ljacomet commented 4 years ago

@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 commented 4 years ago

@eyalbe4 any plans when this will be merged?

eyalbe4 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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

ljacomet commented 4 years ago

At commit 67718c3, I then do the following:

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 commented 4 years ago

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 commented 4 years ago

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.

ljacomet commented 4 years ago

I have pushed two new commits:

@eyalbe4 Could you take another look?

CLAassistant commented 4 years ago

CLA assistant check
All committers have signed the CLA.

ljacomet commented 4 years ago

@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.

eyalbe4 commented 4 years ago

@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
  4. Run the tests as described in the README.
ljacomet commented 4 years ago

@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:

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.

ljacomet commented 3 years ago

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