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.

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

chore: Add java17 profile in jar-parent for graalvm #8676

Merged
merged 4 commits into from
Oct 26, 2022
Merged

Conversation

lqiu96
Copy link
Contributor

@lqiu96 lqiu96 commented Oct 24, 2022

Add in java-os-login java 17 profile for GraalVM.

This was removed in #8637 and was a fix from @mpeddada1's old PR in split repo: googleapis/java-os-login#688

@lqiu96 lqiu96 requested a review from meltsufin October 24, 2022 19:44
@lqiu96 lqiu96 changed the title chore: Add in os-login graalvm profile chore: Add in os-login java17 profile for graalvm Oct 24, 2022
Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

This will be deleted again by the scripts. Can we move it to the jar-parent?

@lqiu96
Copy link
Contributor Author

lqiu96 commented Oct 24, 2022

I believe this is an os-login module only issue. I thought it would make more sense here, but I can move it to jar-parent.

@meltsufin
Copy link
Member

We're trying to centralize the config. Since, it's only affecting tests, it should be fine in the parent.

@lqiu96
Copy link
Contributor Author

lqiu96 commented Oct 24, 2022

We're trying to centralize the config. Since, it's only affecting tests, it should be fine in the parent.

Sure, that makes sense. I'm sure there may be future modules that might run into this issue as well. Will update.

@lqiu96 lqiu96 changed the title chore: Add in os-login java17 profile for graalvm chore: Add java17 profile in jar-parent for graalvm Oct 24, 2022
@meltsufin meltsufin added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 24, 2022
@lqiu96
Copy link
Contributor Author

lqiu96 commented Oct 24, 2022

Merge after @meltsufin releases monorepo (10/24)

@meltsufin meltsufin removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 24, 2022
@meltsufin
Copy link
Member

@lqiu96 CI 17 failed. PTAL.

@lqiu96
Copy link
Contributor Author

lqiu96 commented Oct 24, 2022

I'm seeing this error message in the logs:

2022-10-24T20:51:53.3324481Z Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make protected final java.lang.Class java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int,java.security.ProtectionDomain) throws java.lang.ClassFormatError accessible: module java.base does not "opens java.lang" to unnamed module @5a61f5df

Adding --add-opens=java.base/java.lang=ALL-UNNAMED and checking the nightly graalvm jobs to see if this impacts anything.

@meltsufin meltsufin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 24, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 24, 2022
@lqiu96
Copy link
Contributor Author

lqiu96 commented Oct 25, 2022

DialogFlow ITs are failing these two tests: getAgentTest and searchAgentTest:

org.junit.ComparisonFailure: expected:<...gle-cloud-java-tests[]> but was:<...gle-cloud-java-tests[-3b2d2e]>
	at org.junit.Assert.assertEquals(Assert.java:117)
	at org.junit.Assert.assertEquals(Assert.java:146)
	at com.google.cloud.dialogflow.v2.it.ITSystemTest.getAgentTest(ITSystemTest.java:203)

Looking at the past fusion logs, it has been happening for a while. Going to create a draft PR to investigate.

@meltsufin
Copy link
Member

I opened an issue for it: #8681.
Thanks!

@lqiu96 lqiu96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 25, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 25, 2022
@meltsufin meltsufin merged commit 6295dcc into main Oct 26, 2022
@meltsufin meltsufin deleted the main-fix_graalvm branch October 26, 2022 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants