Adapt to upstream for type safe build globals #1388
Conversation
This commit adapts the elasticsearch-hadoop build to an upstream change to make some build globals type safe. In particular, we adapt the usage of Java versions.
mark-vieira
left a comment
mark-vieira
left a comment
There was a problem hiding this comment.
@jasontedor thanks for doing this. There's some room for further refactoring to use BuildParams over extra properties. If this PR is just enough to get things to work I'm good with than and would be glad to tackle the more substantial cleanup myself.
| project.ext.revHash = project.rootProject.ext.revHash | ||
| project.ext.javaVersions = project.rootProject.ext.javaVersions | ||
| project.ext.javaVersions = BuildParams.javaVersions | ||
| project.ext.isRuntimeJavaHomeSet = project.rootProject.ext.isRuntimeJavaHomeSet |
There was a problem hiding this comment.
isRuntimeJavaHomeSet has been replaced by a property on BuildParams a well
| project.ext.gitHead = project.rootProject.ext.gitHead | ||
| project.ext.revHash = project.rootProject.ext.revHash | ||
| project.ext.javaVersions = project.rootProject.ext.javaVersions | ||
| project.ext.javaVersions = BuildParams.javaVersions |
There was a problem hiding this comment.
Is setting these extra properties strictly necessary? We should be better off replacing their usages with direct usage of BuildParams.
There was a problem hiding this comment.
Looks like we are overriding these up further in this plugin as well. If hadoop is using it's own logic to set runtime java home we should use BuildParams.init() to set those values instead of continuing the practice of using extra properties.
| /** Return the java home used by this node. */ | ||
| String getJavaHome() { | ||
| return javaVersion == null ? project.runtimeJavaHome : project.javaVersions.find { it.version == javaVersion }.javaHome.absolutePath | ||
| return javaVersion == null ? project.runtimeJavaHome : BuildParams.javaVersions.find { it.version == javaVersion }.javaHome.absolutePath |
There was a problem hiding this comment.
Assuming we refactor out usages of extra properties completely, we can replace project.runtimeJavaHome with the BuildParams variant.
|
@mark-vieira Yes, this is enough to make the build successful. I would be happy if you could pick up the follow-ups. |
This commit adapts the elasticsearch-hadoop build to an upstream change to make some build globals type safe. In particular, we adapt the usage of Java versions.
5 participants
This commit adapts the elasticsearch-hadoop build to an upstream change to make some build globals type safe. In particular, we adapt the usage of Java versions.
Relates elastic/elasticsearch#48778