-
-
Notifications
You must be signed in to change notification settings - Fork 275
Add gtest support on jdk15+ #4134
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
base: master
Are you sure you want to change the base?
Conversation
|
I worte it quite clumsy. To keep the build offline, but still built gtests by default, I would like to see something like where |
I think download should be kept in prepareWorkspace.sh, but gtest support can be made conditional.. |
|
Thanks for feedback, I have made gtest optional. Tested (same way as originally + with |
|
Thanx!! Looks nice to me :) |
|
Just side note, you tried that image in jtregs then, right? |
|
@zzambers Why the checks had not run the build of openjdk? Maybe you need to refresh yours actions setup? |
I have not tried to run jtregs with generated image, just examined that libraries for gtest are there. |
|
Results in GHA appeared, there i build failure on windows Error: Details: (Could be bug in older VS (2019), if other systems and also newer VS are not affected. Not yet sure, what to do about it.) |
|
Correction: |
|
I have done quite a lot of testing locally with However, after I updated Btw jdk21u is also affected (but latest jdk is not). I have tried to manually apply some changes to relevant files done in newer jdks (to see if I can work around this somehow). There are only small changes (things like sorting includes...) Did not help (maybe changes in another included files?). There was one change JDK-8347909, which does something about precompiled headers. This gave me an idea to try to run build with Summary:
|
|
I am now in process of bisecting jdk commit, which fixes this issue on newer jdks. (hopefully it will put more light on the problem) |
|
And the winner is JDK-8317132 :) I don't see any change to relevant code, so it seems like adding That is interesting, I would expect permissive mode to be more forgiving and not to fail, where strict/standard-conforming mode passes. I think, it is probably not supposed to be like that and that it is likely bug in VS compiler. I'll probably fill issue to Visual Studio later. |
|
I have filled issue for Visual Studio: https://developercommunity.visualstudio.com/t/OpenJDK-jdk17u-build-failure-with-VS20/10860335 |
|
@zzambers Thanks for you posting feedback. Sorry for leaving a comment here. I saw your feedback and I think it might be more convenient to communicate here. I think: And the stricter Could you please provide a simple example of this problem? It can be a Thanks, |
|
@FrankXie05 Thank you for the swift reply. |
|
@zzambers Thank you for the example, this is the result of my local test: |
@FrankXie05 Didn't work means compiler (i.e. you reproduced this issue) or that reproducer did not work for you? It is compilation without Compilation with |
d914097 to
1b90531
Compare
|
Windows 2022 failure is for downloading gtest: Copilot has this to say: This isn’t the file or GitHub—it’s your Cygwin wget (linked with GnuTLS) tripping over TLS. Browsers on Windows use a different TLS stack (SChannel), so they keep working while Cygwin’s GnuTLS chokes. Here’s the fastest way to get unblocked + the durable fix: Quick workarounds (pick one) wget --secure-protocol=TLSv1_2 -O googletest-1.16.0.tar.gz (This exact switch has helped others seeing “GnuTLS: The request is invalid.”)  curl -L --proto =https --tlsv1.2 -o googletest-1.16.0.tar.gz Proper fix (recommended) Recent GnuTLS releases include important TLS fixes; stale versions often manifest exactly as “The request is invalid.” during handshake. /usr/bin/mk-ca-trust (Out-of-date/incorrect CA bundles typically produce a different error, but it’s quick to rule out.)  Extra diagnostics (if it still fails) wget -d --secure-protocol=TLSv1_2 https://github.com/... gnutls-cli -p 443 github.com Why this happens If you share the output of wget -d (with any tokens redacted), I can pinpoint exactly where the handshake goes sideways and tailor the fix to your host. |
|
I think download of googletest is probably just random infra issue. The other runs show build problem on jdk+win: not yet sure, what is problem there. |
|
I have done some experiments and it seems that build is broken for given commit of adoptium jdk repo: Problem has only shwn up for windows/jdk as testing on linux used different ref/commit. windows/jdk: linux/jdk: Building using temurin-build passed for me locally (windows; same arguments) as it ended up using different commit. However I was able to reproduce build error, if I tried to build jdk manually from the commit as in windows GHA build. |
Adds gtest support for testimage on jdk15+. (in-tree gtest was removed in jdk15) This is needed for some hotspot tests.
Fixes #3956
Testing:
Tested locally on rhel-8 using command:
./makejdk-any-platform.sh -J /usr/lib/jvm/java-17-openjdk --use-adoptium-devkit gcc-11.3.0-Centos7.9.2009-b03 --build-variant temurin jdk17u( fixed the issue, build passed, gtest files appeared in test-image in
hotspot/gtest)