Copilot commented on code in PR #34:
URL:
https://github.com/apache/grails-gradle-publish/pull/34#discussion_r3551167011
##########
plugin/src/main/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePlugin.groovy:
##########
@@ -519,7 +446,227 @@ Note: if project properties are used, the properties must
be defined prior to ap
}
}
- protected void setDependencyVersions(Node pomNode, Project project) {
+ /**
+ * Configures the pom metadata shared by every publication (primary and
additional) from the
+ * extension, with the title, description, and pom customization supplied
per publication.
+ */
+ protected void configurePom(Project project, GrailsPublishExtension gpe,
MavenPublication publication,
+ Provider<String> title, Provider<String> desc,
Provider<Closure> pomCustomization,
+ List<String> versionResolutionConfigurations) {
+ publication.pom { MavenPom pom ->
+ pom.name.set(title.get())
+ pom.description.set(desc.get())
+ pom.url.set(gpe.websiteUrl.get())
+
+ def organization = gpe.organization
+ if (organization.name.isPresent() || organization.url.isPresent())
{
+ pom.organization { org ->
+ if (organization.name.isPresent()) {
+ org.name.set(organization.name)
+ }
+ if (organization.url.isPresent()) {
+ org.url.set(organization.url)
+ }
+ }
+ }
+
+ def license = gpe.license
+ if (license) {
+ def concreteLicense = License.LICENSES.get(license.name)
+ if (concreteLicense) {
+ pom.licenses { MavenPomLicenseSpec licenses ->
+ licenses.license { MavenPomLicense pomLicense ->
+ pomLicense.name.set(concreteLicense.name)
+ pomLicense.url.set(concreteLicense.url)
+
pomLicense.distribution.set(concreteLicense.distribution)
+ }
+ }
+ } else if (license.name && license.url) {
+ pom.licenses { MavenPomLicenseSpec licenses ->
+ licenses.license { MavenPomLicense pomLicense ->
+ pomLicense.name.set(license.name)
+ pomLicense.url.set(license.url)
+ pomLicense.distribution.set(license.distribution)
+ }
+ }
+ }
+ } else {
+ throw new RuntimeException(createErrorMessage('license'))
+ }
Review Comment:
The license validation is ineffective: `gpe.license` is always non-null, so
builds with no license name (or an unknown name with no URL) silently publish a
POM without any `<licenses>` section, which is likely to fail repository
requirements (e.g., Maven Central). Consider failing fast unless a known
license is selected or both `name` and `url` are provided.
##########
plugin/src/main/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePlugin.groovy:
##########
@@ -519,7 +446,227 @@ Note: if project properties are used, the properties must
be defined prior to ap
}
}
- protected void setDependencyVersions(Node pomNode, Project project) {
+ /**
+ * Configures the pom metadata shared by every publication (primary and
additional) from the
+ * extension, with the title, description, and pom customization supplied
per publication.
+ */
+ protected void configurePom(Project project, GrailsPublishExtension gpe,
MavenPublication publication,
+ Provider<String> title, Provider<String> desc,
Provider<Closure> pomCustomization,
+ List<String> versionResolutionConfigurations) {
+ publication.pom { MavenPom pom ->
+ pom.name.set(title.get())
+ pom.description.set(desc.get())
+ pom.url.set(gpe.websiteUrl.get())
+
+ def organization = gpe.organization
+ if (organization.name.isPresent() || organization.url.isPresent())
{
+ pom.organization { org ->
+ if (organization.name.isPresent()) {
+ org.name.set(organization.name)
+ }
+ if (organization.url.isPresent()) {
+ org.url.set(organization.url)
+ }
+ }
+ }
+
+ def license = gpe.license
+ if (license) {
+ def concreteLicense = License.LICENSES.get(license.name)
+ if (concreteLicense) {
+ pom.licenses { MavenPomLicenseSpec licenses ->
+ licenses.license { MavenPomLicense pomLicense ->
+ pomLicense.name.set(concreteLicense.name)
+ pomLicense.url.set(concreteLicense.url)
+
pomLicense.distribution.set(concreteLicense.distribution)
+ }
+ }
+ } else if (license.name && license.url) {
+ pom.licenses { MavenPomLicenseSpec licenses ->
+ licenses.license { MavenPomLicense pomLicense ->
+ pomLicense.name.set(license.name)
+ pomLicense.url.set(license.url)
+ pomLicense.distribution.set(license.distribution)
+ }
+ }
+ }
+ } else {
+ throw new RuntimeException(createErrorMessage('license'))
+ }
+
+ pom.scm { MavenPomScm scm ->
+ scm.url.set(gpe.scmUrl.get())
+ scm.connection.set(gpe.scmUrlConnection.get())
+ scm.developerConnection.set(gpe.scmUrlConnection.get())
+ }
+
+ pom.issueManagement { MavenPomIssueManagement issue ->
+ issue.system.set(gpe.issueTrackerName.get())
+ issue.url.set(gpe.issueTrackerUrl.get())
+ }
+
+ if (gpe.developers) {
+ pom.developers { MavenPomDeveloperSpec devs ->
+ for (MavenPomDeveloper source : gpe.developers.get()) {
+ devs.developer { MavenPomDeveloper target ->
+ cloneDeveloper(source, target)
+ }
+ }
+ }
+ } else {
+ throw new RuntimeException(createErrorMessage('developers'))
+ }
Review Comment:
The `developers` requirement check is currently a no-op: `gpe.developers` is
a `ListProperty` and is always truthy, so publishing proceeds even when the
list is empty, producing a POM without developers despite the intent to require
them (via `createErrorMessage('developers')`).
##########
plugin/src/main/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePlugin.groovy:
##########
@@ -599,7 +749,25 @@ Note: if project properties are used, the properties must
be defined prior to ap
return
}
- publication.from project.components.named('java').get()
+ def javaComponent = project.components.named('java').get()
+ if (gpe.additionalPublications) {
+ // Declare the additional publications' components as children of
the primary
+ // component so the project's published components form a single
tree — the only
+ // shape Gradle can resolve a project dependency against when one
project publishes
+ // multiple coordinates (see GrailsRootSoftwareComponent).
+ List<SoftwareComponent> childComponents =
gpe.additionalPublications.collect { AdditionalPublication additional ->
+ String componentName = additional.componentName.get()
+ SoftwareComponent component =
project.components.findByName(componentName)
+ if (component == null) {
+ throw new GradleException("Additional publication
`${additional.name}` of project `${project.name}` requires a software component
named `${componentName}`, but none exists. Create the component (e.g. via
SoftwareComponentFactory.adhoc) before the project is evaluated, or set
`componentName` to an existing component.")
+ }
+ component
+ }
+ publication.from(new
GrailsRootSoftwareComponent((SoftwareComponentInternal) javaComponent,
childComponents))
+ } else {
+ publication.from(javaComponent)
+ }
Review Comment:
`javaComponent` is cast to `SoftwareComponentInternal` without a type check.
If Gradle ever changes the implementation of the `java` component, this will
fail with a `ClassCastException` at publish time. Add a guard that throws a
clear `GradleException` when the component is not `SoftwareComponentInternal`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
