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

HLREST: Bundle the x-pack protocol project #31904

Merged
merged 7 commits into from
Jul 10, 2018
Merged

HLREST: Bundle the x-pack protocol project #31904

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
376ef5a
HLREST: Bundle the x-pack protocol project
nik9000 Jul 9, 2018
ed5f1ea
Fix indentation
nik9000 Jul 9, 2018
3260c3d
Switch to shadow!
nik9000 Jul 10, 2018
578bf12
Publishing
nik9000 Jul 10, 2018
4786607
no more println
nik9000 Jul 10, 2018
1af3dbb
Merge branch 'master' into bundle_x_pack_protocol
nik9000 Jul 10, 2018
33fef73
Use the shadow configuration
nik9000 Jul 10, 2018
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
49 changes: 48 additions & 1 deletion client/rest-high-level/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,32 @@ publishing {
}
}

// We bundle the x-pack:protocol project into the jar.
configurations {
bundled
Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, another spot where indentation doesn't match the rest of the project. I'll fix.

}
sourceSets {
main {
compileClasspath += configurations.bundled
}
test {
compileClasspath += configurations.bundled
}
}

dependencies {
compile "org.elasticsearch:elasticsearch:${version}"
compile "org.elasticsearch.client:elasticsearch-rest-client:${version}"
compile "org.elasticsearch.plugin:parent-join-client:${version}"
compile "org.elasticsearch.plugin:aggs-matrix-stats-client:${version}"
compile "org.elasticsearch.plugin:rank-eval-client:${version}"
compile "org.elasticsearch.plugin:lang-mustache-client:${version}"
compile project(':x-pack:protocol') // TODO bundle into the jar
// Eclipse can't comprehend the bundled configuration...
if (isEclipse) {
compile project(':x-pack:protocol')
} else {
bundled project(':x-pack:protocol')
}

testCompile "org.elasticsearch.client:test:${version}"
testCompile "org.elasticsearch.test:framework:${version}"
Expand All @@ -64,3 +82,32 @@ forbiddenApisMain {
signaturesURLs += [PrecommitTasks.getResource('/forbidden/http-signatures.txt')]
signaturesURLs += [file('src/main/resources/forbidden/rest-high-level-signatures.txt').toURI().toURL()]
}

javadoc {
// Bundle javadoc from all bundled projects into this project's javadoc
configurations.bundled.dependencies.all { Dependency dep ->
Project p = dependencyToProject(dep)
if (p != null) {
evaluationDependsOn(p.path)
Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't find a way to make this sufficiently lazy to avoid forcing evaluation of the other project. I believe I could use afterProjectsEvaluated but this feels much more precise.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be something in BuildPlugin? Seems like we would want this for any cross project dependencies?

Copy link
Member Author

Choose a reason for hiding this comment

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

We only want this if we want to combine that javadocs from a bunch of different projects into one place. It feels like something we might only want in the high level rest client. Maybe into the plugin API when we make a jar for it. If we make a jar for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

So after thinking about this some more, maybe! I think we might want this if we detect that the shadow plugin is around.

source += p.sourceSets.main.allJava
}
}
}
jar {
from({configurations.bundled.collect { it.isDirectory() ? it : zipTree(it) }}) {
exclude 'META-INF/*'
Copy link
Member

Choose a reason for hiding this comment

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

Won't this break any SPI used by the "bundled" jar?

Copy link
Member

Choose a reason for hiding this comment

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

It seems like this is trying to manually do the work of the shadow plugin?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes on both counts. I started with the shadow plugin but it fought me and it feels so "big". I'll see about using it though. It absolutely handles SPI properly and this doesn't.

}
}
/*
* Use the jar for testing so we have tests of the bundled jar.
*/
test {
classpath -= compileJava.outputs.files
classpath += jar.outputs.files
dependsOn jar
}
integTestRunner {
classpath -= compileJava.outputs.files
Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, indentation doesn't match. I'll fix.

classpath += jar.outputs.files
dependsOn jar
}