Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Teach the build about betas and rcs #26066

Merged
merged 7 commits into from
Aug 10, 2017
Merged
Show file tree
Hide file tree
Changes from 5 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
25 changes: 15 additions & 10 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ configure(subprojects.findAll { it.projectDir.toPath().startsWith(rootPath) }) {

/* Introspect all versions of ES that may be tested agains for backwards
* compatibility. It is *super* important that this logic is the same as the
* logic in VersionUtils.java, modulo alphas, betas, and rcs which are ignored
Copy link
Member Author

Choose a reason for hiding this comment

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

This was out of date.

* in gradle because they don't have any backwards compatibility guarantees
* but are not ignored in VersionUtils.java because the tests expect them not
* to be. */
* logic in VersionUtils.java, throwing out alphas because they don't have any
* backwards compatibility guarantees and only keeping the latest beta or rc
* in a branch if there are only betas and rcs in the branch so we have
* *something* to test against. */
Version currentVersion = Version.fromString(VersionProperties.elasticsearch.minus('-SNAPSHOT'))
int prevMajor = currentVersion.major - 1
File versionFile = file('core/src/main/java/org/elasticsearch/Version.java')
Expand All @@ -84,11 +84,16 @@ for (String line : versionLines) {
int major = Integer.parseInt(match.group(1))
int minor = Integer.parseInt(match.group(2))
int bugfix = Integer.parseInt(match.group(3))
Version foundVersion = new Version(major, minor, bugfix, false)
String suffix = (match.group(4) ?: '').replace('_', '-')
Version foundVersion = new Version(major, minor, bugfix, suffix, false)
if (currentVersion != foundVersion
&& (major == prevMajor || major == currentVersion.major)
&& (versions.isEmpty() || versions.last() != foundVersion)) {
versions.add(foundVersion)
Copy link
Member Author

Choose a reason for hiding this comment

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

This was keeping beta-1 when it should have been keeping beta-2.

&& (major == prevMajor || major == currentVersion.major)) {
if (versions.isEmpty() || versions.last() != foundVersion) {
versions.add(foundVersion)
} else {
// Replace the earlier betas with later ones
versions.set(versions.size() - 1, foundVersion)
Copy link
Member

Choose a reason for hiding this comment

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

Can you assert/error here if foundVersion and/or the previous version.last() is not a beta/rc?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

}
if (major == prevMajor && minor > lastPrevMinor) {
prevMinorIndex = versions.size() - 1
lastPrevMinor = minor
Expand All @@ -106,10 +111,10 @@ if (currentVersion.bugfix == 0) {
// unreleased version of closest branch. So for those cases, the version includes -SNAPSHOT,
// and the bwc distribution will checkout and build that version.
Version last = versions[-1]
versions[-1] = new Version(last.major, last.minor, last.bugfix, true)
versions[-1] = new Version(last.major, last.minor, last.bugfix, last.suffix, true)
if (last.bugfix == 0) {
versions[-2] = new Version(
versions[-2].major, versions[-2].minor, versions[-2].bugfix, true)
versions[-2].major, versions[-2].minor, versions[-2].bugfix, versions[-2].suffix, true)
}
}

Expand Down
27 changes: 17 additions & 10 deletions buildSrc/src/main/groovy/org/elasticsearch/gradle/Version.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
package org.elasticsearch.gradle

import groovy.transform.Sortable
import java.util.regex.Matcher
import org.gradle.api.InvalidUserDataException

/**
* Encapsulates comparison and printing logic for an x.y.z version.
Expand All @@ -32,32 +34,37 @@ public class Version {
final int bugfix
final int id
final boolean snapshot
/**
* Suffix on the version name. Unlike Version.java the build does not
* consider alphas and betas different versions, it just preserves the
* suffix that the version was declared with in Version.java.
*/
final String suffix

public Version(int major, int minor, int bugfix, boolean snapshot) {
public Version(int major, int minor, int bugfix,
String suffix, boolean snapshot) {
this.major = major
this.minor = minor
this.bugfix = bugfix
this.snapshot = snapshot
this.suffix = suffix
this.id = major * 100000 + minor * 1000 + bugfix * 10 +
(snapshot ? 1 : 0)
}

public static Version fromString(String s) {
String[] parts = s.split('\\.')
String bugfix = parts[2]
boolean snapshot = false
if (bugfix.contains('-')) {
snapshot = bugfix.endsWith('-SNAPSHOT')
bugfix = bugfix.split('-')[0]
Matcher m = s =~ /(\d+)\.(\d+)\.(\d+)(-alpha\d+|-beta\d+|-rc\d+)?(-SNAPSHOT)?/
if (m.matches() == false) {
throw new InvalidUserDataException("Invalid version [${s}]")
}
return new Version(parts[0] as int, parts[1] as int, bugfix as int,
snapshot)
return new Version(m.group(1) as int, m.group(2) as int,
m.group(3) as int, m.group(4) ?: '', m.group(5) != null)
}

@Override
public String toString() {
String snapshotStr = snapshot ? '-SNAPSHOT' : ''
return "${major}.${minor}.${bugfix}${snapshotStr}"
return "${major}.${minor}.${bugfix}${suffix}${snapshotStr}"
}

public boolean before(String compareTo) {
Expand Down
8 changes: 8 additions & 0 deletions distribution/bwc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,14 @@ if (enabled) {
dependsOn checkoutBwcBranch
dir = checkoutDir
tasks = [':distribution:deb:assemble', ':distribution:rpm:assemble', ':distribution:zip:assemble']
doLast {
List missing = [bwcDeb, bwcRpm, bwcZip].grep { file ->
false == file.exists() }
if (false == missing.empty) {
throw new InvalidUserDataException(
"Building bwc version didn't generate expected files ${missing}")
}
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,15 @@ public void testGradleVersionsMatchVersionUtils() {
// First check the index compatible versions
VersionsFromProperty indexCompatible = new VersionsFromProperty("tests.gradle_index_compat_versions");
List<Version> released = VersionUtils.allReleasedVersions().stream()
/* We skip alphas, betas, and the like in gradle because they don't have
* backwards compatibility guarantees even though they are technically
* released. */
.filter(v -> v.isRelease() && (v.major == Version.CURRENT.major || v.major == Version.CURRENT.major - 1))
// Java lists some non-index compatible versions but gradle does not include them.
.filter(v -> v.major == Version.CURRENT.major || v.major == Version.CURRENT.major - 1)
/* Gradle will never include *released* alphas or betas because it will prefer
* the unreleased branch head. Gradle is willing to use branch heads that are
* beta or rc so that we have *something* to test against even though we
* do not offer backwards compatibility for alphas, betas, or rcs. */
.filter(Version::isRelease)
.collect(toList());

List<String> releasedIndexCompatible = released.stream()
.map(Object::toString)
.collect(toList());
Expand All @@ -195,7 +199,15 @@ public void testGradleVersionsMatchVersionUtils() {
/* Gradle skips the current version because being backwards compatible
* with yourself is implied. Java lists the version because it is useful. */
.filter(v -> v != Version.CURRENT)
.map(v -> v.major + "." + v.minor + "." + v.revision)
/* Note that gradle skips alphas because they don't have any backwards
* compatibility guarantees but keeps the last beta and rc in a branch
* on when there are only betas an RCs in that branch so that we have
* *something* to test that branch against. There is no need to recreate
* that logic here because allUnreleasedVersions already only contains
* the heads of branches so it should be good enough to just keep all
* the non-alphas.*/
.filter(v -> false == v.isAlpha())
.map(Object::toString)
.collect(toCollection(LinkedHashSet::new)));
assertEquals(unreleasedIndexCompatible, indexCompatible.unreleased);

Expand Down