Move to unified gradle build #1835

loosebazooka · 2019-07-10T21:10:27Z

Fixes #1790.

prerequisites to merging:

Try Google Application Default Credentials for GCR (gcr.io) auth #1902 needs to be properly merged into here (dependencies)
eclipse loading instructions needs to be updated
test if Javadoc errors (correctly) fail the build
we will do this after the 1.5.0 release

post-merge

ensure you submit cr/263181540 after so the maven release works.

about:

unified build ./gradlew build or ./gradlew <anytask> will run on all projects
./gradlew <subproject>:build will limit the build to a subproject and it's dependencies
IDEs will require certain pre-steps for some of their tests that test against a plugin build (might be easier to run those from the commandline)
- gradle: ./gradlew jib-gradle-plugin:pluginUnderTestMetadata
- maven: ./gradlew jib-maven-plugin:install
build.sh is gone, all builds should use ./gradlew directly
any common dependency version should be defined in the parent build.gradle and all dependencies are implementation and are exported to runtime (not compile) scope transitively.
The releases should work, but we should probably do a little testing after this merges to truly ensure that.

chanseokoh

Whatever it's doing, I like moving away from the current setup where we copy source directories of other projects. I gave a try in Eclipse.

In Eclipse, looks like I need to import 5 Gradle projects, which doesn't look wrong: jib, jib-core, jib-gradle-plugin, jib-maven-plugin, jib-plugins-common.

Now, I had to additionally configure extra build path through the IDE mechanism. jib-gradle-plugin and jib-maven-plugin could not see classes in jib-core, so I had to add jib-core as an additional dependent project for them. Interestingly, I did not have to add jib-plugins-common as an explicit project dependency. I wonder if this is also the case in IntelliJ.

Now, unrelated, I was always not really a fan of making src/main/resources and src/[integration-]test/resources as Java source files. Conceptually, this doesn't make sense, because resources there are not project source. (I know they were added as "source" for the sake of convenience in running tests.) Consequently, Eclipse sees the test resources whose extension is .java as the genuine project source to compile and include, hence showing compilation failure. I have been manually excluding some sub-directories (e.g., ...test/resources/gradle/projects) through the IDE build path to let it not think they are project source.

build.gradle

loosebazooka · 2019-07-26T20:31:29Z

jib-core/kokoro/release_build.sh

-cd github/jib/jib-core
-./gradlew prepareRelease
+cd github/jib
+./gradlew :jib-core:prepareRelease


I'm not actually convinced this matches up with our release scripts, I'll have to see.

build.gradle

loosebazooka · 2019-08-05T17:08:34Z

some of these changes can probably be broken out into unrelated fixes, I'll do that to make this isolated to the build stuff.

loosebazooka · 2019-08-06T19:33:49Z

jib-gradle-plugin/build.gradle

@@ -185,5 +67,5 @@ pluginBundle {
    }
  }
 }
-tasks.publishPlugins.dependsOn integrationTest
+//tasks.publishPlugins.dependsOn integrationTest


I need to uncomment this

TadCordle · 2019-08-07T19:31:44Z

We'll probably need to update CONTRIBUTING.md as well.

loosebazooka · 2019-08-13T19:23:37Z

This should be ready now to look at.

CONTRIBUTING.md

jib-core/build.gradle

TadCordle

Tried this on my laptop and it feels a lot nicer/simpler. LGTM once eclipse is confirmed to work.

CONTRIBUTING.md

chanseokoh · 2019-08-20T21:07:21Z

Yeah, definitely this is a lot nicer. No more copying source directories, and I really like it.

For the record, the outcome of the most recent try on Eclipse:

Now, I had to additionally configure extra build path through the IDE mechanism. jib-gradle-plugin and jib-maven-plugin could not see classes in jib-core, so I had to add jib-core as an additional dependent project for them. Interestingly, I did not have to add jib-plugins-common as an explicit project dependency. I wonder if this is also the case in IntelliJ.

This is still true, unfortunately. Not a big deal for me, as I only have to do this once and it's done.

Okay, I manually ran ./gradlew pluginUnderTestMetadata at the root (the jib/ directory), which created jib-gradle-plugin/build/pluginUnderTestMetadata/plugin-under-test-metadata.properties.

This was probably due to my lack of understanding of this thing. The old setup had the same problem; nothing new. I learned running test on the command line just generates this file. Once generated, I'm good.

Then I manually added jib-gradle-plugin/build/pluginUnderTestMetadata as an extra classpath folder for my test run configuration in Eclipse, and it works. Something unnecessary in the old setup.

No longer the case. No need to add an extra classpath. It works now. Or maybe I just misunderstood this part back then.

loosebazooka · 2019-08-21T19:12:05Z

@chanseokoh about the javadoc, are you sure the bad @param is on a public method? The gradle javadoc (maybe different than maven) only generates javadoc for public apis

chanseokoh · 2019-08-21T20:01:11Z

Yeah, it does fail. Didn't realized it only works for public things.

TadCordle · 2019-08-23T17:03:55Z

Is there more work to be done on this or is this good to go?

loosebazooka · 2019-08-23T17:47:41Z

I gotta rebase on the 1.5.1 release, but other than that, yeah

- also upgrades gradle to 5.2.1 - all builds should happen from root :<project>:<task>, build.sh removed - tests that run the plugin may not work in IDEs unless correct setup task run first - special eclipse helpers to clear up classpath - adds protections for transitive dependency overrides

loosebazooka · 2019-08-27T17:27:50Z

build.gradle

+
+  // Use this to ensure we correctly override transitive dependencies
+  // TODO: There might be a plugin that does this
+  task ensureTransitiveDependencyOverrides {


This is some new code to protect against us accidentally import httpclient of a different version transitively. It does the job, but I worry about the maintainability, as there may not be a way to know if it suddenly stops working. I guess currently it works, worst case it may not work.

loosebazooka requested review from briandealwis, TadCordle and chanseokoh July 10, 2019 21:10

googlebot added the cla: yes label Jul 10, 2019

loosebazooka force-pushed the reworkBuild branch from 0d19869 to ee92735 Compare July 10, 2019 21:11

chanseokoh reviewed Jul 11, 2019

View reviewed changes

build.gradle Show resolved Hide resolved

This comment has been minimized.

Sign in to view

loosebazooka added the PR: Not Ready label Jul 19, 2019

loosebazooka changed the title ~~Move to unified gradle build (WIP~~ Move to unified gradle build (WIP) Jul 19, 2019

loosebazooka force-pushed the reworkBuild branch from ee92735 to 0d2a481 Compare July 25, 2019 20:06

This comment has been minimized.

Sign in to view

loosebazooka force-pushed the reworkBuild branch 3 times, most recently from 22e38b8 to 9a0672f Compare July 26, 2019 20:30

loosebazooka commented Jul 26, 2019

View reviewed changes

chanseokoh reviewed Aug 1, 2019

View reviewed changes

build.gradle Outdated Show resolved Hide resolved

loosebazooka force-pushed the reworkBuild branch from 9a0672f to dd1c289 Compare August 5, 2019 19:16

This was referenced Aug 5, 2019

Minor tests cleanup #1890

Merged

Use one multi-module gradle build #1791

Closed

This comment has been minimized.

Sign in to view

loosebazooka force-pushed the reworkBuild branch 2 times, most recently from 7c90542 to 1c6636f Compare August 6, 2019 19:18

loosebazooka commented Aug 6, 2019

View reviewed changes

loosebazooka force-pushed the reworkBuild branch from 1c6636f to ce56aa0 Compare August 6, 2019 19:55

loosebazooka force-pushed the reworkBuild branch 4 times, most recently from 1f44fee to d9f495a Compare August 13, 2019 19:16

loosebazooka changed the title ~~Move to unified gradle build (WIP)~~ Move to unified gradle build Aug 13, 2019

loosebazooka removed the PR: Not Ready label Aug 13, 2019

loosebazooka commented Aug 13, 2019

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

loosebazooka removed the cla: yes label Aug 13, 2019

googlebot added the cla: yes label Aug 13, 2019

loosebazooka added this to the v1.6.0 milestone Aug 13, 2019

chanseokoh reviewed Aug 13, 2019

View reviewed changes

jib-core/build.gradle Show resolved Hide resolved

loosebazooka force-pushed the reworkBuild branch 2 times, most recently from 448b483 to ec61dab Compare August 19, 2019 21:03

TadCordle reviewed Aug 20, 2019

View reviewed changes

CONTRIBUTING.md Show resolved Hide resolved

CONTRIBUTING.md Outdated Show resolved Hide resolved

CONTRIBUTING.md Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

loosebazooka force-pushed the reworkBuild branch from 3b2ae76 to 58699ef Compare August 27, 2019 17:25

loosebazooka commented Aug 27, 2019

View reviewed changes

TadCordle approved these changes Aug 27, 2019

View reviewed changes

loosebazooka merged commit af34d9f into master Aug 27, 2019

loosebazooka deleted the reworkBuild branch August 27, 2019 18:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move to unified gradle build #1835

Move to unified gradle build #1835

loosebazooka commented Jul 10, 2019 •

edited

Loading

chanseokoh left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

loosebazooka Jul 26, 2019

loosebazooka commented Aug 5, 2019

This comment has been minimized.

loosebazooka Aug 6, 2019 •

edited

Loading

TadCordle commented Aug 7, 2019

loosebazooka commented Aug 13, 2019

TadCordle left a comment

chanseokoh commented Aug 20, 2019

This comment has been minimized.

This comment has been minimized.

loosebazooka commented Aug 21, 2019

chanseokoh commented Aug 21, 2019

TadCordle commented Aug 23, 2019

loosebazooka commented Aug 23, 2019

loosebazooka Aug 27, 2019

Move to unified gradle build #1835

Move to unified gradle build #1835

Conversation

loosebazooka commented Jul 10, 2019 • edited Loading

chanseokoh left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

loosebazooka Jul 26, 2019

Choose a reason for hiding this comment

loosebazooka commented Aug 5, 2019

This comment has been minimized.

loosebazooka Aug 6, 2019 • edited Loading

Choose a reason for hiding this comment

TadCordle commented Aug 7, 2019

loosebazooka commented Aug 13, 2019

TadCordle left a comment

Choose a reason for hiding this comment

chanseokoh commented Aug 20, 2019

This comment has been minimized.

This comment has been minimized.

loosebazooka commented Aug 21, 2019

chanseokoh commented Aug 21, 2019

TadCordle commented Aug 23, 2019

loosebazooka commented Aug 23, 2019

loosebazooka Aug 27, 2019

Choose a reason for hiding this comment

loosebazooka commented Jul 10, 2019 •

edited

Loading

loosebazooka Aug 6, 2019 •

edited

Loading