versionWithBranch respecing release tags | fixes #969

Author: bgalek

src/main/groovy/pl/allegro/tech/build/axion/release/domain/PredefinedVersionCreator.groovy

@@ -10,14 +10,14 @@ enum PredefinedVersionCreator {
     }),
 
     VERSION_WITH_BRANCH('versionWithBranch', { String versionFromTag, ScmPosition position ->
-        if ((position.branch != 'master' && position.branch != 'main') && position.branch != 'HEAD') {
+        if (!position.isReleaseBranch && position.branch != 'HEAD' && !position.isTagRef) {
             return "$versionFromTag-$position.branch".toString()
         }
         return versionFromTag
     }),
 
     VERSION_WITH_COMMIT_HASH('versionWithCommitHash', { String versionFromTag, ScmPosition position ->
-    if ((position.branch != 'master' && position.branch != 'main') && position.branch != 'HEAD') {
+    if (!position.isReleaseBranch && position.branch != 'HEAD' && !position.isTagRef) {
         return "$versionFromTag-$position.shortRevision".toString()
     }
     return versionFromTag

src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmPosition.java

@@ -8,19 +8,31 @@ public class ScmPosition {
     private final String shortRevision;
     private final String branch;
     private final boolean isClean;
+    private final boolean isReleaseBranch;
+    private final boolean isTagRef;
 
-    public ScmPosition(String revision, String shortRevision, String branch, boolean isClean) {
+    public ScmPosition(String revision, String shortRevision, String branch, boolean isClean, boolean isReleaseBranch, boolean isTagRef) {
         this.revision = revision;
         this.shortRevision = shortRevision;
         this.branch = branch;
         this.isClean = isClean;
+        this.isReleaseBranch = isReleaseBranch;
+        this.isTagRef = isTagRef;
+    }
+
+    public ScmPosition(String revision, String shortRevision, String branch, boolean isClean, boolean isReleaseBranch) {
+        this(revision, shortRevision, branch, isClean, isReleaseBranch, false);
+    }
+
+    public ScmPosition(String revision, String shortRevision, String branch, boolean isClean) {
+        this(revision, shortRevision, branch, isClean, false, false);
     }
 
     public ScmPosition(String revision, String shortRevision, String branch) {
-        this(revision, shortRevision, branch, true);
+        this(revision, shortRevision, branch, true, false, false);
     }
 
-    public ScmPosition(String revision, String branch, boolean isClean) {
+    public ScmPosition(String revision, String branch, boolean isClean, boolean isReleaseBranch, boolean isTagRef) {
         this.revision = revision;
         if (revision.length() > 7) {
             this.shortRevision = revision.substring(0, 7);
@@ -29,18 +41,30 @@ public ScmPosition(String revision, String branch, boolean isClean) {
         }
         this.branch = branch;
         this.isClean = isClean;
+        this.isReleaseBranch = isReleaseBranch;
+        this.isTagRef = isTagRef;
+    }
+
+    public ScmPosition(String revision, String branch, boolean isClean, boolean isReleaseBranch) {
+        this(revision, branch, isClean, isReleaseBranch, false);
+    }
+
+    public ScmPosition(String revision, String branch, boolean isClean) {
+        this(revision, branch, isClean, false, false);
     }
 
     public ScmPosition(String revision, String branch) {
-        this(revision, branch, true);
+        this(revision, branch, true, false, false);
     }
 
     @Override
     public String toString() {
         return "ScmPosition[revision = " + revision
             + ", shortRevision = " + shortRevision
             + ", branch = " + branch
-            + ", isClean = " + isClean + "]";
+            + ", isClean = " + isClean
+            + ", isReleaseBranch = " + isReleaseBranch
+            + ", isTagRef = " + isTagRef + "]";
     }
 
     @Input
@@ -62,4 +86,14 @@ public String getBranch() {
     public boolean getIsClean() {
         return isClean;
     }
+
+    @Input
+    public boolean getIsReleaseBranch() {
+        return isReleaseBranch;
+    }
+
+    @Input
+    public boolean getIsTagRef() {
+        return isTagRef;
+    }
 }

src/main/java/pl/allegro/tech/build/axion/release/infrastructure/git/GitRepository.java

@@ -56,6 +56,7 @@
 
 import static java.util.stream.Collectors.toList;
 import static pl.allegro.tech.build.axion.release.TagPrefixConf.fullLegacyPrefix;
+import static pl.allegro.tech.build.axion.release.TagPrefixConf.fullPrefix;
 
 public class GitRepository implements ScmRepository {
     private static final Logger logger = Logging.getLogger(GitRepository.class);
@@ -208,7 +209,7 @@ private ScmPushResult verifyPushResults(Iterable<PushResult> pushResults) {
 
         Optional<RemoteRefUpdate> failedRefUpdate = pushResult.getRemoteUpdates().stream().filter(ref ->
             !ref.getStatus().equals(RemoteRefUpdate.Status.OK)
-                && !ref.getStatus().equals(RemoteRefUpdate.Status.UP_TO_DATE)
+            && !ref.getStatus().equals(RemoteRefUpdate.Status.UP_TO_DATE)
         ).findFirst();
 
         boolean isSuccess = failedRefUpdate.isEmpty();
@@ -308,7 +309,9 @@ public ScmPosition positionOfLastChangeIn(String path, List<String> excludeSubFo
         return new ScmPosition(
             lastCommit.getName(),
             currentPosition.getBranch(),
-            currentPosition.getIsClean()
+            currentPosition.getIsClean(),
+            currentPosition.getIsReleaseBranch(),
+            currentPosition.getIsTagRef()
         );
     }
 
@@ -350,7 +353,14 @@ public ScmPosition currentPosition() {
         String revision = getRevision();
         String branchName = branchName();
         boolean isClean = !checkUncommittedChanges();
-        return new ScmPosition(revision, branchName, isClean);
+        boolean isReleaseBranch = properties.getReleaseBranchNames() != null && properties.getReleaseBranchNames().contains(branchName);
+        boolean isTagRef = isVersionTagRef(properties.getOverriddenBranchName() != null ? properties.getOverriddenBranchName() : branchName);
+        return new ScmPosition(revision, branchName, isClean, isReleaseBranch, isTagRef);
+    }
+
+    private boolean isVersionTagRef(String branchName) {
+        return branchName.startsWith(GIT_TAG_PREFIX + fullPrefix())
+               || branchName.startsWith(GIT_TAG_PREFIX + fullLegacyPrefix());
     }
 
     private String getRevision() {
@@ -620,7 +630,7 @@ public boolean isLegacyDefTagnameRepo() {
             List<Ref> call = jgitRepository.tagList().call();
             if (call.isEmpty()) return false;
 
-            return call.stream().allMatch(ref -> ref.getName().startsWith("refs/tags/" + fullLegacyPrefix()));
+            return call.stream().allMatch(ref -> ref.getName().startsWith(GIT_TAG_PREFIX + fullLegacyPrefix()));
         } catch (GitAPIException e) {
             throw new ScmException(e);
         }

src/test/groovy/pl/allegro/tech/build/axion/release/domain/PredefinedVersionCreatorTest.groovy

@@ -12,26 +12,61 @@ class PredefinedVersionCreatorTest extends Specification {
         PredefinedVersionCreator.SIMPLE.versionCreator.apply('version',scmPosition('master')) == 'version'
     }
 
-    def "versionWithBranch version creator should return simple version when on master"() {
+    def "versionWithBranch should not append on release branch"() {
         expect:
-        PredefinedVersionCreator.VERSION_WITH_BRANCH.versionCreator.apply('version', scmPosition('master')) == 'version'
+        PredefinedVersionCreator.VERSION_WITH_BRANCH.versionCreator.apply('version', scmPosition().withBranch('release').asReleaseBranch().build()) == 'version'
     }
 
-    def "versionWithBranch version creator should return version with appended branch name when not on master"() {
+    def "versionWithBranch should append branch name when not on release branch"() {
         expect:
-        PredefinedVersionCreator.VERSION_WITH_BRANCH.versionCreator.apply('version', scmPosition('branch')) == 'version-branch'
+        PredefinedVersionCreator.VERSION_WITH_BRANCH.versionCreator.apply('version', scmPosition('feature/branch')) == 'version-feature/branch'
     }
 
-    def "versionWithCommitHash version creator should return simple version when on main"() {
+    def "versionWithCommitHash should not append on release branch"() {
         expect:
-        PredefinedVersionCreator.VERSION_WITH_COMMIT_HASH.versionCreator.apply('version', scmPosition('main')) == 'version'
+        PredefinedVersionCreator.VERSION_WITH_COMMIT_HASH.versionCreator.apply('version', scmPosition().withBranch('release').asReleaseBranch().build()) == 'version'
     }
 
-    def "versionWithCommitHash version creator should return version with appended short SHA-1 hash when not on main"() {
+    def "versionWithCommitHash should append short SHA-1 hash when not on release branch"() {
         expect:
         PredefinedVersionCreator.VERSION_WITH_COMMIT_HASH.versionCreator.apply('version', scmPosition('branch')) == 'version-c143976'
     }
 
+    def "VERSION_WITH_BRANCH should not append when ref is a version tag"() {
+        given:
+        def pos = scmPosition().withBranch('refs/tags/v1.0.0').build()
+        expect:
+        PredefinedVersionCreator.VERSION_WITH_BRANCH.versionCreator.apply('version', pos) == 'version'
+    }
+
+    def "VERSION_WITH_BRANCH should not append when ref is a random tag"() {
+        given:
+        def pos = scmPosition().withBranch('refs/tags/random-tag').build()
+        expect:
+        PredefinedVersionCreator.VERSION_WITH_BRANCH.versionCreator.apply('version', pos) == 'version-refs/tags/random-tag'
+    }
+
+    def "VERSION_WITH_BRANCH should append when ref is a version tag"() {
+        given:
+        def pos = scmPosition().withBranch('refs/tags/v1.0.0').build()
+        expect:
+        PredefinedVersionCreator.VERSION_WITH_BRANCH.versionCreator.apply('version', pos) == 'version'
+    }
+
+    def "VERSION_WITH_COMMIT_HASH should append when ref is a random tag"() {
+        given:
+        def pos = scmPosition().withBranch('refs/tags/random-tag').build()
+        expect:
+        PredefinedVersionCreator.VERSION_WITH_COMMIT_HASH.versionCreator.apply('version', pos) == 'version-c143976'
+    }
+
+    def "VERSION_WITH_COMMIT_HASH should not append when ref is a version tag"() {
+        given:
+        def pos = scmPosition().withBranch('refs/tags/v1.0.0').build()
+        expect:
+        PredefinedVersionCreator.VERSION_WITH_COMMIT_HASH.versionCreator.apply('version', pos) == 'version'
+    }
+
     def "should return version creator of given type"() {
         expect:
         PredefinedVersionCreator.versionCreatorFor('simple').apply('version', null) == 'version'

src/test/groovy/pl/allegro/tech/build/axion/release/domain/scm/ScmPositionBuilder.groovy

@@ -1,5 +1,8 @@
 package pl.allegro.tech.build.axion.release.domain.scm
 
+import static pl.allegro.tech.build.axion.release.TagPrefixConf.fullLegacyPrefix
+import static pl.allegro.tech.build.axion.release.TagPrefixConf.fullPrefix
+
 class ScmPositionBuilder {
 
     private String branch = 'master'
@@ -10,6 +13,8 @@ class ScmPositionBuilder {
 
     private boolean isClean = true
 
+    private boolean isReleaseBranch = false
+
     private ScmPositionBuilder() {
     }
 
@@ -22,7 +27,8 @@ class ScmPositionBuilder {
     }
 
     ScmPosition build() {
-        return new ScmPosition(revision, shortRevision, branch, isClean)
+        def isTagRef = branch.startsWith("refs/tags/" + fullPrefix()) || branch.startsWith("refs/tags/" + fullLegacyPrefix());
+        return new ScmPosition(revision, shortRevision, branch, isClean, isReleaseBranch, isTagRef)
     }
 
     ScmPositionBuilder withBranch(String branch) {
@@ -40,4 +46,9 @@ class ScmPositionBuilder {
         this.isClean = false
         return this
     }
+
+    ScmPositionBuilder asReleaseBranch() {
+        this.isReleaseBranch = true
+        return this
+    }
 }