Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class AnnotatedTagFinder {
this.versionNamer = versionNamer;
}

List<AnnotatedTag> tagsForVersion(Git git, String module, String versionWithoutBuildNumber) throws MojoExecutionException {
List<AnnotatedTag> tagsForVersion(Git git, String module, String versionWithoutBuildNumber, String tagNameSeparator) throws MojoExecutionException {
ArrayList<AnnotatedTag> results = new ArrayList<AnnotatedTag>();
List<Ref> tags;
try {
Expand All @@ -27,7 +27,7 @@ List<AnnotatedTag> tagsForVersion(Git git, String module, String versionWithoutB
throw new MojoExecutionException("Error while getting a list of tags in the local repo", e);
}
Collections.reverse(tags);
String tagWithoutBuildNumber = module + "-" + versionWithoutBuildNumber;
String tagWithoutBuildNumber = module + tagNameSeparator + versionWithoutBuildNumber;
for (Ref tag : tags) {
if (isPotentiallySameVersionIgnoringBuildNumber(tagWithoutBuildNumber, tag.getName())) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,13 @@ public abstract class BaseMojo extends AbstractMojo {
@Parameter(property = "arguments")
public String arguments;

/**
* <p>Determines the separator between artifactId and groupId<p>
Copy link
Owner

Choose a reason for hiding this comment

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

I think this documentation needs more detail about what this is for and what tags look like, which is {artifactid}-{version} (doesn't actually have groupid). Maybe this should actually be a string like {artifactid}-{version} and the code just does a string replace on {artifactid} and {version}. There could be {groupid} too.

Copy link
Author

@thomasbonset thomasbonset Oct 10, 2019

Choose a reason for hiding this comment

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

Ok I will try to do that. That's what i will implement :
if my custom tag name parameter contains at least substrings "{artifactId}" and "{version}" (minimum required to guarantee uniqueness of a git tag) then do a

tagName=customTagName
        		.replace("{artifactId}", project.getArtifactId())
        		.replace("{groupId}", project.getGroupId())
        		.replace("{version}", version.releaseVersion());

else log a warn and use default value {artifactid}-{version}

Copy link
Owner

Choose a reason for hiding this comment

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

Oh wait, would that complicate the tag lookups in the part of the code that guesses the next build number? (It's been a while since I worked on this so can't really remember)

* <p>By default, set to '-'</p>
*/
@Parameter(alias = "tagNameSeparator", required = false, readonly = true, defaultValue = "-", property = "tagNameSeparator")
protected String tagNameSeparator;

final void setSettings(final Settings settings) {
this.settings = settings;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
.credentialsProvider(getCredentialsProvider(log))
.buildFromCurrentDir();
ResolverWrapper resolverWrapper = new ResolverWrapper(factory, artifactResolver, remoteRepositories, localRepository);
Reactor reactor = Reactor.fromProjects(log, repo, project, projects, buildNumber, modulesToForceRelease, noChangesAction, resolverWrapper, versionNamer);
Reactor reactor = Reactor.fromProjects(log, repo, project, projects, buildNumber, modulesToForceRelease, noChangesAction, resolverWrapper, versionNamer, tagNameSeparator);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not ideal to make an excessively long list of parameters even longer. I have an upcoming PR that fixes this and I would appreciated if you could hold back this PR until then.

Copy link
Author

Choose a reason for hiding this comment

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

Of course, first come first served ! As soon as your PR is approved and merged, il will rebase and adapt mine.

if (reactor == null) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public List<ReleasableModule> getModulesInBuildOrder() {
return modulesInBuildOrder;
}

public static Reactor fromProjects(Log log, LocalGitRepo gitRepo, MavenProject rootProject, List<MavenProject> projects, Long buildNumber, List<String> modulesToForceRelease, NoChangesAction actionWhenNoChangesDetected, ResolverWrapper resolverWrapper, VersionNamer versionNamer) throws ValidationException, GitAPIException, MojoExecutionException {
public static Reactor fromProjects(Log log, LocalGitRepo gitRepo, MavenProject rootProject, List<MavenProject> projects, Long buildNumber, List<String> modulesToForceRelease, NoChangesAction actionWhenNoChangesDetected, ResolverWrapper resolverWrapper, VersionNamer versionNamer, String tagNameSeparator) throws ValidationException, GitAPIException, MojoExecutionException {
DiffDetector detector = new TreeWalkingDiffDetector(gitRepo.git.getRepository());
List<ReleasableModule> modules = new ArrayList<ReleasableModule>();

Expand All @@ -39,7 +39,7 @@ public static Reactor fromProjects(Log log, LocalGitRepo gitRepo, MavenProject r
String relativePathToModule = calculateModulePath(rootProject, project);
String artifactId = project.getArtifactId();
String versionWithoutBuildNumber = project.getVersion().replace("-SNAPSHOT", "");
List<AnnotatedTag> previousTagsForThisModule = annotatedTagFinder.tagsForVersion(gitRepo.git, artifactId, versionWithoutBuildNumber);
List<AnnotatedTag> previousTagsForThisModule = annotatedTagFinder.tagsForVersion(gitRepo.git, artifactId, versionWithoutBuildNumber, tagNameSeparator);


Collection<Long> previousBuildNumbers = new ArrayList<Long>();
Expand Down Expand Up @@ -108,7 +108,7 @@ public static Reactor fromProjects(Log log, LocalGitRepo gitRepo, MavenProject r
log.info("Will use version " + newVersion.releaseVersion() + " for " + artifactId + " as it has changed since the last release.");
}
}
ReleasableModule module = new ReleasableModule(project, newVersion, equivalentVersion, relativePathToModule);
ReleasableModule module = new ReleasableModule(project, newVersion, equivalentVersion, relativePathToModule, tagNameSeparator);
modules.add(module);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ public class ReleasableModule {
private final String tagName;
private final String equivalentVersion;
private final String relativePathToModule;
private final String tagNameSeparator;

public ReleasableModule(MavenProject project, VersionName version, String equivalentVersion, String relativePathToModule) {
public ReleasableModule(MavenProject project, VersionName version, String equivalentVersion, String relativePathToModule, String tagNameSeparator) {
this.project = project;
this.version = version;
this.equivalentVersion = equivalentVersion;
this.relativePathToModule = relativePathToModule;
this.tagName = project.getArtifactId() + "-" + version.releaseVersion();
this.tagNameSeparator = tagNameSeparator;
this.tagName = project.getArtifactId() + tagNameSeparator + version.releaseVersion();
}

public String getTagName() {
Expand Down Expand Up @@ -71,6 +73,6 @@ public String getRelativePathToModule() {
}

public ReleasableModule createReleasableVersion() {
return new ReleasableModule(project, version, null, relativePathToModule);
return new ReleasableModule(project, version, null, relativePathToModule, tagNameSeparator);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
repo.errorIfNotClean();

ResolverWrapper resolverWrapper = new ResolverWrapper(factory, artifactResolver, remoteRepositories, localRepository);
Reactor reactor = Reactor.fromProjects(log, repo, project, projects, buildNumber, modulesToForceRelease, noChangesAction, resolverWrapper, versionNamer);
Reactor reactor = Reactor.fromProjects(log, repo, project, projects, buildNumber, modulesToForceRelease, noChangesAction, resolverWrapper, versionNamer, tagNameSeparator);
if (reactor == null) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ public void findsTheLatestCommitWhereThereHaveBeenNoBranches() throws Exception
AnnotatedTag tag2 = saveFileInModule(project, "core-utils", "2", 0);
AnnotatedTag tag3 = saveFileInModule(project, "console-app", "1.2", 4);

assertThat(annotatedTagFinder.tagsForVersion(project.local, "console-app", "1.3"), hasSize(0));
assertThat(annotatedTagFinder.tagsForVersion(project.local, "console-app", "1.2"), containsInAnyOrder(tag1, tag3));
assertThat(annotatedTagFinder.tagsForVersion(project.local, "core-utils", "2"), contains(tag2));
assertThat(annotatedTagFinder.tagsForVersion(project.local, "console-app", "1.3", "-"), hasSize(0));
assertThat(annotatedTagFinder.tagsForVersion(project.local, "console-app", "1.2", "-"), containsInAnyOrder(tag1, tag3));
assertThat(annotatedTagFinder.tagsForVersion(project.local, "core-utils", "2", "-"), contains(tag2));
}

static AnnotatedTag saveFileInModule(TestProject project, String moduleName, String version, long buildNumber) throws IOException, GitAPIException {
Expand Down Expand Up @@ -58,7 +58,7 @@ public void returnsMultipleTagsOnASingleCommit() throws IOException, GitAPIExcep
AnnotatedTag tag1 = tagLocalRepo(project, "console-app-1.1.1.1", "1.1.1", 1);
AnnotatedTag tag3 = tagLocalRepo(project, "console-app-1.1.1.3", "1.1.1", 3);
AnnotatedTag tag2 = tagLocalRepo(project, "console-app-1.1.1.2", "1.1.1", 2);
List<AnnotatedTag> annotatedTags = annotatedTagFinder.tagsForVersion(project.local, "console-app", "1.1.1");
List<AnnotatedTag> annotatedTags = annotatedTagFinder.tagsForVersion(project.local, "console-app", "1.1.1", "-");
assertThat(annotatedTags, containsInAnyOrder(tag1, tag2, tag3));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public void aReleaseableModuleCanBeCreatedFromAnUnreleasableOne() {
project.setArtifactId("some-arty");
project.setGroupId("some-group");
ReleasableModule first = new ReleasableModule(
project, new VersionName("1.2.3-SNAPSHOT", "1.2.3", 12), "1.2.3.11", "somewhere"
project, new VersionName("1.2.3-SNAPSHOT", "1.2.3", 12), "1.2.3.11", "somewhere", "-"
);
assertThat(first.willBeReleased(), is(false));

Expand Down
89 changes: 89 additions & 0 deletions src/test/java/e2e/InheritedVersionsWithTagNameSeparatorTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package e2e;

import org.apache.maven.shared.invoker.MavenInvocationException;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.BeforeClass;
import org.junit.Test;
import scaffolding.MvnRunner;
import scaffolding.TestProject;

import java.io.IOException;
import java.util.List;

import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsEqual.equalTo;
import static scaffolding.ExactCountMatcher.oneOf;
import static scaffolding.ExactCountMatcher.twoOf;
import static scaffolding.GitMatchers.hasCleanWorkingDirectory;
import static scaffolding.GitMatchers.hasTag;
import static scaffolding.MvnRunner.assertArtifactInLocalRepo;

public class InheritedVersionsWithTagNameSeparatorTest {
Copy link
Owner

Choose a reason for hiding this comment

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

This test should be more focused on the actual change. So delete the copied tests around inheritance and rollbacks etc and just test that the tag separator logic is working. Include tests for invalid separators (i.e. things that are not allowed in git tags).


public static final String[] ARTIFACT_IDS = new String[]{"inherited-versions-from-parent", "core-utils", "console-app"};
final String buildNumber = String.valueOf(System.currentTimeMillis());
final String expected = "1.0." + buildNumber;
final TestProject testProject = TestProject.inheritedVersionsFromParentWithTagNameSeparator();

@BeforeClass
public static void installPluginToLocalRepo() throws MavenInvocationException {
MvnRunner.installReleasePluginToLocalRepo();
}

@Test
public void buildsAndInstallsAndTagsAllModules() throws Exception {
buildsEachProjectOnceAndOnlyOnce(testProject.mvnRelease(buildNumber));
installsAllModulesIntoTheRepoWithTheBuildNumber();
theLocalAndRemoteGitReposAreTaggedWithTheModuleNameAndVersion();
}

private void buildsEachProjectOnceAndOnlyOnce(List<String> commandOutput) throws Exception {
assertThat(
commandOutput,
allOf(
oneOf(containsString("Going to release inherited-versions-from-parent " + expected)),
twoOf(containsString("Building inherited-versions-from-parent")), // once for initial build; once for release build
oneOf(containsString("Building core-utils")),
oneOf(containsString("Building console-app")),
oneOf(containsString("The Calculator Test has run"))
)
);
}

private void installsAllModulesIntoTheRepoWithTheBuildNumber() throws Exception {
assertArtifactInLocalRepo("com.github.danielflower.mavenplugins.testprojects.versioninheritor", "inherited-versions-from-parent", expected);
assertArtifactInLocalRepo("com.github.danielflower.mavenplugins.testprojects.versioninheritor", "core-utils", expected);
assertArtifactInLocalRepo("com.github.danielflower.mavenplugins.testprojects.versioninheritor", "console-app", expected);
}

private void theLocalAndRemoteGitReposAreTaggedWithTheModuleNameAndVersion() throws IOException, InterruptedException {
for (String artifactId : ARTIFACT_IDS) {
String expectedTag = artifactId + "/" + expected;
assertThat(testProject.local, hasTag(expectedTag));
assertThat(testProject.origin, hasTag(expectedTag));
}
}

@Test
public void thePomChangesAreRevertedAfterTheRelease() throws IOException, InterruptedException {
ObjectId originHeadAtStart = head(testProject.origin);
ObjectId localHeadAtStart = head(testProject.local);
assertThat(originHeadAtStart, equalTo(localHeadAtStart));
testProject.mvnRelease(buildNumber);
assertThat(head(testProject.origin), equalTo(originHeadAtStart));
assertThat(head(testProject.local), equalTo(localHeadAtStart));
assertThat(testProject.local, hasCleanWorkingDirectory());
}

// @Test
// public void whenOneModuleDependsOnAnotherThenWhenReleasingThisDependencyHasTheRelaseVersion() {
// // TODO: implement this
// }

private ObjectId head(Git git) throws IOException {
return git.getRepository().getRefDatabase().findRef("HEAD").getObjectId();
}
}
2 changes: 1 addition & 1 deletion src/test/java/scaffolding/ReleasableModuleBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public ReleasableModuleBuilder withSnapshotVersion(String snapshotVersion) {
}

public ReleasableModule build() throws ValidationException {
return new ReleasableModule(project, versionNamer.name(project.getVersion(), buildNumber, null), equivalentVersion, relativePathToModule);
return new ReleasableModule(project, versionNamer.name(project.getVersion(), buildNumber, null), equivalentVersion, relativePathToModule, "-");
}

public static ReleasableModuleBuilder aModule() {
Expand Down
6 changes: 5 additions & 1 deletion src/test/java/scaffolding/TestProject.java
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,11 @@ public static TestProject moduleWithSnapshotDependenciesWithVersionProperties()
public static TestProject differentDelimiterProject() {
return project("different-delimiter");
}


public static TestProject inheritedVersionsFromParentWithTagNameSeparator() {
return project("inherited-versions-from-parent-with-tag-name-separator");
}

public void setMvnRunner(MvnRunner mvnRunner) {
this.mvnRunner = mvnRunner;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
target
.idea/
*.iml
.classpath
.settings
.project
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<parent>
<artifactId>inherited-versions-from-parent</artifactId>
<groupId>com.github.danielflower.mavenplugins.testprojects.versioninheritor</groupId>
<version>1.0-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>console-app</artifactId>

<dependencies>
<dependency>
<groupId>com.github.danielflower.mavenplugins.testprojects.versioninheritor</groupId>
<artifactId>core-utils</artifactId>
<version>1.0-SNAPSHOT</version>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.github.danielflower.mavenplugins.testprojects.versioninheritor;

public class App {
public static void main(String[] args) {
Calculator calculator = new Calculator();
System.out.println("1 + 2 = " + calculator.add(1, 2));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<parent>
<artifactId>inherited-versions-from-parent</artifactId>
<groupId>com.github.danielflower.mavenplugins.testprojects.versioninheritor</groupId>
<version>1.0-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>core-utils</artifactId>

<dependencies>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.11</version>
<scope>test</scope>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.github.danielflower.mavenplugins.testprojects.versioninheritor;

public class Calculator {

public int add(int a, int b) {
return a + b;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.github.danielflower.mavenplugins.testprojects.versioninheritor;


import org.junit.Assert;
import org.junit.Test;

public class CalculatorTest {

@Test
public void testAdd() throws Exception {
Assert.assertEquals(3, new Calculator().add(1, 2));
System.out.println("The Calculator Test has run"); // used in a test to assert this has run
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">

<modelVersion>4.0.0</modelVersion>

<groupId>com.github.danielflower.mavenplugins.testprojects.versioninheritor</groupId>
<artifactId>inherited-versions-from-parent</artifactId>
<version>1.0-SNAPSHOT</version>
<modules>
<module>core-utils</module>
<module>console-app</module>
</modules>
<packaging>pom</packaging>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>

<build>
<plugins>
<plugin>
<groupId>com.github.danielflower.mavenplugins</groupId>
<artifactId>multi-module-maven-release-plugin</artifactId>
<version>${current.plugin.version}</version>
<configuration>
<releaseGoals>
<releaseGoal>install</releaseGoal>
</releaseGoals>
<tagNameSeparator>/</tagNameSeparator>
</configuration>
</plugin>
</plugins>
</build>
</project>