-
Notifications
You must be signed in to change notification settings - Fork 197
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
Conversation
@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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
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.
@eyalbe4 I have updated the PR to the latest As a reminder, this PR uses API that appeared in Gradle 4.8 and thus will not work with a lower Gradle version. |
@eyalbe4 any plans when this will be merged? |
@ljacomet,
|
@eyalbe4 Indeed, my setup to run the project test was invalid and so I did not test what I wanted. I again confirmed with a minimal reproducer that this PR allows to upload Gradle Module Metadata when it is produced. |
@ljacomet, |
Let me try again and describe the steps I am taking. |
At commit 67718c3, I then do the following:
And the result is
|
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. |
There is something I do not understand there.
which will only resolve the plugin from real repositories. There is no logic that consumes the plugin code from the project. |
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
I have pushed two new commits:
@eyalbe4 Could you take another look? |
@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') { |
There was a problem hiding this comment.
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
I double checked and this PR is definitely failing the tests with
|
@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 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 |
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') |
There was a problem hiding this comment.
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
Given the sunsetting of Bintray/JCenter, this no longer matters. |
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.