fix: Prevent commits from long lived branches to be counted more than…

… once (#60)

src/main/kotlin/git/semver/plugin/scm/Commit.kt

@@ -2,7 +2,7 @@ package git.semver.plugin.scm
 
 import java.util.Date
 
-class Commit(override val text: String, override val sha: String, val parents: Sequence<Commit>,
+class Commit(override val text: String, override val sha: String, val commitTime: Int, val parents: Sequence<Commit>,
     val authorName:String = "", val authorEmail:String = "", val authorWhen:Date = Date()) : IRefInfo {
     override fun toString(): String = text
 }

src/main/kotlin/git/semver/plugin/scm/GitProvider.kt

@@ -9,6 +9,7 @@ import org.eclipse.jgit.lib.Ref
 import org.eclipse.jgit.lib.Repository
 import org.eclipse.jgit.lib.RepositoryBuilder
 import org.eclipse.jgit.revwalk.RevCommit
+import org.eclipse.jgit.revwalk.RevSort
 import org.eclipse.jgit.revwalk.RevWalk
 import org.eclipse.jgit.util.FS
 import org.slf4j.LoggerFactory
@@ -119,14 +120,14 @@ internal class GitProvider(private val settings: SemverSettings) {
 
     internal fun getHeadCommit(it: Repository): Commit {
         val revWalk = RevWalk(it)
-        val head = it.resolve("HEAD") ?: return Commit("", "", emptySequence())
+        val head = it.resolve("HEAD") ?: return Commit("", "", 0, emptySequence())
         val revCommit = revWalk.parseCommit(head)
         revWalk.markStart(revCommit)
         return getCommit(revCommit, revWalk)
     }
 
     private fun getCommit(commit: RevCommit, revWalk: RevWalk): Commit {
-        return Commit(commit.fullMessage, commit.name, sequence {
+        return Commit(commit.fullMessage, commit.name, commit.commitTime, sequence {
             for (parent in commit.parents) {
                 revWalk.parseHeaders(parent)
                 yield(getCommit(parent, revWalk))

src/main/kotlin/git/semver/plugin/semver/VersionFinder.kt

@@ -4,6 +4,7 @@ import git.semver.plugin.scm.Commit
 import git.semver.plugin.scm.IRefInfo
 import org.slf4j.LoggerFactory
 import java.util.ArrayDeque
+import java.util.PriorityQueue
 
 class VersionFinder(private val settings: SemverSettings, private val tags: Map<String, List<IRefInfo>>) {
     private val logger = LoggerFactory.getLogger(javaClass)
@@ -51,62 +52,113 @@ class VersionFinder(private val settings: SemverSettings, private val tags: Map<
         return findVersion(sequenceOf(startCommit), changeLog)
     }
 
+    data class CommitData(
+        val commit: Commit,
+        val parents: MutableList<String>,
+        val isParentOfReleaseCommit: Boolean
+    ) : Comparable<CommitData> {
+
+        override fun compareTo(other: CommitData): Int {
+            return other.commit.commitTime.compareTo(commit.commitTime)
+        }
+    }
+
     private fun findVersion(
         commitsList: Sequence<Commit>,
         changeLog: MutableList<Commit>?
     ): MutableSemVersion {
 
+        var liveBranchCount = 1;
         var lastFoundVersion = versionZero()
+
         // This code is a recursive algoritm rewritten as iterative to avoid stack overflow exception.
         // Unfortunately that makes it hard to understand.
-        val commits = ArrayDeque(commitsList.map { it to ArrayList<String>(1) }.toList())
+
+        val commits = PriorityQueue(commitsList.map { CommitData(it, ArrayList(1), false) }.toList())
         val visitedCommits = mutableMapOf<String, MutableSemVersion?>()
+        val includedCommits = ArrayDeque<CommitData>()
+
         while (commits.isNotEmpty()) {
-            val peek = commits.peek()
-            val currentCommit = peek.first
-            val currentParentList = peek.second
-            if (!visitedCommits.containsKey(currentCommit.sha)) {
-                // First time we visit this commit
+            val commitData = commits.remove()
+            val currentCommit = commitData.commit
+
+            // First time we visit this commit
+
+            if (commitData.isParentOfReleaseCommit) {
+                // This commit is a parent of a release commit
+                markParentCommitsAsVisited(liveBranchCount, currentCommit, visitedCommits, commits)
+            } else if (!visitedCommits.containsKey(currentCommit.sha)) {
+
                 val releaseVersion = getReleaseSemVersionFromCommit(currentCommit)
                 visitedCommits[currentCommit.sha] = releaseVersion
+
                 if (isRelease(releaseVersion)) {
                     logger.debug("Release version found: {}", releaseVersion)
                     // Release fond so no need to visit this commit again
-                    commits.pop()
                     lastFoundVersion = releaseVersion!!
+
+                    liveBranchCount -= 1
+
+                    markParentCommitsAsVisited(liveBranchCount, currentCommit, visitedCommits, commits)
                 } else {
+                    // This is a normal commit or a pre-release. We will visit this again in the second phase.
+                    includedCommits.push(commitData)
+
                     currentCommit.parents.forEach {
-                        currentParentList.add(it.sha)
+                        commitData.parents.add(it.sha)
                         if (!visitedCommits.containsKey(it.sha)) {
                             // prepare to visit parent commit
-                            commits.push(it to ArrayList(1))
+                            commits.add(CommitData(it, ArrayList(1), false))
                         }
                     }
+                    liveBranchCount += commitData.parents.size - 1
                 }
-            } else {
-                // Second time we visit this commit after visiting parent commits
-                addToChangeLog(currentCommit, changeLog, currentParentList.size > 1)
-
-                // Check if we found a preRelease version first time we visited this commit
-                val preReleaseVersion = visitedCommits[currentCommit.sha]
-
-                // Get and clear the semVersions for the parents so that they are not counted twice
-                val parentSemVersions = currentParentList
-                    .mapNotNull { visitedCommits.put(it, null) }
-                    .toList()
-
-                val maxVersionFromParents = parentSemVersions.maxOrNull() ?: versionZero()
-                maxVersionFromParents.mergeChanges(parentSemVersions)
-                maxVersionFromParents.updateFromCommit(currentCommit, settings, preReleaseVersion)
-                visitedCommits[currentCommit.sha] = maxVersionFromParents
-
-                commits.pop()
-                lastFoundVersion = maxVersionFromParents
             }
         }
+
+        while (includedCommits.isNotEmpty()) {
+
+            val commitData = includedCommits.pop()
+            val currentCommit = commitData.commit
+            val currentParentList = commitData.parents
+
+            // Second time we visit this commit after visiting parent commits
+            addToChangeLog(currentCommit, changeLog, currentParentList.size > 1)
+
+            // Check if we found a preRelease version first time we visited this commit
+            val preReleaseVersion = visitedCommits[currentCommit.sha]
+
+            // Get and clear the semVersions for the parents so that they are not counted twice
+            val parentSemVersions = currentParentList
+                .mapNotNull { visitedCommits.put(it, null) }
+                .toList()
+
+            val maxVersionFromParents = parentSemVersions.maxOrNull() ?: versionZero()
+            maxVersionFromParents.mergeChanges(parentSemVersions)
+            maxVersionFromParents.updateFromCommit(currentCommit, settings, preReleaseVersion)
+            visitedCommits[currentCommit.sha] = maxVersionFromParents
+
+            lastFoundVersion = maxVersionFromParents
+
+        }
         return lastFoundVersion
     }
 
+    private fun markParentCommitsAsVisited(
+        liveBranchCount: Int,
+        currentCommit: Commit,
+        visitedCommits: MutableMap<String, MutableSemVersion?>,
+        commits: PriorityQueue<CommitData>
+    ) {
+        if (liveBranchCount == 0) {
+            return
+        }
+        currentCommit.parents.filter { !visitedCommits.containsKey(it.sha) }.forEach {
+            visitedCommits[it.sha] = null
+            commits.add(CommitData(it, ArrayList(1), true))
+        }
+    }
+
     private fun addToChangeLog(
         currentCommit: Commit,
         changeLog: MutableList<Commit>?,

src/test/kotlin/git/semver/plugin/changelog/ChangeLogFormatTest.kt

@@ -133,11 +133,11 @@ class ChangeLogFormatTest {
     fun format_no_grouping_nor_sorting() {
         val settings = SemverSettings()
         val changeLog = listOf(
-            Commit("fix: B", "1", emptySequence()),
-            Commit("fix: A", "2", emptySequence()),
-            Commit("ignore: B", "5", emptySequence()),
-            Commit("fix: B", "3", emptySequence()),
-            Commit("fix: A", "4", emptySequence()),
+            Commit("fix: B", "1", 0, emptySequence()),
+            Commit("fix: A", "2", 1, emptySequence()),
+            Commit("ignore: B", "5", 2, emptySequence()),
+            Commit("fix: B", "3", 3, emptySequence()),
+            Commit("fix: A", "4", 4, emptySequence()),
         )
         val c = ChangeLogTexts(mutableMapOf(
             "fix" to "FIX",
@@ -177,6 +177,6 @@ class ChangeLogFormatTest {
             "0100000" to "xyz: Some other change",
             "0110000" to "An uncategorized change"
         )
-        return changeLog.map { Commit(it.value, it.key, emptySequence()) }
+        return changeLog.map { Commit(it.value, it.key, 0, emptySequence()) }
     }
 }

src/test/kotlin/git/semver/plugin/gradle/GitSemverPluginExtensionTest.kt

@@ -62,7 +62,7 @@ class GitSemverPluginExtensionTest {
         }
 
         val actual = semver.changeLogFormat.formatLog(listOf(
-            Commit("test: Test Commit", "sha", emptySequence(), "John Doe", "john.doe@example.com", Date())),
+            Commit("test: Test Commit", "sha", 0, emptySequence(), "John Doe", "john.doe@example.com", Date())),
             semver.createSettings(),
             semver.changeLogTexts)
 

src/test/kotlin/git/semver/plugin/gradle/GitSemverPluginTest.kt

@@ -113,4 +113,19 @@ class GitSemverPluginTest {
             task.createRelease()
         }.doesNotThrowAnyException()
     }
+
+    @Test
+    fun `plugin version for a git directory`() {
+        val project = ProjectBuilder.builder().build()
+        project.plugins.apply("com.github.jmongard.git-semver-plugin")
+        val c = project.extensions.findByName("semver") as GitSemverPluginExtension
+
+//        c.gitDirectory.set(File("c:/dev/src/test1"))
+        c.gitDirectory.set(project.layout.projectDirectory)
+
+        val task = project.tasks.findByName("printVersion") as PrintTask
+
+        assertThat(task).isNotNull()
+        assertThatCode { task.print() }.doesNotThrowAnyException()
+    }
 }

src/test/kotlin/git/semver/plugin/semver/MutableSemVersionTest.kt

@@ -90,7 +90,7 @@ class MutableSemVersionTest {
     fun revisionString() {
         val settings = SemverSettings()
         val semver = MutableSemVersion.tryParse(Tag("1.2.3", SHA))!!
-        val commit = Commit("fix: a fix", SHA, emptySequence())
+        val commit = Commit("fix: a fix", SHA, 0, emptySequence())
         semver.updateFromCommit(commit,  settings, null)
         semver.updateFromCommit(commit,  settings, null)
         semver.updateFromCommit(commit,  settings, null)
@@ -99,8 +99,8 @@ class MutableSemVersionTest {
 
         val actual = semver.toSemVersion().revisionString()
 
-        assertThat(actual).isEqualTo("1.2.3.4");
-        assertThat(semver.toSemVersion()).hasToString("1.2.4+004.sha.8727a3e");
+        assertThat(actual).isEqualTo("1.2.3.4")
+        assertThat(semver.toSemVersion()).hasToString("1.2.4+004.sha.8727a3e")
     }
 
     @ParameterizedTest

src/test/kotlin/git/semver/plugin/semver/SemVersionFinderTest.kt

@@ -73,20 +73,20 @@ class SemVersionFinderTest {
             Tag("v1.3.1-RC", "SHA_B1")
         )
 
-        val a0 = Commit("fix: msg a0", "SHA_A0", sequenceOf())
-        val a1 = Commit("fix: msg a1", "SHA_A1", sequenceOf(a0))
+        val a0 = Commit("fix: msg a0", "SHA_A0", 0, sequenceOf())
+        val a1 = Commit("fix: msg a1", "SHA_A1", 1, sequenceOf(a0))
 
-        val b0 = Commit("fix: msg b0", "SHA_B0", sequenceOf(a1))
-        val b1 = Commit("fix: msg b1", "SHA_B1", sequenceOf(b0))
-        val b2 = Commit("fix: msg b2", "SHA_B2", sequenceOf(b1))
+        val b0 = Commit("fix: msg b0", "SHA_B0", 2, sequenceOf(a1))
+        val b1 = Commit("fix: msg b1", "SHA_B1", 4, sequenceOf(b0))
+        val b2 = Commit("fix: msg b2", "SHA_B2", 6, sequenceOf(b1))
 
-        val c0 = Commit("fix: msg c0", "SHA_C0", sequenceOf(a1))
-        val c1 = Commit("fix: msg c1", "SHA_C1", sequenceOf(c0))
-        val c2 = Commit("fix: msg c2", "SHA_C2", sequenceOf(c1))
-        val c3 = Commit("fix: msg c3", "SHA_C3", sequenceOf(c2))
+        val c0 = Commit("fix: msg c0", "SHA_C0", 3, sequenceOf(a1))
+        val c1 = Commit("fix: msg c1", "SHA_C1", 5, sequenceOf(c0))
+        val c2 = Commit("fix: msg c2", "SHA_C2", 7, sequenceOf(c1))
+        val c3 = Commit("fix: msg c3", "SHA_C3", 9, sequenceOf(c2))
 
-        val d0 = Commit("fix: msg d0", "SHA_D0", sequenceOf(c3, b2))
-        val d1 = Commit("fix: msg d1", "SHA_D1", sequenceOf(d0))
+        val d0 = Commit("fix: msg d0", "SHA_D0", 10, sequenceOf(c3, b2))
+        val d1 = Commit("fix: msg d1", "SHA_D1", 11, sequenceOf(d0))
 
         // when
         val versions = getVersion(tags, d1)
@@ -103,19 +103,19 @@ class SemVersionFinderTest {
             Tag("v0.4.0", "SHA0")
         )
 
-        val a0 = Commit("a msg1", "SHA0", sequenceOf())
-        val a1 = Commit("feat: a feature", "SHA1", sequenceOf(a0))
-        val a2 = Commit("a msg3", "SHA2", sequenceOf(a1))
+        val a0 = Commit("a msg1", "SHA0", 0, sequenceOf())
+        val a1 = Commit("feat: a feature", "SHA1", 1, sequenceOf(a0))
+        val a2 = Commit("a msg3", "SHA2", 2, sequenceOf(a1))
 
-        val b0 = Commit("fix: test 11", "SHA11", sequenceOf(a2))
-        val b1 = Commit("fix: test 12", "SHA12", sequenceOf(b0))
-        val b2 = Commit("fix: test 13", "SHA13", sequenceOf(b1))
+        val b0 = Commit("fix: test 11", "SHA11", 3, sequenceOf(a2))
+        val b1 = Commit("fix: test 12", "SHA12", 4, sequenceOf(b0))
+        val b2 = Commit("fix: test 13", "SHA13", 5, sequenceOf(b1))
 
-        val c0 = Commit("fix: test 21", "SHA21", sequenceOf(a2))
-        val c1 = Commit("fix: test 22", "SHA22", sequenceOf(c0))
+        val c0 = Commit("fix: test 21", "SHA21", 6, sequenceOf(a2))
+        val c1 = Commit("fix: test 22", "SHA22", 7, sequenceOf(c0))
 
-        val d0 = Commit("merge msg", "SHA31", sequenceOf(b2, c1))
-        val d1 = Commit("fix: msg", "SHA32", sequenceOf(d0))
+        val d0 = Commit("merge msg", "SHA31", 8, sequenceOf(b2, c1))
+        val d1 = Commit("fix: msg", "SHA32", 9, sequenceOf(d0))
 
         // when
         val versions = getVersion(tags, d1, groupVersions = false)
@@ -134,14 +134,14 @@ class SemVersionFinderTest {
             Tag("v0.4.2-Alpha.1", "SHA12"),
         )
 
-        val a0 = Commit("a msg1", "SHA0", sequenceOf())
-        val a1 = Commit("a msg2", "SHA1", sequenceOf(a0))
-        val a2 = Commit("a msg3", "SHA2", sequenceOf(a1))
+        val a0 = Commit("a msg1", "SHA0", 0, sequenceOf())
+        val a1 = Commit("a msg2", "SHA1", 2, sequenceOf(a0))
+        val a2 = Commit("a msg3", "SHA2", 3, sequenceOf(a1))
 
-        val b0 = Commit("fix: test 11", "SHA11", sequenceOf(a2))
-        val b1 = Commit("fix: test 12", "SHA12", sequenceOf(b0))
-        val b2 = Commit("fix: test 13", "SHA13", sequenceOf(b1))
-        val b3 = Commit("fix: msg", "SHA14", sequenceOf(b2))
+        val b0 = Commit("fix: test 11", "SHA11", 4, sequenceOf(a2))
+        val b1 = Commit("fix: test 12", "SHA12", 5, sequenceOf(b0))
+        val b2 = Commit("fix: test 13", "SHA13", 6, sequenceOf(b1))
+        val b3 = Commit("fix: msg", "SHA14", 7, sequenceOf(b2))
 
         // when
         val versions = getVersion(tags, b3, groupVersions = false)
@@ -433,6 +433,27 @@ class SemVersionFinderTest {
         assertEquals("1.0.0-RC.3+001", actual.toInfoVersionString())
     }
 
+    @Test
+    fun `test long lived develop branch should not count commits twice`() {
+        val m1 = Commit("Initial commit", "1", 1, emptySequence())
+        val d2 = Commit("fix: a fix", "2", 2, sequenceOf(m1))
+        val d3 = Commit("feat: a feat", "3", 3, sequenceOf(d2))
+        val m4 = Commit("merge branch develop", "4", 4, sequenceOf(m1, d3))
+        val m5 = Commit("release: v1.0.0", "5", 5, sequenceOf(m4))
+        val d6 = Commit("merge branch master into develop", "6", 6, sequenceOf(d3, m5))
+        val d7 = Commit("feat: a feat", "7", 7, sequenceOf(d6))
+        val d8 = Commit("feat: a feat", "8", 8, sequenceOf(d7))
+        val m9 = Commit("merge branch develop", "9", 9, sequenceOf(m5,d8))
+        val m10 = Commit("docs: some doc", "10", 10, sequenceOf(m9))
+        val m11 = Commit("release: 1.1.0", "11", 11, sequenceOf(m10))
+        val d12 = Commit("merge branch master into develop", "12", 12, sequenceOf(d8, m11))
+
+        val actual = getVersion(emptyList(), d12, false)
+
+        assertEquals("1.1.1-SNAPSHOT+001", actual.toInfoVersionString())
+    }
+
+
     @Test
     fun testIncrementVersion_dirty() {
         assertEquals("1.1.1-SNAPSHOT", getVersionFromTagAndDirty("v1.1.0"))
@@ -525,12 +546,12 @@ class SemVersionFinderTest {
 
     private fun asCommit(commits: List<String>) = asCommits(commits.reversed()).first()
 
-    private fun asCommits(commits: List<String>): Sequence<Commit> {
-        return commits.take(1).map { Commit("commit message", it, asCommits(commits.drop(1))) }.asSequence()
+    private fun asCommits(commits: List<String>, commitTime: Int = 0): Sequence<Commit> {
+        return commits.take(1).map { Commit("commit message", it, commitTime, asCommits(commits.drop(1), commitTime + 1)) }.asSequence()
     }
 
-    private fun asCommits(shas: Iterable<Pair<String, String>>): Sequence<Commit> {
-        return shas.take(1).map { Commit(it.second, it.first, asCommits(shas.drop(1))) }.asSequence()
+    private fun asCommits(shas: Iterable<Pair<String, String>>, commitTime: Int = 0): Sequence<Commit> {
+        return shas.take(1).map { Commit(it.second, it.first, commitTime,  asCommits(shas.drop(1), commitTime + 1)) }.asSequence()
     }
 
     private fun generateSHAString(range: IntRange): List<String> {