Skip to content

Commit

Permalink
Fix version logic when bumping major version (#38595)
Browse files Browse the repository at this point in the history 
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)

- Special logic so that the 7.0.0 build knows that we do not plan
   to have any 6.x releases after the 6.7 series
 
Backport of: #38593 
Partial Backport of: #38513

Co-authored-by: Jason Tedor <[email protected]>
  • Loading branch information
@tvernum @jasontedor
tvernum and jasontedor authored Feb 8, 2019
1 parent 5e798c1 commit 558b454
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 21 deletions.
67 changes: 54 additions & 13 deletions buildSrc/src/main/java/org/elasticsearch/gradle/VersionCollection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,30 +171,38 @@ public UnreleasedVersionInfo unreleasedInfo(Version version) {
}

public void forPreviousUnreleased(Consumer<UnreleasedVersionInfo> consumer) {
getUnreleased().stream()
List<UnreleasedVersionInfo> collect = getUnreleased().stream()
.filter(version -> version.equals(currentVersion) == false)
.forEach(version -> consumer.accept(
new UnreleasedVersionInfo(
.map(version -> new UnreleasedVersionInfo(
version,
getBranchFor(version),
getGradleProjectNameFor(version)
)
));
)
.collect(Collectors.toList());

collect.forEach(uvi -> consumer.accept(uvi));
}

private String getGradleProjectNameFor(Version version) {
if (version.equals(currentVersion)) {
throw new IllegalArgumentException("The Gradle project to build " + version + " is the current build.");
}

Map<Integer, List<Version>> releasedMajorGroupedByMinor = getReleasedMajorGroupedByMinor();

if (version.getRevision() == 0) {
if (releasedMajorGroupedByMinor
.get(releasedMajorGroupedByMinor.keySet().stream().max(Integer::compareTo).orElse(0))
.contains(version)) {
return "minor";
List<Version> unreleasedStagedOrMinor = getUnreleased().stream()
.filter(v -> v.getRevision() == 0)
.collect(Collectors.toList());
if (unreleasedStagedOrMinor.size() > 2) {
if (unreleasedStagedOrMinor.get(unreleasedStagedOrMinor.size() - 2).equals(version)) {
return "minor";
} else{
return "staged";
}
} else {
return "staged";
return "minor";
}
} else {
if (releasedMajorGroupedByMinor
Expand All @@ -210,7 +218,14 @@ private String getGradleProjectNameFor(Version version) {
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() && isFinalMinor(version) == false) {
return version.getMajor() + ".x";
} else {
return version.getMajor() + "." + version.getMinor();
}
case "staged":
case "maintenance":
case "bugfix":
Expand All @@ -220,13 +235,30 @@ private String getBranchFor(Version version) {
}
}

/**
* There is no way to infer that 6.7 is the final minor release in the 6.x series until we add a 7.0.1 or 7.1.0 version.
* Based on the available versions (7.0.0, 6.7.0, 6.6.1, 6.6.0) the logical conclusion is that 7.0.0 is "master" and 6.7.0 is "6.x"
* This method force 6.7.0 to be recognised as being on the "6.7" branch
*/
private boolean isFinalMinor(Version version) {
return (version.getMajor() == 6 && version.getMinor() == 7);
}

public List<Version> getUnreleased() {
List<Version> unreleased = new ArrayList<>();
// The current version is being worked, is always unreleased
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<Integer, List<Version>> groupByMinor = getReleasedMajorGroupedByMinor();
int greatestMinor = groupByMinor.keySet().stream().max(Integer::compareTo).orElse(0);
Expand All @@ -239,8 +271,10 @@ public List<Version> getUnreleased() {
unreleased.add(getLatestVersionByKey(groupByMinor, greatestMinor - 1));
if (groupByMinor.getOrDefault(greatestMinor - 1, emptyList()).size() == 1) {
// we found that the previous minor is staged but not yet released
// in this case, the minor before that has a bugfix
unreleased.add(getLatestVersionByKey(groupByMinor, greatestMinor - 2));
// in this case, the minor before that has a bugfix, should there be such a minor
if (greatestMinor >= 2) {
unreleased.add(getLatestVersionByKey(groupByMinor, greatestMinor - 2));
}
}
}

Expand All @@ -252,6 +286,13 @@ public List<Version> getUnreleased() {
);
}

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<Integer, List<Version>> groupByMajor, int key) {
return groupByMajor.getOrDefault(key, emptyList()).stream()
.max(Version::compareTo)
Expand Down
37 changes: 36 additions & 1 deletion buildSrc/src/test/java/org/elasticsearch/gradle/VersionCollectionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ public class VersionCollectionTests extends GradleUnitTestCase {
"6_0_0", "6_0_1", "6_1_0", "6_1_1", "6_1_2", "6_1_3", "6_1_4", "6_2_0", "6_2_1", "6_2_2", "6_2_3",
"6_2_4", "6_3_0", "6_3_1", "6_3_2", "6_4_0", "6_4_1", "6_4_2"
));
sampleVersions.put("7.0.0", asList(
"7_0_0", "6_7_0", "6_6_2", "6_6_1", "6_6_0"
));
sampleVersions.put("7.1.0", asList(
"7_1_0", "7_0_0", "6_7_0", "6_6_1", "6_6_0"
));
}

@Test(expected = IllegalArgumentException.class)
Expand Down Expand Up @@ -145,6 +151,11 @@ public void testWireCompatible() {
singletonList("7.3.0"),
getVersionCollection("8.0.0").getWireCompatible()
);
assertVersionsEquals(
asList("6.7.0", "7.0.0"),
getVersionCollection("7.1.0").getWireCompatible()
);

}

public void testWireCompatibleUnreleased() {
Expand All @@ -171,6 +182,10 @@ public void testWireCompatibleUnreleased() {
singletonList("7.3.0"),
getVersionCollection("8.0.0").getUnreleasedWireCompatible()
);
assertVersionsEquals(
asList("6.7.0", "7.0.0"),
getVersionCollection("7.1.0").getWireCompatible()
);
}

public void testIndexCompatible() {
Expand Down Expand Up @@ -270,6 +285,14 @@ public void testGetUnreleased() {
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()
);
assertVersionsEquals(
asList("6.6.2", "6.7.0", "7.0.0"),
getVersionCollection("7.0.0").getUnreleased()
);
}

public void testGetBranch() {
Expand All @@ -293,6 +316,14 @@ public void testGetBranch() {
asList("7.1", "7.2", "7.x"),
getVersionCollection("8.0.0")
);
assertUnreleasedBranchNames(
asList("6.6", "6.7", "7.0"),
getVersionCollection("7.1.0")
);
assertUnreleasedBranchNames(
asList("6.6", "6.7"),
getVersionCollection("7.0.0")
);
}

public void testGetGradleProjectName() {
Expand All @@ -309,13 +340,17 @@ public void testGetGradleProjectName() {
getVersionCollection("6.4.2")
);
assertUnreleasedGradleProjectNames(
asList("maintenance", "bugfix", "staged"),
asList("maintenance", "bugfix", "minor"),
getVersionCollection("6.6.0")
);
assertUnreleasedGradleProjectNames(
asList("bugfix", "staged", "minor"),
getVersionCollection("8.0.0")
);
assertUnreleasedGradleProjectNames(
asList("maintenance", "staged", "minor"),
getVersionCollection("7.1.0")
);
}

public void testCompareToAuthoritative() {
Expand Down
27 changes: 20 additions & 7 deletions test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Version>, List<Version>> resolveReleasedVersions(Version current, Class<?> versionClass) {
// group versions into major version
Expand All @@ -52,7 +52,7 @@ static Tuple<List<Version>, List<Version>> resolveReleasedVersions(Version curre
// this breaks b/c 5.x is still in version list but master doesn't care about it!
//assert majorVersions.size() == 2;
// TODO: remove oldVersions, we should only ever have 2 majors in Version
List<Version> oldVersions = majorVersions.getOrDefault((int)current.major - 2, Collections.emptyList());
List<List<Version>> oldVersions = splitByMinor(majorVersions.getOrDefault((int)current.major - 2, Collections.emptyList()));
List<List<Version>> previousMajor = splitByMinor(majorVersions.get((int)current.major - 1));
List<List<Version>> currentMajor = splitByMinor(majorVersions.get((int)current.major));

Expand All @@ -67,7 +67,11 @@ static Tuple<List<Version>, List<Version>> resolveReleasedVersions(Version curre
// 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
Expand All @@ -78,12 +82,21 @@ static Tuple<List<Version>, List<Version>> resolveReleasedVersions(Version curre
moveLastToUnreleased(stableVersions, unreleasedVersions);
}
// remove the next bugfix
moveLastToUnreleased(stableVersions, unreleasedVersions);
if (stableVersions.isEmpty() == false) {
moveLastToUnreleased(stableVersions, unreleasedVersions);
}
}

List<Version> releasedVersions = Stream.concat(oldVersions.stream(),
Stream.concat(previousMajor.stream(), currentMajor.stream()).flatMap(List::stream))
.collect(Collectors.toList());
// If none of the previous major was released, then the last minor and bugfix of the old version was not released either.
if (previousMajor.isEmpty()) {
assert currentMajor.isEmpty() : currentMajor;
// minor of the old version is being staged
moveLastToUnreleased(oldVersions, unreleasedVersions);
// bugix of the old version is also being staged
moveLastToUnreleased(oldVersions, unreleasedVersions);
}
List<Version> releasedVersions = Stream.of(oldVersions, previousMajor, currentMajor)
.flatMap(List::stream).flatMap(List::stream).collect(Collectors.toList());
Collections.sort(unreleasedVersions); // we add unreleased out of order, so need to sort here
return new Tuple<>(Collections.unmodifiableList(releasedVersions), Collections.unmodifiableList(unreleasedVersions));
}
Expand Down

0 comments on commit 558b454

Please sign in to comment.