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

Build plugin with platform specific JDK #69

Merged
merged 5 commits into from
Apr 26, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ dependencies {
}

// If the license plugin is applied, disable license checks for the autogenerated source sets
plugins.withId('com.github.hierynomus.license') {
licenseHive.enabled = false
licensePresto.enabled = false
licenseSpark.enabled = false
tasks.whenTaskAdded { task ->
if (task.name.startsWith("licensePresto")
|| task.name.startsWith("licenseHive")
|| task.name.startsWith("licenseSpark")) {
task.enabled = false
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this change is no longer needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed


// TODO: Add a debugPlatform flag to allow debugging specific test methods in IntelliJ
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.io.InputStream;
import java.util.List;
import java.util.Properties;
import org.gradle.jvm.toolchain.JavaLanguageVersion;

import static com.linkedin.transport.plugin.ConfigurationType.*;

Expand Down Expand Up @@ -61,6 +62,7 @@ private static Properties loadDefaultVersions() {
"presto",
Language.JAVA,
PrestoWrapperGenerator.class,
JavaLanguageVersion.of(11),
ImmutableList.of(
getDependencyConfiguration(IMPLEMENTATION, "com.linkedin.transport:transportable-udfs-presto",
"transport"),
Expand All @@ -78,6 +80,7 @@ private static Properties loadDefaultVersions() {
"hive",
Language.JAVA,
HiveWrapperGenerator.class,
JavaLanguageVersion.of(8),
ImmutableList.of(
getDependencyConfiguration(IMPLEMENTATION, "com.linkedin.transport:transportable-udfs-hive", "transport"),
getDependencyConfiguration(COMPILE_ONLY, "org.apache.hive:hive-exec", "hive")
Expand All @@ -91,6 +94,7 @@ private static Properties loadDefaultVersions() {
"spark",
Language.SCALA,
SparkWrapperGenerator.class,
JavaLanguageVersion.of(8),
ImmutableList.of(
getDependencyConfiguration(IMPLEMENTATION, "com.linkedin.transport:transportable-udfs-spark",
"transport"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.linkedin.transport.codegen.WrapperGenerator;
import com.linkedin.transport.plugin.packaging.Packaging;
import java.util.List;
import org.gradle.jvm.toolchain.JavaLanguageVersion;


/**
Expand All @@ -21,12 +22,14 @@ public class Platform {
private final List<DependencyConfiguration> _defaultWrapperDependencyConfigurations;
private final List<DependencyConfiguration> _defaultTestDependencyConfigurations;
private final List<Packaging> _packaging;
private final JavaLanguageVersion _javaLanguageVersion;

public Platform(String name, Language language, Class<? extends WrapperGenerator> wrapperGeneratorClass,
List<DependencyConfiguration> defaultWrapperDependencyConfigurations,
JavaLanguageVersion javaLanguageVersion, List<DependencyConfiguration> defaultWrapperDependencyConfigurations,
List<DependencyConfiguration> defaultTestDependencyConfigurations, List<Packaging> packaging) {
_name = name;
_language = language;
_javaLanguageVersion = javaLanguageVersion;
_wrapperGeneratorClass = wrapperGeneratorClass;
_defaultWrapperDependencyConfigurations = defaultWrapperDependencyConfigurations;
_defaultTestDependencyConfigurations = defaultTestDependencyConfigurations;
Expand Down Expand Up @@ -56,4 +59,8 @@ public List<DependencyConfiguration> getDefaultTestDependencyConfigurations() {
public List<Packaging> getPackaging() {
return _packaging;
}

public JavaLanguageVersion getJavaLanguageVersion() {
return _javaLanguageVersion;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
import org.gradle.api.plugins.scala.ScalaPlugin;
import org.gradle.api.tasks.SourceSet;
import org.gradle.api.tasks.TaskProvider;
import org.gradle.api.tasks.compile.JavaCompile;
import org.gradle.api.tasks.testing.Test;
import org.gradle.jvm.toolchain.JavaToolchainService;
import org.gradle.language.base.plugins.LifecycleBasePlugin;
import org.gradle.testing.jacoco.plugins.JacocoPlugin;
import org.gradle.testing.jacoco.plugins.JacocoTaskExtension;
Expand Down Expand Up @@ -64,6 +66,7 @@ public void apply(Project project) {
Defaults.DEFAULT_PLATFORMS.forEach(
platform -> configurePlatform(project, platform, mainSourceSet, testSourceSet, extension.outputDirFile));
});

// Disable Jacoco for platform test tasks as it is known to cause issues with Presto and Hive tests
project.getPlugins().withType(JacocoPlugin.class, (jacocoPlugin) -> {
Defaults.DEFAULT_PLATFORMS.forEach(platform -> {
Expand Down Expand Up @@ -119,6 +122,7 @@ private SourceSet configureSourceSet(Project project, Platform platform, SourceS
Path wrapperResourceOutputDir = platformBaseDir.resolve("resources");

return javaConvention.getSourceSets().create(platform.getName(), sourceSet -> {

/*
Creates a SourceSet and set the source directories for a given platform. E.g. For the Presto platform,

Expand Down Expand Up @@ -192,9 +196,33 @@ private TaskProvider<GenerateWrappersTask> configureGenerateWrappersTask(Project
task.dependsOn(project.getTasks().named(inputSourceSet.getClassesTaskName()));
});

project.getTasks()
.named(outputSourceSet.getCompileTaskName(platform.getLanguage().toString()))
.configure(task -> task.dependsOn(generateWrappersTask));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we leave this as is and then replace the switch block below with if (platform.getLanguage() = JAVA)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated


switch (platform.getLanguage()) {
case JAVA:
project.getTasks()
.named(outputSourceSet.getCompileTaskName(platform.getLanguage().toString()), JavaCompile.class)
.configure(task -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.named(outputSourceSet.getCompileTaskName(platform.getLanguage().toString()), JavaCompile.class)
.configure(task -> {
.named(outputSourceSet.getCompileTaskName(platform.getLanguage().toString()), JavaCompile.class, task -> {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

task.dependsOn(generateWrappersTask);
// configure compile task to run with platform specific jdk
JavaToolchainService javaToolchains = project.getExtensions().getByType(JavaToolchainService.class);
task.getJavaCompiler().set(javaToolchains.compilerFor(toolChainSpec -> {
toolChainSpec.getLanguageVersion().set(platform.getJavaLanguageVersion());
}));
});
break;
case SCALA:
default:
project.getTasks()
.named(outputSourceSet.getCompileTaskName(platform.getLanguage().toString()))
.configure(task -> {
task.dependsOn(generateWrappersTask);
// todo: configure task to run with platform specific jdk (currently picks from local env)
// Toolchain support is only available in the Java plugins and for the tasks they define.
// Support for the Scala plugin is not released yet.
// Ref: https://docs.gradle.org/7.0/userguide/toolchains.html#sec:consuming
});
break;
}

return generateWrappersTask;
}
Expand Down Expand Up @@ -257,6 +285,12 @@ task prestoTest(type: Test, dependsOn: test) {
task.setClasspath(testClasspath);
task.useTestNG();
task.mustRunAfter(project.getTasks().named("test"));

// configure test task to run with platform specific jdk
JavaToolchainService javaToolchains = project.getExtensions().getByType(JavaToolchainService.class);
task.getJavaLauncher().set(javaToolchains.launcherFor(toolChainSpec -> {
toolChainSpec.getLanguageVersion().set(platform.getJavaLanguageVersion());
}));
});
}

Expand Down
23 changes: 2 additions & 21 deletions transportable-udfs-presto/build.gradle
Original file line number Diff line number Diff line change
@@ -1,26 +1,7 @@
apply plugin: 'java'

buildscript {
repositories {
mavenCentral()
}
dependencies {
classpath group:'io.prestosql', name: 'presto-main', version: project.ext.'presto-version'
}
}

import com.google.common.base.StandardSystemProperty
import io.prestosql.server.JavaVersion
task verifyPrestoJvmRequirements(type:Exec) {
String javaVersion = StandardSystemProperty.JAVA_VERSION.value()
if (javaVersion == null) {
throw new GradleException("Java version not defined")
}
JavaVersion version = JavaVersion.parse(javaVersion)
if (!(version.getMajor() == 8 && version.getUpdate().isPresent() && version.getUpdate().getAsInt() >= 151)
|| (version.getMajor() >= 9)) {
throw new GradleException(String.format("Presto requires Java 8u151+ (found %s)", version))
}
java {
toolchain.languageVersion.set(JavaLanguageVersion.of(11))
}

dependencies {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
apply plugin: 'java'

java {
toolchain.languageVersion.set(JavaLanguageVersion.of(11))
}

dependencies {
compile project(":transportable-udfs-api")
compile project(":transportable-udfs-test:transportable-udfs-test-api")
Expand Down