Lower build requirement from Java 14+ to Java 11+ #636

rmuir · 2021-04-29T06:49:57Z

Currently java14+ is required to use the gradle build. Requiring only java11 makes the source packages easier to consume, because the most recent LTS java release will work fine.

Avoid use of -Werror -Xlint:all, which may change significantly across java releases (new warnings could be added). Instead, just list the warnings individually.

Workaround JDK 11 compiler bug (JDK-8209058) that only impacts test fixture code in the build itself.

Signed-off-by: Robert Muir rmuir@apache.org

opensearch-ci-bot · 2021-04-29T06:53:45Z

✅ DCO Check Passed 89478ea

opensearch-ci-bot · 2021-04-29T06:53:45Z

✅ Gradle Wrapper Validation success 89478ea

opensearch-ci-bot · 2021-04-29T07:01:04Z

❌ Gradle Precommit failure 89478ea
Log 340

opensearch-ci-bot · 2021-04-29T07:31:48Z

✅ Gradle Wrapper Validation success bc72cd3

opensearch-ci-bot · 2021-04-29T07:31:48Z

✅ DCO Check Passed bc72cd3

opensearch-ci-bot · 2021-04-29T07:33:42Z

✅ Gradle Precommit success bc72cd3

dblock · 2021-04-29T14:00:55Z

I like this, thanks! We also need to lower the Java version in CI, I am not sure where that is.

rmuir · 2021-04-29T14:07:21Z

I like this, thanks! We also need to lower the Java version in CI, I am not sure where that is.

@dblock Just to be clear this doesn't change how the test suite runs or what is packaged. E.G. if you use java 11 to build, it still downloads java 15 into your gradle caches and uses that java 15 jvm to run unit tests, and it still creates packages in distribution with java 15 images.

So it really only impacts gradle and javac, but I think it makes life easier for developers, packagers, etc. But yes, agree it would be nice for CI to use the minimum required build version to ensure that you can build with java 11.

dblock · 2021-04-29T14:14:41Z

I like this, thanks! We also need to lower the Java version in CI, I am not sure where that is.

@dblock Just to be clear this doesn't change how the test suite runs or what is packaged. E.G. if you use java 11 to build, it still downloads java 15 into your gradle caches and uses that java 15 jvm to run unit tests, and it still creates packages in distribution with java 15 images.

So it really only impacts gradle and javac, but I think it makes life easier for developers, packagers, etc. But yes, agree it would be nice for CI to use the minimum required build version to ensure that you can build with java 11.

I think I'm saying the same thing. If we don't include building with Java 11 in CI, we will keep breaking this "minimum" version. I just don't know where that change needs to be.

rmuir · 2021-04-29T14:34:37Z

I think I'm saying the same thing. If we don't include building with Java 11 in CI, we will keep breaking this "minimum" version. I just don't know where that change needs to be.

Yeah, I'm not sure either. Maybe it is just whatever happens to be in PATH/JAVA_HOME on the ci machines? Or maybe .ci/java-versions.properties is used for configuring this? Currently this file contains OPENSEARCH_BUILD_JAVA=openjdk14. If the file is not actually used by CI, let's delete it :)

dblock · 2021-04-29T14:39:29Z

I think I'm saying the same thing. If we don't include building with Java 11 in CI, we will keep breaking this "minimum" version. I just don't know where that change needs to be.

Yeah, I'm not sure either. Maybe it is just whatever happens to be in PATH/JAVA_HOME on the ci machines? Or maybe .ci/java-versions.properties is used for configuring this? Currently this file contains OPENSEARCH_BUILD_JAVA=openjdk14. If the file is not actually used by CI, let's delete it :)

@bbarani @peterzhuamazon do you guys know where to change this as part of this PR?

ssgao · 2021-04-29T19:15:52Z

DEVELOPER_GUIDE.md

@@ -50,7 +50,7 @@ sdk install java 14.0.2-open
 sdk use java 14.0.2-open


should these be updated as well?

I'm not familiar with this sdk tool, so I wasn't sure what to do here. By only requiring the LTS java 11, the user could probably just avoid this tool and use their normal system package manager?

It's here as just an example, swap 14 -> 11 and we're good to go?

I'd prefer to remove it. I have no idea how to test my instructions. Sorry if it comes across as rude, but I'm seriously not gonna run any curl | bash stuff.

deleting the block makes more sense to me
there are other ways to manage different versions of java locally

FWIW I added this because I was trying to make it super simple for someone to start contributing and the sdk tool is super popular in my world, along with the equivalents of nvm for node or rvm for ruby. I am always puzzled why people hand-install SDKs when this is so much easier. Obviously | sh anything can be replaced by a download or a brew install for Mac or whatever.

No strong feelings if you want to just remove this as part of this PR.

ok, ill nuke it. I think telling the user how to install java is helpful when you're asking someone to install a specific EOL/unsupported version of java such as 14: it is indeed obscure to locate and install it, it's been unsupported since september 2020.

But, now that problem goes away if the build works with LTS 11+: really things will just work. I don't install any software outside of package manager for a large number of reasons (it will be maintained and patched, stay compatible across upgrades, get security fixes, get catalogued properly in the local package registry, dependencies will be taken care of, ...).

I really have no idea what this sdkman.io is going to do to my computer (even looking at the script it runs, which seems to download more code and then run that).

java 11 is widely available via any standard package manager, whether that's ancient centos 7 yum or bleeding edge arch's pacman. And if you want to install java manually, its easy to get from adoptopenjdk too. So java 11 is a way simpler ask on any user and I don't think we need to instruct users on how to get it.

just as a side, as far as this specific tool piping curl through bash, I am also super suspicious about stuff like that, and don't think we should suggest it to users. I know there are some defenders of the curl | bash, yet we still problems from this stuff such as https://thenewstack.io/not-your-usual-supply-chain-hack-the-codecov-bash-uploader-blunder/

at my job at least, given recent supply chain attacks in the news, there is a lot of tightening of controls around this kind of stuff.

Thanks for calling this out.

nknize · 2021-04-29T20:35:18Z

start gradle check

dblock · 2021-04-29T20:36:44Z

start gradle check

I am not sure we should merge this without lowering the CI Java version, @nknize

opensearch-ci-bot · 2021-04-29T20:57:45Z

✅ Gradle Check success bc72cd3
Log 150

Reports 150

opensearch-ci-bot · 2021-04-29T21:02:48Z

❌ Gradle Check failure bc72cd3
Log 151

Reports 151

rmuir · 2021-04-29T21:20:46Z

The failure there is unrelated to this PR. It is the typical flaky failure caused by assertBusy() stuff in those integration tests. This assert is very confusing, does multiple invocations with arbitrary sleep() calls, and then assertions fail because tasks happen to not complete within some timeframe.

org.opensearch.client.ReindexIT > testDeleteByQueryTask FAILED
    java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([15840D7A65B9BE8E:D855C460211408B1]:0)
        at org.junit.Assert.fail(Assert.java:86)
        at org.junit.Assert.assertTrue(Assert.java:41)
        at org.junit.Assert.assertTrue(Assert.java:52)
        at org.opensearch.client.OpenSearchRestHighLevelClientTestCase.lambda$checkTaskCompletionStatus$2(OpenSearchRestHighLevelClientTestCase.java:360)
        at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:1019)
        at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:992)
        at org.opensearch.client.ReindexIT.testDeleteByQueryTask(ReindexIT.java:327)

dblock · 2021-05-03T20:51:57Z

Thanks @rmuir. You're right about those tests.

Let's leave this open for now and I'll try to chase what's going on for builds. I don't want to be saying use Java 11 and not be running it in CI, but we'll get to it sooner than later.

opensearch-ci-bot · 2021-05-04T23:55:51Z

✅ DCO Check Passed 4f0b498

opensearch-ci-bot · 2021-05-04T23:55:51Z

✅ Gradle Wrapper Validation success 4f0b498

opensearch-ci-bot · 2021-05-05T00:03:04Z

✅ Gradle Precommit success 4f0b498

nknize · 2021-05-15T03:15:17Z

10 days old since last auto update so labeling this as blocked by CI before it goes stale and forgotten

rmuir · 2021-05-15T13:19:38Z

yeah i've already given up. there's nothing wrong with the PR, you can merge it. And there's nothing i can do about your CI system not testing everything you want, its not transparent and should be solved separately.

nknize · 2021-05-17T14:02:12Z

start gradle check

nknize · 2021-05-17T14:05:01Z

yeah i've already given up. there's nothing wrong with the PR, you can merge it. And there's nothing i can do about your CI system not testing everything you want, its not transparent and should be solved separately.

I agree. Two weeks of no CI movement shouldn't prevent a valid PR from being merged.

mikemccand · 2021-06-09T14:30:20Z

+1, this will make the gradle infra far more portable across distros/OSs. Java 11 is an LTS release, heavily used, but Java 14 is not. So this is really a no-brainer -- we should not require Java 14+ at a minimum! Not yet anyways ;)

I am not sure we should merge this without lowering the CI Java version, @nknize

Hmm, where is that CI Java version configured? Is that configuration open-source? Someone could make a quick PR to fix the CI too?

dblock · 2021-06-09T14:35:34Z

Hmm, where is that CI Java version configured? Is that configuration open-source? Someone could make a quick PR to fix the CI too?

Unfortunately not yet, cc: @bbarani.

mikemccand · 2021-06-14T15:32:12Z

Hmm, where is that CI Java version configured? Is that configuration open-source? Someone could make a quick PR to fix the CI too?

Unfortunately not yet, cc: @bbarani.

OK I see, that's too bad -- is there a process to track that internal fix? Maybe we should open a dedicated GitHub issue to switch the internal CI build to JDK 11, and mark it as a blocker for this PR?

And maybe we should also open a separate issue to improve the CI configuration/build to be visible to the full community?

nknize · 2021-06-24T16:37:55Z

Maybe we should open a dedicated GitHub issue to switch the internal CI build to JDK 11, and mark it as a blocker for this PR?

Great idea. And done here

Marking this as stalled.

nknize

Unfortunate that this has stalled for two months. Opened a blocker issue on the devops repo to try and move this forward.

dblock · 2021-06-24T16:48:03Z

I like this, thanks! We also need to lower the Java version in CI, I am not sure where that is.

@dblock Just to be clear this doesn't change how the test suite runs or what is packaged. E.G. if you use java 11 to build, it still downloads java 15 into your gradle caches and uses that java 15 jvm to run unit tests, and it still creates packages in distribution with java 15 images.

So it really only impacts gradle and javac, but I think it makes life easier for developers, packagers, etc. But yes, agree it would be nice for CI to use the minimum required build version to ensure that you can build with java 11.

Well, if we don't both build and run tests on Java 11 we don't know if it works.

rmuir · 2021-06-25T10:19:16Z

Well, if we don't both build and run tests on Java 11 we don't know if it works.

you can be damn sure it doesn't work on Java 11 right now! And from the looks of things, never will.

You can be part of the solution instead of part of the problem. Just let go, let this be a community project instead of an amazon one.

dblock · 2021-06-25T14:46:54Z

You can be part of the solution instead of part of the problem. Just let go, let this be a community project instead of an amazon one.

You're right. Let me try. We got a Jenkins instance with configuration that is not open-source, that is hard-coding JAVA 14. I believe nobody opened an issue on this in https://github.com/opensearch-project/opensearch-build until @nknize did, so this just wasn't seen. My bad too. That CI is owned by @bbarani, I'll make sure he sees it this time.

I really dislike the non-open-source CI - we should be able to modify which version of Java we're building with in this repo. I opened #881 to track that.

The long version is that we had built out a Jenkins cluster for CI that uses very large amounts of hardware to get fast builds and run integration tests. That's what is kicked off when you see those start gradle check comments, DCO checks, etc. This repo doesn't use GitHub actions (it should probably). That hardware and configuration is provisioned and paid for by Amazon via an internal system because it was the fastest way to get it, and we have hard security lines that aren't trivial to cross. So those files can't just be put on GitHub, yet, and maybe we don't want to use Jenkins at all moving forward. I had opened opensearch-project/opensearch-build#73 as something related.

opensearch-ci-bot · 2021-06-25T15:16:10Z

✅ Gradle Check success 4f0b498
Log 254

Reports 254

dblock · 2021-06-25T15:16:58Z

✅ Gradle Check success 4f0b498
Log 254

Reports 254

We can't discuss start and gradle and check without it kicking off 🤦

peterzhuamazon · 2021-06-29T18:50:13Z

Hi @rmuir could you fix the conflicts of the PR?
Would would love to merge it once you have the conflicts resolved.
Thanks.

opensearch-ci-bot · 2021-07-02T22:15:42Z

✅ Gradle Wrapper Validation success 4f0b498

opensearch-ci-bot · 2021-07-02T22:16:30Z

✅ DCO Check Passed 4f0b498

opensearch-ci-bot · 2021-07-02T22:44:36Z

✅ Gradle Precommit success 4f0b498

opensearch-ci-bot · 2021-07-06T15:46:32Z

❌ Gradle Wrapper Validation failure c0680575c1d4d9daa3fcf254b8596aee10d489e3

:alert: Gradle Wrapper integrity has been altered

opensearch-ci-bot · 2021-07-06T15:47:29Z

❌ DCO Check Failed c0680575c1d4d9daa3fcf254b8596aee10d489e3
Run ./dev-tools/signoff-check.sh remotes/origin/main c0680575c1d4d9daa3fcf254b8596aee10d489e3 to check locally
Use git commit with -s to add 'Signed-of-by: {EMAIL}' on impacted commits

opensearch-ci-bot · 2021-07-06T15:47:29Z

❌ Gradle Precommit failure c0680575c1d4d9daa3fcf254b8596aee10d489e3
Log 747

dblock · 2021-07-06T15:52:18Z

I messed up and accidentally force pushed a rewinded branch on rmuir:java_11_build_support. My sincere apologies. I reflog-ed back to it. @rmuir - GitHub won't let me re-force push up to your fork because I suppose the PR is closed. Maybe if you want to force push your local version back up if you have a chance.

I'm finishing this in #940. Thanks for the hard work on this PR and apologies for taking so long (I went offline for a week which didn't help).

dblock · 2021-07-06T16:56:18Z

#940 was merged. I'm tracking down opensearch-project/opensearch-build#74 now.

dblock · 2021-07-09T22:24:53Z

FYI we still haven't switched infra, despite claims of such earlier, I reopened opensearch-project/opensearch-build#74

…opensearch-project#636) * Bump org.apache.httpcomponents.core5:httpcore5-h2 from 5.2.2 to 5.2.3 Bumps [org.apache.httpcomponents.core5:httpcore5-h2](https://github.com/apache/httpcomponents-core) from 5.2.2 to 5.2.3. - [Changelog](https://github.com/apache/httpcomponents-core/blob/master/RELEASE_NOTES.txt) - [Commits](apache/httpcomponents-core@rel/v5.2.2...rel/v5.2.3) --- updated-dependencies: - dependency-name: org.apache.httpcomponents.core5:httpcore5-h2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * Update changelog Signed-off-by: dependabot[bot] <support@github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>

ssgao reviewed Apr 29, 2021

View reviewed changes

dblock self-assigned this May 3, 2021

dblock added CI CI related enhancement Enhancement or improvement to existing feature or request labels May 3, 2021

nknize added Build Libraries & Interfaces blocked Identifies issues that are blocked labels May 15, 2021

nknize added the stalled Issues that have stalled label Jun 24, 2021

nknize approved these changes Jun 24, 2021

View reviewed changes

nknize assigned peterzhuamazon and bbarani Jun 24, 2021

dblock mentioned this pull request Jun 25, 2021

Add ability to modify which version of Java is being used for builds #881

Open

nknize mentioned this pull request Jun 25, 2021

Lower CI Java Version to Java 11 opensearch-project/opensearch-build#74

Closed

dblock closed this Jul 6, 2021

dblock force-pushed the java_11_build_support branch from c068057 to a428e6f Compare July 6, 2021 15:44

dblock mentioned this pull request Jul 6, 2021

Lower build requirement from Java 14+ to Java 11+ #940

Merged

1 task

dblock mentioned this pull request Jul 9, 2021

Cleanup TESTING and DEVELOPER_GUIDE markdowns #946

Merged

1 task

		@@ -50,7 +50,7 @@ sdk install java 14.0.2-open
		sdk use java 14.0.2-open

Lower build requirement from Java 14+ to Java 11+ #636

Lower build requirement from Java 14+ to Java 11+ #636

Conversation

rmuir commented Apr 29, 2021

opensearch-ci-bot commented Apr 29, 2021

opensearch-ci-bot commented Apr 29, 2021

opensearch-ci-bot commented Apr 29, 2021

opensearch-ci-bot commented Apr 29, 2021

opensearch-ci-bot commented Apr 29, 2021

opensearch-ci-bot commented Apr 29, 2021

dblock commented Apr 29, 2021

rmuir commented Apr 29, 2021

dblock commented Apr 29, 2021

rmuir commented Apr 29, 2021

dblock commented Apr 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nknize commented Apr 29, 2021

dblock commented Apr 29, 2021

opensearch-ci-bot commented Apr 29, 2021

opensearch-ci-bot commented Apr 29, 2021

rmuir commented Apr 29, 2021

dblock commented May 3, 2021

opensearch-ci-bot commented May 4, 2021

opensearch-ci-bot commented May 4, 2021

opensearch-ci-bot commented May 5, 2021

nknize commented May 15, 2021 • edited

rmuir commented May 15, 2021

nknize commented May 17, 2021

nknize commented May 17, 2021

mikemccand commented Jun 9, 2021

dblock commented Jun 9, 2021 • edited

mikemccand commented Jun 14, 2021

nknize commented Jun 24, 2021 • edited

nknize left a comment

Choose a reason for hiding this comment

dblock commented Jun 24, 2021

rmuir commented Jun 25, 2021

dblock commented Jun 25, 2021 • edited

opensearch-ci-bot commented Jun 25, 2021

dblock commented Jun 25, 2021

peterzhuamazon commented Jun 29, 2021

opensearch-ci-bot commented Jul 2, 2021

opensearch-ci-bot commented Jul 2, 2021

opensearch-ci-bot commented Jul 2, 2021

opensearch-ci-bot commented Jul 6, 2021

opensearch-ci-bot commented Jul 6, 2021

opensearch-ci-bot commented Jul 6, 2021

dblock commented Jul 6, 2021 • edited

dblock commented Jul 6, 2021

dblock commented Jul 9, 2021 • edited

nknize commented May 15, 2021 •

edited

dblock commented Jun 9, 2021 •

edited

nknize commented Jun 24, 2021 •

edited

dblock commented Jun 25, 2021 •

edited

dblock commented Jul 6, 2021 •

edited

dblock commented Jul 9, 2021 •

edited