Skip to content

Commit

Permalink
Further simplification of the ZIP publication implementation
Browse files Browse the repository at this point in the history
This is WIP, and I am kindly asking for a feedback.

This is a follow-up to PR opensearch-project#4156 and brings in a further simplification of the ZIP publication implementation and a new gradle test.

In the mentioned PR we specifically implemented the functionality to use `project.group` value for the `artifactId` and if there was an override provided by user (ie the `groupId` value was explicitly specified in POM configuration) then it was used instead.

Further, we were explicitly setting the artifactId and version as well.

But in fact all this has been already happening under the hood (by the libraries that do the heavy lifting of publication tasks processing). There is really no need to (re-)implement it again. As we can see from the modified code and tests that we can perfectly remove almost all the ZIP publication initialization code and all the things still work as expected. And I think it is because this is how the Project Object Model (POM) was designed to work.

Because of that the condition to test the groupId presence: `if groupId == null` was useless. It was never `true`. If the `project.group` is not defined the publication task fails with an exception (we test it), if there is a custom `groupId` value setup in publication config then it overrides the `project.group` as well. Again, we test it. :-)

I added new tests:

- to verify we can run "publishToMavenLocal" and get expected results. The inspiration for this comes from opensearch-project/opensearch-plugin-template-java#35

- to verify that applying only the 'opensearch.pluginzip' is enough, because it adds both the NebulaPublish and MavenPublishPlugin plugins automatically.

- to verify that if the plugin is applied but no publication is defined then a message is printed for the user.

Signed-off-by: Lukáš Vlček <lukas.vlcek@aiven.io>
  • Loading branch information
lukas-vlcek committed Sep 13, 2022
1 parent 51a529f commit 49ce45b
Show file tree
Hide file tree
Showing 10 changed files with 327 additions and 67 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Add index specific setting for remote repository ([#4253](https://github.com/opensearch-project/OpenSearch/pull/4253))
- [Segment Replication] Update replicas to commit SegmentInfos instead of relying on SIS files from primary shards. ([#4402](https://github.com/opensearch-project/OpenSearch/pull/4402))
- [CCR] Add getHistoryOperationsFromTranslog method to fetch the history snapshot from translogs ([#3948](https://github.com/opensearch-project/OpenSearch/pull/3948))
- Further simplification of the ZIP publication implementation ([#4360](https://github.com/opensearch-project/OpenSearch/pull/4360))

### Deprecated

Expand Down Expand Up @@ -88,4 +89,4 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)


[Unreleased]: https://github.com/opensearch-project/OpenSearch/compare/2.2.0...HEAD
[2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.2.0...2.x
[2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.2.0...2.x
83 changes: 36 additions & 47 deletions buildSrc/src/main/java/org/opensearch/gradle/pluginzip/Publish.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,83 +9,72 @@

import org.gradle.api.Plugin;
import org.gradle.api.Project;
import org.gradle.api.logging.Logger;
import org.gradle.api.logging.Logging;
import org.gradle.api.publish.PublishingExtension;
import org.gradle.api.publish.maven.MavenPublication;
import org.gradle.api.publish.maven.plugins.MavenPublishPlugin;

import java.nio.file.Path;
import org.gradle.api.Task;
import org.gradle.api.publish.maven.plugins.MavenPublishPlugin;

public class Publish implements Plugin<Project> {

private static final Logger LOGGER = Logging.getLogger(Publish.class);

public final static String EXTENSION_NAME = "zipmavensettings";
// public final static String PLUGIN_ZIP_PUBLISH_POM_TASK = "generatePomFileForPluginZipPublication";
public final static String PUBLICATION_NAME = "pluginZip";
public final static String STAGING_REPO = "zipStaging";
public final static String PLUGIN_ZIP_PUBLISH_POM_TASK = "generatePomFileForPluginZipPublication";
public final static String LOCALMAVEN = "publishToMavenLocal";
public final static String LOCAL_STAGING_REPO_PATH = "/build/local-staging-repo";
public String zipDistributionLocation = "/build/distributions/";
// TODO: Does the path ^^ need to use platform dependant file separators ?

private boolean isZipPublicationPresent(Project project) {
PublishingExtension pe = project.getExtensions().findByType(PublishingExtension.class);
if (pe == null) {
return false;
}
return pe.getPublications().findByName(PUBLICATION_NAME) != null;
}

public static void configMaven(Project project) {
private void addLocalMavenRepo(Project project) {
final Path buildDirectory = project.getRootDir().toPath();
project.getPluginManager().apply(MavenPublishPlugin.class);
project.getExtensions().configure(PublishingExtension.class, publishing -> {
publishing.repositories(repositories -> {
repositories.maven(maven -> {
maven.setName(STAGING_REPO);
maven.setUrl(buildDirectory.toString() + LOCAL_STAGING_REPO_PATH);
});
});
});
}

private void addZipArtifact(Project project) {
project.getExtensions().configure(PublishingExtension.class, publishing -> {
publishing.publications(publications -> {
MavenPublication mavenZip = (MavenPublication) publications.findByName(PUBLICATION_NAME);

if (mavenZip == null) {
mavenZip = publications.create(PUBLICATION_NAME, MavenPublication.class);
if (mavenZip != null) {
mavenZip.artifact(project.getTasks().named("bundlePlugin"));
}

String groupId = mavenZip.getGroupId();
if (groupId == null) {
// The groupId is not customized thus we get the value from "project.group".
// See https://docs.gradle.org/current/userguide/publishing_maven.html#sec:identity_values_in_the_generated_pom
groupId = getProperty("group", project);
}

String artifactId = project.getName();
String pluginVersion = getProperty("version", project);
mavenZip.artifact(project.getTasks().named("bundlePlugin"));
mavenZip.setGroupId(groupId);
mavenZip.setArtifactId(artifactId);
mavenZip.setVersion(pluginVersion);
});
});
}

static String getProperty(String name, Project project) {
if (project.hasProperty(name)) {
Object property = project.property(name);
if (property != null) {
return property.toString();
}
}
return null;
}

@Override
public void apply(Project project) {
project.getPluginManager().apply("nebula.maven-base-publish");
project.getPluginManager().apply(MavenPublishPlugin.class);
project.afterEvaluate(evaluatedProject -> {
configMaven(project);
Task validatePluginZipPom = project.getTasks().findByName("validatePluginZipPom");
if (validatePluginZipPom != null) {
project.getTasks().getByName("validatePluginZipPom").dependsOn("generatePomFileForNebulaPublication");
}
Task publishPluginZipPublicationToZipStagingRepository = project.getTasks()
.findByName("publishPluginZipPublicationToZipStagingRepository");
if (publishPluginZipPublicationToZipStagingRepository != null) {
publishPluginZipPublicationToZipStagingRepository.dependsOn("generatePomFileForNebulaPublication");
if (isZipPublicationPresent(project)) {
addLocalMavenRepo(project);
addZipArtifact(project);
Task validatePluginZipPom = project.getTasks().findByName("validatePluginZipPom");
if (validatePluginZipPom != null) {
validatePluginZipPom.dependsOn("generatePomFileForNebulaPublication");
}
Task publishPluginZipPublicationToZipStagingRepository = project.getTasks()
.findByName("publishPluginZipPublicationToZipStagingRepository");
if (publishPluginZipPublicationToZipStagingRepository != null) {
publishPluginZipPublicationToZipStagingRepository.dependsOn("generatePomFileForNebulaPublication");
}
} else {
project.getLogger()
.warn(String.format("Plugin 'opensearch.pluginzip' is applied but no '%s' publication is defined.", PUBLICATION_NAME));
}
});
}
Expand Down
Loading

0 comments on commit 49ce45b

Please sign in to comment.