From edeef5165d771356a9f4abdaad0e2596311765e2 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 13 Apr 2018 13:22:57 -0400 Subject: [PATCH 1/3] Make NodeInfo#nodeVersion strongly-typed as Version Today we have a nodeVersion property on the NodeInfo class that we use to carry around information about a standalone node that we will start during tests. This property is a String which we usually end up parsing to a Version anyway to do various checks on it. This can end up happening a lot during configuration so it would be more efficient and safer to have this already be strongly-typed as a Version and parsed from a String only once for each instance of NodeInfo. Therefore, this commit makes NodeInfo#nodeVersion strongly-typed as a Version. --- .../gradle/test/ClusterFormationTasks.groovy | 21 +++++++++++-------- .../elasticsearch/gradle/test/NodeInfo.groovy | 12 +++++------ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy index f0b232c0d47ae..87a7a03f6ef5e 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy @@ -99,7 +99,7 @@ class ClusterFormationTasks { configureDistributionDependency(project, config.distribution, bwcDistro, config.bwcVersion) for (Map.Entry entry : config.plugins.entrySet()) { - configureBwcPluginDependency("${prefix}_elasticsearchBwcPlugins", project, entry.getValue(), bwcPlugins, config.bwcVersion) + configureBwcPluginDependency("${prefix}_elasticsearchBwcPlugins", project, entry.getValue(), bwcPlugins, Version.fromString(config.bwcVersion)) } bwcDistro.resolutionStrategy.cacheChangingModulesFor(0, TimeUnit.SECONDS) bwcPlugins.resolutionStrategy.cacheChangingModulesFor(0, TimeUnit.SECONDS) @@ -107,11 +107,14 @@ class ClusterFormationTasks { for (int i = 0; i < config.numNodes; i++) { // we start N nodes and out of these N nodes there might be M bwc nodes. // for each of those nodes we might have a different configuration - String elasticsearchVersion = VersionProperties.elasticsearch - Configuration distro = currentDistro + final Configuration distro + final Version elasticsearchVersion if (i < config.numBwcNodes) { - elasticsearchVersion = config.bwcVersion + elasticsearchVersion = Version.fromString(config.bwcVersion) distro = bwcDistro + } else { + elasticsearchVersion = Version.fromString(VersionProperties.elasticsearch) + distro = currentDistro } NodeInfo node = new NodeInfo(config, i, project, prefix, elasticsearchVersion, sharedDir) nodes.add(node) @@ -137,7 +140,7 @@ class ClusterFormationTasks { } /** Adds a dependency on a different version of the given plugin, which will be retrieved using gradle's dependency resolution */ - static void configureBwcPluginDependency(String name, Project project, Project pluginProject, Configuration configuration, String elasticsearchVersion) { + static void configureBwcPluginDependency(String name, Project project, Project pluginProject, Configuration configuration, Version elasticsearchVersion) { verifyProjectHasBuildPlugin(name, elasticsearchVersion, project, pluginProject) final String pluginName = findPluginName(pluginProject) project.dependencies.add(configuration.name, "org.elasticsearch.plugin:${pluginName}:${elasticsearchVersion}@zip") @@ -183,7 +186,7 @@ class ClusterFormationTasks { setup = configureAddKeystoreFileTasks(prefix, project, setup, node) if (node.config.plugins.isEmpty() == false) { - if (node.nodeVersion == VersionProperties.elasticsearch) { + if (node.nodeVersion == Version.fromString(VersionProperties.elasticsearch)) { setup = configureCopyPluginsTask(taskName(prefix, node, 'copyPlugins'), project, setup, node, prefix) } else { setup = configureCopyBwcPluginsTask(taskName(prefix, node, 'copyBwcPlugins'), project, setup, node, prefix) @@ -303,7 +306,7 @@ class ClusterFormationTasks { // Default the watermarks to absurdly low to prevent the tests from failing on nodes without enough disk space esConfig['cluster.routing.allocation.disk.watermark.low'] = '1b' esConfig['cluster.routing.allocation.disk.watermark.high'] = '1b' - if (Version.fromString(node.nodeVersion).major >= 6) { + if (node.nodeVersion.major >= 6) { esConfig['cluster.routing.allocation.disk.watermark.flood_stage'] = '1b' } // increase script compilation limit since tests can rapid-fire script compilations @@ -514,7 +517,7 @@ class ClusterFormationTasks { static Task configureInstallPluginTask(String name, Project project, Task setup, NodeInfo node, Project plugin, String prefix) { final FileCollection pluginZip; - if (node.nodeVersion != VersionProperties.elasticsearch) { + if (node.nodeVersion != Version.fromString(VersionProperties.elasticsearch)) { pluginZip = project.configurations.getByName(pluginBwcConfigurationName(prefix, plugin)) } else { pluginZip = project.configurations.getByName(pluginConfigurationName(prefix, plugin)) @@ -803,7 +806,7 @@ class ClusterFormationTasks { return retVal } - static void verifyProjectHasBuildPlugin(String name, String version, Project project, Project pluginProject) { + static void verifyProjectHasBuildPlugin(String name, Version version, Project project, Project pluginProject) { if (pluginProject.plugins.hasPlugin(PluginBuildPlugin) == false && pluginProject.plugins.hasPlugin(MetaPluginBuildPlugin) == false) { throw new GradleException("Task [${name}] cannot add plugin [${pluginProject.path}] with version [${version}] to project's " + "[${project.path}] dependencies: the plugin is not an esplugin or es_meta_plugin") diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy index 2898df0445f04..0f23db5d5f812 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy @@ -102,10 +102,10 @@ class NodeInfo { ByteArrayOutputStream buffer = new ByteArrayOutputStream() /** the version of elasticsearch that this node runs */ - String nodeVersion + Version nodeVersion /** Holds node configuration for part of a test cluster. */ - NodeInfo(ClusterConfiguration config, int nodeNum, Project project, String prefix, String nodeVersion, File sharedDir) { + NodeInfo(ClusterConfiguration config, int nodeNum, Project project, String prefix, Version nodeVersion, File sharedDir) { this.config = config this.nodeNum = nodeNum this.sharedDir = sharedDir @@ -166,13 +166,13 @@ class NodeInfo { final String javaHome final Map javaVersions = project.javaVersions - if (Version.fromString(nodeVersion).before("6.2.0")) { + if (nodeVersion.before("6.2.0")) { final String java8Home = javaVersions.get(8) if (java8Home == null) { throw new GradleException("JAVA8_HOME must be set to run BWC tests against [" + nodeVersion + "]") } javaHome = java8Home - } else if (Version.fromString(nodeVersion).onOrAfter("6.2.0") && Version.fromString(nodeVersion).before("6.3.0")) { + } else if (nodeVersion.onOrAfter("6.2.0") && nodeVersion.before("6.3.0")) { final String java9Home = javaVersions.get(9) if (java9Home == null) { throw new GradleException("JAVA9_HOME must be set to run BWC tests against [" + nodeVersion + "]") @@ -304,7 +304,7 @@ class NodeInfo { } /** Returns the directory elasticsearch home is contained in for the given distribution */ - static File homeDir(File baseDir, String distro, String nodeVersion) { + static File homeDir(File baseDir, String distro, Version nodeVersion) { String path switch (distro) { case 'integ-test-zip': @@ -322,7 +322,7 @@ class NodeInfo { return new File(baseDir, path) } - static File pathConf(File baseDir, String distro, String nodeVersion) { + static File pathConf(File baseDir, String distro, Version nodeVersion) { switch (distro) { case 'integ-test-zip': case 'zip': From ee06d86a172fcb0a8408e106184c3fe591f3eb4e Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 13 Apr 2018 14:38:24 -0400 Subject: [PATCH 2/3] Make bwcVersion a Version too --- build.gradle | 6 ++--- .../elasticsearch/gradle/BuildPlugin.groovy | 4 ++-- .../org/elasticsearch/gradle/Version.groovy | 24 +++++++++++++++---- .../gradle/VersionProperties.groovy | 4 ++-- .../gradle/doc/DocsTestPlugin.groovy | 2 +- .../gradle/plugin/PluginPropertiesTask.groovy | 2 +- .../gradle/test/ClusterConfiguration.groovy | 3 ++- .../gradle/test/ClusterFormationTasks.groovy | 12 +++++----- 8 files changed, 37 insertions(+), 20 deletions(-) diff --git a/build.gradle b/build.gradle index dce2adf5ee0bd..ae472cc134095 100644 --- a/build.gradle +++ b/build.gradle @@ -36,7 +36,7 @@ import java.security.MessageDigest // common maven publishing configuration subprojects { group = 'org.elasticsearch' - version = VersionProperties.elasticsearch + version = VersionProperties.elasticsearch.toString() description = "Elasticsearch subproject ${project.path}" } @@ -80,7 +80,7 @@ configure(subprojects.findAll { it.projectDir.toPath().startsWith(rootPath) }) { * in a branch if there are only betas and rcs in the branch so we have * *something* to test against. */ VersionCollection versions = new VersionCollection(file('server/src/main/java/org/elasticsearch/Version.java').readLines('UTF-8')) -if (versions.currentVersion.toString() != VersionProperties.elasticsearch) { +if (versions.currentVersion != VersionProperties.elasticsearch) { throw new GradleException("The last version in Versions.java [${versions.currentVersion}] does not match " + "VersionProperties.elasticsearch [${VersionProperties.elasticsearch}]") } @@ -245,7 +245,7 @@ subprojects { // other packages (e.g org.elasticsearch.client) will point to server rather than // their own artifacts. if (project.plugins.hasPlugin(BuildPlugin)) { - String artifactsHost = VersionProperties.elasticsearch.endsWith("-SNAPSHOT") ? "https://snapshots.elastic.co" : "https://artifacts.elastic.co" + String artifactsHost = VersionProperties.elasticsearch.toString().endsWith("-SNAPSHOT") ? "https://snapshots.elastic.co" : "https://artifacts.elastic.co" Closure sortClosure = { a, b -> b.group <=> a.group } Closure depJavadocClosure = { dep -> if (dep.group != null && dep.group.startsWith('org.elasticsearch')) { diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index 444f2283be4e7..d6deea1bd21ac 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -550,8 +550,8 @@ class BuildPlugin implements Plugin { jarTask.destinationDir = new File(project.buildDir, 'distributions') // fixup the jar manifest jarTask.doFirst { - boolean isSnapshot = VersionProperties.elasticsearch.endsWith("-SNAPSHOT"); - String version = VersionProperties.elasticsearch; + final String version = VersionProperties.elasticsearch.toString() + boolean isSnapshot = version.endsWith("-SNAPSHOT"); if (isSnapshot) { version = version.substring(0, version.length() - 9) } diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/Version.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/Version.groovy index 419d3792bb616..c28738d7695eb 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/Version.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/Version.groovy @@ -74,20 +74,36 @@ public class Version { return "${major}.${minor}.${revision}${suffix}${snapshotStr}" } + public boolean before(Version compareTo) { + return id < compareTo.id + } + public boolean before(String compareTo) { - return id < fromString(compareTo).id + return before(fromString(compareTo)) + } + + public boolean onOrBefore(Version compareTo) { + return id <= compareTo.id } public boolean onOrBefore(String compareTo) { - return id <= fromString(compareTo).id + return onOrBefore(fromString(compareTo)) + } + + public boolean onOrAfter(Version compareTo) { + return id >= compareTo.id } public boolean onOrAfter(String compareTo) { - return id >= fromString(compareTo).id + return onOrAfter(fromString(compareTo)) + } + + public boolean after(Version compareTo) { + return id > compareTo.id } public boolean after(String compareTo) { - return id > fromString(compareTo).id + return after(fromString(compareTo)) } public boolean onOrBeforeIncludingSuffix(Version otherVersion) { diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.groovy index c24431b4cbc1b..6983d12872f23 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.groovy @@ -22,7 +22,7 @@ package org.elasticsearch.gradle * Accessor for shared dependency versions used by elasticsearch, namely the elasticsearch and lucene versions. */ class VersionProperties { - static final String elasticsearch + static final Version elasticsearch static final String lucene static final Map versions = new HashMap<>() static { @@ -32,7 +32,7 @@ class VersionProperties { throw new RuntimeException('/version.properties resource missing') } props.load(propsStream) - elasticsearch = props.getProperty('elasticsearch') + elasticsearch = Version.fromString(props.getProperty('elasticsearch')) lucene = props.getProperty('lucene') for (String property : props.stringPropertyNames()) { versions.put(property, props.getProperty(property)) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy index d2802638ce512..f674dbd33cdfd 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy @@ -41,7 +41,7 @@ public class DocsTestPlugin extends RestTestPlugin { * to the version being built for testing but needs to resolve to * the last released version for docs. */ '\\{version\\}': - VersionProperties.elasticsearch.replace('-SNAPSHOT', ''), + VersionProperties.elasticsearch.toString().replace('-SNAPSHOT', ''), '\\{lucene_version\\}' : VersionProperties.lucene.replaceAll('-snapshot-\\w+$', ''), ] Task listSnippets = project.tasks.create('listSnippets', SnippetsTask) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy index f5dbcfd8b0d48..8e913153f05ad 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy @@ -77,7 +77,7 @@ class PluginPropertiesTask extends Copy { 'name': extension.name, 'description': extension.description, 'version': stringSnap(extension.version), - 'elasticsearchVersion': stringSnap(VersionProperties.elasticsearch), + 'elasticsearchVersion': stringSnap(VersionProperties.elasticsearch.toString()), 'javaVersion': project.targetCompatibility as String, 'classname': extension.classname, 'extendedPlugins': extension.extendedPlugins.join(','), diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy index 1e609c5e05f85..5aaf54454e137 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy @@ -18,6 +18,7 @@ */ package org.elasticsearch.gradle.test +import org.elasticsearch.gradle.Version import org.gradle.api.GradleException import org.gradle.api.Project import org.gradle.api.tasks.Input @@ -37,7 +38,7 @@ class ClusterConfiguration { int numBwcNodes = 0 @Input - String bwcVersion = null + Version bwcVersion = null @Input int httpPort = 0 diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy index 87a7a03f6ef5e..5f9e4c49b34e9 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy @@ -99,7 +99,7 @@ class ClusterFormationTasks { configureDistributionDependency(project, config.distribution, bwcDistro, config.bwcVersion) for (Map.Entry entry : config.plugins.entrySet()) { - configureBwcPluginDependency("${prefix}_elasticsearchBwcPlugins", project, entry.getValue(), bwcPlugins, Version.fromString(config.bwcVersion)) + configureBwcPluginDependency("${prefix}_elasticsearchBwcPlugins", project, entry.getValue(), bwcPlugins, config.bwcVersion) } bwcDistro.resolutionStrategy.cacheChangingModulesFor(0, TimeUnit.SECONDS) bwcPlugins.resolutionStrategy.cacheChangingModulesFor(0, TimeUnit.SECONDS) @@ -110,10 +110,10 @@ class ClusterFormationTasks { final Configuration distro final Version elasticsearchVersion if (i < config.numBwcNodes) { - elasticsearchVersion = Version.fromString(config.bwcVersion) + elasticsearchVersion = config.bwcVersion distro = bwcDistro } else { - elasticsearchVersion = Version.fromString(VersionProperties.elasticsearch) + elasticsearchVersion = VersionProperties.elasticsearch distro = currentDistro } NodeInfo node = new NodeInfo(config, i, project, prefix, elasticsearchVersion, sharedDir) @@ -129,7 +129,7 @@ class ClusterFormationTasks { } /** Adds a dependency on the given distribution */ - static void configureDistributionDependency(Project project, String distro, Configuration configuration, String elasticsearchVersion) { + static void configureDistributionDependency(Project project, String distro, Configuration configuration, Version elasticsearchVersion) { String packaging = distro if (distro == 'tar') { packaging = 'tar.gz' @@ -186,7 +186,7 @@ class ClusterFormationTasks { setup = configureAddKeystoreFileTasks(prefix, project, setup, node) if (node.config.plugins.isEmpty() == false) { - if (node.nodeVersion == Version.fromString(VersionProperties.elasticsearch)) { + if (node.nodeVersion == VersionProperties.elasticsearch) { setup = configureCopyPluginsTask(taskName(prefix, node, 'copyPlugins'), project, setup, node, prefix) } else { setup = configureCopyBwcPluginsTask(taskName(prefix, node, 'copyBwcPlugins'), project, setup, node, prefix) @@ -517,7 +517,7 @@ class ClusterFormationTasks { static Task configureInstallPluginTask(String name, Project project, Task setup, NodeInfo node, Project plugin, String prefix) { final FileCollection pluginZip; - if (node.nodeVersion != Version.fromString(VersionProperties.elasticsearch)) { + if (node.nodeVersion != VersionProperties.elasticsearch) { pluginZip = project.configurations.getByName(pluginBwcConfigurationName(prefix, plugin)) } else { pluginZip = project.configurations.getByName(pluginConfigurationName(prefix, plugin)) From be69351cfcabd284042cbcc573f5fe03c69e07b5 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 13 Apr 2018 14:50:01 -0400 Subject: [PATCH 3/3] An improvement? --- .../org/elasticsearch/gradle/BuildPlugin.groovy | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index d6deea1bd21ac..46dd3b902dc12 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -550,17 +550,18 @@ class BuildPlugin implements Plugin { jarTask.destinationDir = new File(project.buildDir, 'distributions') // fixup the jar manifest jarTask.doFirst { - final String version = VersionProperties.elasticsearch.toString() - boolean isSnapshot = version.endsWith("-SNAPSHOT"); - if (isSnapshot) { - version = version.substring(0, version.length() - 9) - } + final Version versionWithoutSnapshot = new Version( + VersionProperties.elasticsearch.major, + VersionProperties.elasticsearch.minor, + VersionProperties.elasticsearch.revision, + VersionProperties.elasticsearch.suffix, + false) // this doFirst is added before the info plugin, therefore it will run // after the doFirst added by the info plugin, and we can override attributes jarTask.manifest.attributes( - 'X-Compile-Elasticsearch-Version': version, + 'X-Compile-Elasticsearch-Version': versionWithoutSnapshot, 'X-Compile-Lucene-Version': VersionProperties.lucene, - 'X-Compile-Elasticsearch-Snapshot': isSnapshot, + 'X-Compile-Elasticsearch-Snapshot': VersionProperties.elasticsearch.isSnapshot(), 'Build-Date': ZonedDateTime.now(ZoneOffset.UTC), 'Build-Java-Version': project.compilerJavaVersion) if (jarTask.manifest.attributes.containsKey('Change') == false) {