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.

Sign up for GitHub

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

Import sources for javadoc through a dependency #1461

Merged
merged 3 commits into from
Apr 10, 2020
Merged

Import sources for javadoc through a dependency #1461

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
24b4410
Import sources for javadoc through a dependency
jbaiera Apr 9, 2020
94c0133
Don't need a javadoc configuration. The dependencies for the javadoc …
jbaiera Apr 9, 2020
1b0ce05
PR Feedback
jbaiera Apr 10, 2020
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
47 changes: 47 additions & 0 deletions buildSrc/src/main/groovy/org/elasticsearch/hadoop/gradle/BuildPlugin.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@ import org.gradle.api.artifacts.ProjectDependency
import org.gradle.api.artifacts.ResolutionStrategy
import org.gradle.api.artifacts.maven.MavenPom
import org.gradle.api.artifacts.maven.MavenResolver
import org.gradle.api.attributes.LibraryElements
import org.gradle.api.attributes.Usage
import org.gradle.api.file.CopySpec
import org.gradle.api.file.FileCollection
import org.gradle.api.java.archives.Manifest
import org.gradle.api.plugins.JavaPlugin
import org.gradle.api.plugins.MavenPlugin
import org.gradle.api.plugins.MavenPluginConvention
import org.gradle.api.tasks.SourceSet
import org.gradle.api.tasks.SourceSetContainer
import org.gradle.api.tasks.TaskProvider
import org.gradle.api.tasks.Upload
Expand Down Expand Up @@ -84,6 +88,32 @@ class BuildPlugin implements Plugin<Project> {
}

private static void configureConfigurations(Project project) {
if (project != project.rootProject) {
// Set up avenues for sharing source files between projects in order to create embedded Javadocs
// Import source configuration
Configuration sources = project.configurations.create("sources")
jbaiera marked this conversation as resolved.
Show resolved Hide resolved
sources.canBeConsumed = false
sources.canBeResolved = true
Copy link
Contributor

Choose a reason for hiding this comment

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

this is be false too ? the sources shouldn't be resolved since they should only be declared as a single project reference ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, gradle does not seem to like this. In order for Gradle to convert the project dependency into actual files it needs to resolve the configuration. It needs to do this for the sources jar, and any time a file collection that contains the configuration is mutated.

sources.attributes {
// Changing USAGE is required when working with Scala projects, otherwise the source dirs get pulled
// into incremental compilation analysis.
attribute(Usage.USAGE_ATTRIBUTE, project.objects.named(Usage, 'java-source'))
attribute(LibraryElements.LIBRARY_ELEMENTS_ATTRIBUTE, project.objects.named(LibraryElements, 'sources'))
}

// Export source configuration
Configuration sourceElements = project.configurations.create("sourceElements")
sourceElements.canBeConsumed = true
sourceElements.canBeResolved = false
sourceElements.extendsFrom(sources)
sourceElements.attributes {
// Changing USAGE is required when working with Scala projects, otherwise the source dirs get pulled
// into incremental compilation analysis.
attribute(Usage.USAGE_ATTRIBUTE, project.objects.named(Usage, 'java-source'))
attribute(LibraryElements.LIBRARY_ELEMENTS_ATTRIBUTE, project.objects.named(LibraryElements, 'sources'))
}
}

if (project.path.startsWith(":qa")) {
return
}
Expand Down Expand Up @@ -200,6 +230,15 @@ class BuildPlugin implements Plugin<Project> {
project.sourceCompatibility = '1.8'
project.targetCompatibility = '1.8'

// TODO: Remove all root project distribution logic. It should exist in a separate dist project.
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 is a problem that I'm working on addressing in another PR. The root project is all sorts of messed up because it depends (without using dependencies in a lot of cases 😱) on all the subprojects to build an uber jar. This uberjar/dist zip should be generated in a subproject instead of the root project, using dependencies, configurations, and all other appropriate build script hygiene to resolve the cross project links.

if (project != project.rootProject) {
SourceSet mainSourceSet = project.sourceSets.main
FileCollection javaSourceDirs = mainSourceSet.java.sourceDirectories
javaSourceDirs.each { File srcDir ->
Copy link
Contributor

Choose a reason for hiding this comment

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

is this loop correct ? I would have expected to see something like:

project.getArtifacts().add('sourceElements', mainSourceSet.java.sourceDirectories)

(no loop)

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe that will work since sourceDirectories is a FileCollection. The ArtifactHandler.add() doesn't accept collections. You need to add the directories individually.

That said, because of this, we are open to configuration ordering issues here. So any additional source directories need to be added before this code runs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I wasn't able to find anything on FileCollections that allows you to listen for changes like other domain object collections. I think anything outside of the default java directory needs to be added to the source set explicitly, and thus should be getting added to the sourceElements configuration explicitly as well.

project.getArtifacts().add('sourceElements', srcDir)
}
}

JavaCompile compileJava = project.tasks.getByName('compileJava') as JavaCompile
compileJava.getOptions().setCompilerArgs(['-Xlint:unchecked', '-Xlint:options'])

Expand Down Expand Up @@ -235,6 +274,10 @@ class BuildPlugin implements Plugin<Project> {
sourcesJar.dependsOn(project.tasks.classes)
sourcesJar.classifier = 'sources'
sourcesJar.from(project.sourceSets.main.allSource)
// TODO: Remove when root project does not handle distribution
if (project != project.rootProject) {
sourcesJar.from(project.configurations.sources)
}

// Configure javadoc
Javadoc javadoc = project.tasks.getByName('javadoc') as Javadoc
Expand All @@ -246,6 +289,10 @@ class BuildPlugin implements Plugin<Project> {
"org/elasticsearch/hadoop/util/**",
"org/apache/hadoop/hive/**"
]
// TODO: Remove when root project does not handle distribution
if (project != project.rootProject) {
javadoc.source += project.configurations.sources
}
// Set javadoc executable to runtime Java (1.8)
javadoc.executable = new File(project.ext.runtimeJavaHome, 'bin/javadoc')

Expand Down
11 changes: 2 additions & 9 deletions hive/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ dependencies {
itestImplementation("org.apache.hive:hive-jdbc:$hiveVersion") {
exclude module: "log4j-slf4j-impl"
}

sources(project(":elasticsearch-hadoop-mr"))
}

jar {
Expand All @@ -42,12 +44,3 @@ jar {
include "META-INF/services/*"
}
}

javadoc {
source += project(":elasticsearch-hadoop-mr").sourceSets.main.allJava
classpath += files(project(":elasticsearch-hadoop-mr").sourceSets.main.compileClasspath)
}

sourcesJar {
from project(":elasticsearch-hadoop-mr").sourceSets.main.allJava.srcDirs
}
11 changes: 2 additions & 9 deletions pig/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ dependencies {

itestImplementation(project(":test:shared"))
itestImplementation("dk.brics.automaton:automaton:1.11-8")

sources(project(":elasticsearch-hadoop-mr"))
}

jar {
Expand All @@ -40,12 +42,3 @@ jar {
include "META-INF/services/*"
}
}

javadoc {
source += project(":elasticsearch-hadoop-mr").sourceSets.main.allJava
classpath += files(project(":elasticsearch-hadoop-mr").sourceSets.main.compileClasspath)
}

sourcesJar {
from project(":elasticsearch-hadoop-mr").sourceSets.main.allJava.srcDirs
}
17 changes: 10 additions & 7 deletions spark/sql-13/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,15 @@ dependencies {
testImplementation("org.apache.spark:spark-sql_${project.ext.scalaMajorVersion}:$sparkVersion") {
exclude group: 'org.apache.hadoop'
}

sources(project(":elasticsearch-hadoop-mr"))
}

// Export generated Java code from the genjavadoc compiler plugin
artifacts {
sourceElements(project.file("$buildDir/generated/java")) {
builtBy compileScala
}
}

jar {
Expand All @@ -173,13 +182,7 @@ jar {

javadoc {
dependsOn compileScala
source += project(":elasticsearch-hadoop-mr").sourceSets.main.allJava
source += "$buildDir/generated/java"
classpath += files(project(":elasticsearch-hadoop-mr").sourceSets.main.compileClasspath)
}

sourcesJar {
from project(":elasticsearch-hadoop-mr").sourceSets.main.allJava.srcDirs
}

scaladoc {
Expand All @@ -193,4 +196,4 @@ tasks.withType(ScalaCompile) {
"-P:genjavadoc:out=$buildDir/generated/java".toString()
]
}
}
}
17 changes: 10 additions & 7 deletions spark/sql-20/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,15 @@ dependencies {
itestImplementation("org.apache.spark:spark-streaming_${project.ext.scalaMajorVersion}:$sparkVersion") {
exclude group: 'org.apache.hadoop'
}

sources(project(":elasticsearch-hadoop-mr"))
}

// Export generated Java code from the genjavadoc compiler plugin
artifacts {
sourceElements(project.file("$buildDir/generated/java")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

does adding the sourceElements here conflict with plugin addition ? ( I don't know if it is additive or a replacement)

Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't adding a configuration. It's adding an artifact to the existing sourceElements configuration.

Copy link
Member Author

@jbaiera jbaiera Apr 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure what you mean by this question. Do you mean the scala compiler plugin or the build plugin?

Edit: Github did not refresh before Mark answered :P

builtBy compileScala
}
}

jar {
Expand All @@ -168,13 +177,7 @@ jar {

javadoc {
dependsOn compileScala
source += project(":elasticsearch-hadoop-mr").sourceSets.main.allJava
source += "$buildDir/generated/java"
classpath += files(project(":elasticsearch-hadoop-mr").sourceSets.main.compileClasspath)
}

sourcesJar {
from project(":elasticsearch-hadoop-mr").sourceSets.main.allJava.srcDirs
}

scaladoc {
Expand All @@ -188,4 +191,4 @@ tasks.withType(ScalaCompile) {
"-P:genjavadoc:out=$buildDir/generated/java".toString()
]
}
}
}
11 changes: 2 additions & 9 deletions storm/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ dependencies {
itestImplementation(project(":test:shared"))
itestImplementation("com.google.guava:guava:16.0.1")
itestImplementation("com.twitter:carbonite:1.4.0")

sources(project(":elasticsearch-hadoop-mr"))
}

jar {
Expand All @@ -37,13 +39,4 @@ jar {
}
}

javadoc {
source += project(":elasticsearch-hadoop-mr").sourceSets.main.allJava
classpath += files(project(":elasticsearch-hadoop-mr").sourceSets.main.compileClasspath)
}

sourcesJar {
from project(":elasticsearch-hadoop-mr").sourceSets.main.allJava.srcDirs
}

tasks.getByName('integrationTest').enabled = false