From 84483b26cfd293fb90f4fa09c0cd55656ab6b684 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Fri, 8 Feb 2019 18:00:03 +1100 Subject: [PATCH] Fix version logic when bumping major version (#38593) When we are preparing to release a major version the rules around "unreleased" versions and branches get a bit more complex. This change implements the following rules: - If the tip version on the previous major is a .0 (e.g. 6.7.0) then the tip of the minor before that (e.g. 6.6.1) must be unreleased. (This is because 6.7.0 would be "staged" in preparation for release, but 6.6.1 would be open for bug fixes on the release 6.6.x line) (in VersionCollection & VersionUtils) - The "major.x" branch (if it exists) will always point to the latest minor in that series. Anything that is not the latest minor, must therefore be on a the "major.minor" branch For example, if v7.1.0 exists then the "7.x" branch must be 7.1.0, and 7.0.0 must be on the "7.0" branch (in VersionCollection) --- .../gradle/VersionCollection.java | 26 +++++++++++++++++-- .../gradle/VersionCollectionTests.java | 12 +++++++-- .../org/elasticsearch/test/VersionUtils.java | 8 ++++-- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/VersionCollection.java b/buildSrc/src/main/java/org/elasticsearch/gradle/VersionCollection.java index cf8eb1829ab..96fa2074214 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/VersionCollection.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/VersionCollection.java @@ -218,7 +218,14 @@ public class VersionCollection { private String getBranchFor(Version version) { switch (getGradleProjectNameFor(version)) { case "minor": - return version.getMajor() + ".x"; + // The .x branch will always point to the latest minor (for that major), so a "minor" project will be on the .x branch + // unless there is more recent (higher) minor. + final Version latestInMajor = getLatestVersionByKey(groupByMajor, version.getMajor()); + if (latestInMajor.getMinor() == version.getMinor()) { + return version.getMajor() + ".x"; + } else { + return version.getMajor() + "." + version.getMinor(); + } case "staged": case "maintenance": case "bugfix": @@ -234,7 +241,15 @@ public class VersionCollection { unreleased.add(currentVersion); // the tip of the previous major is unreleased for sure, be it a minor or a bugfix - unreleased.add(getLatestVersionByKey(this.groupByMajor, currentVersion.getMajor() - 1)); + final Version latestOfPreviousMajor = getLatestVersionByKey(this.groupByMajor, currentVersion.getMajor() - 1); + unreleased.add(latestOfPreviousMajor); + if (latestOfPreviousMajor.getRevision() == 0) { + // if the previous major is a x.y.0 release, then the tip of the minor before that (y-1) is also unreleased + final Version previousMinor = getLatestInMinor(latestOfPreviousMajor.getMajor(), latestOfPreviousMajor.getMinor() - 1); + if (previousMinor != null) { + unreleased.add(previousMinor); + } + } final Map> groupByMinor = getReleasedMajorGroupedByMinor(); int greatestMinor = groupByMinor.keySet().stream().max(Integer::compareTo).orElse(0); @@ -262,6 +277,13 @@ public class VersionCollection { ); } + private Version getLatestInMinor(int major, int minor) { + return groupByMajor.get(major).stream() + .filter(v -> v.getMinor() == minor) + .max(Version::compareTo) + .orElse(null); + } + private Version getLatestVersionByKey(Map> groupByMajor, int key) { return groupByMajor.getOrDefault(key, emptyList()).stream() .max(Version::compareTo) diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/VersionCollectionTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/VersionCollectionTests.java index 4ea33b240e5..01387f974c4 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/VersionCollectionTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/VersionCollectionTests.java @@ -282,6 +282,10 @@ public class VersionCollectionTests extends GradleUnitTestCase { asList("7.1.1", "7.2.0", "7.3.0", "8.0.0"), getVersionCollection("8.0.0").getUnreleased() ); + assertVersionsEquals( + asList("6.6.1", "6.7.0", "7.0.0", "7.1.0"), + getVersionCollection("7.1.0").getUnreleased() + ); } public void testGetBranch() { @@ -298,13 +302,17 @@ public class VersionCollectionTests extends GradleUnitTestCase { getVersionCollection("6.4.2") ); assertUnreleasedBranchNames( - asList("5.6", "6.4", "6.x"), + asList("5.6", "6.4", "6.5"), getVersionCollection("6.6.0") ); assertUnreleasedBranchNames( asList("7.1", "7.2", "7.x"), getVersionCollection("8.0.0") ); + assertUnreleasedBranchNames( + asList("6.6", "6.7", "7.0"), + getVersionCollection("7.1.0") + ); } public void testGetGradleProjectName() { @@ -329,7 +337,7 @@ public class VersionCollectionTests extends GradleUnitTestCase { getVersionCollection("8.0.0") ); assertUnreleasedGradleProjectNames( - asList("staged", "minor"), + asList("maintenance", "staged", "minor"), getVersionCollection("7.1.0") ); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java b/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java index f775f9f5b01..20c4601c199 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java @@ -43,7 +43,7 @@ public class VersionUtils { * rules here match up with the rules in gradle then this should * produce sensible results. * @return a tuple containing versions with backwards compatibility - * guarantees in v1 and versions without the guranteees in v2 + * guarantees in v1 and versions without the guarantees in v2 */ static Tuple, List> resolveReleasedVersions(Version current, Class versionClass) { // group versions into major version @@ -67,7 +67,11 @@ public class VersionUtils { // on a stable or release branch, ie N.x stableVersions = currentMajor; // remove the next maintenance bugfix - moveLastToUnreleased(previousMajor, unreleasedVersions); + final Version prevMajorLastMinor = moveLastToUnreleased(previousMajor, unreleasedVersions); + if (prevMajorLastMinor.revision == 0 && previousMajor.isEmpty() == false) { + // The latest minor in the previous major is a ".0" release, so there must be an unreleased bugfix for the minor before that + moveLastToUnreleased(previousMajor, unreleasedVersions); + } } // remove next minor