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

[JENKINS-68930] Fix Java17 Error on agent start #206

Merged
merged 3 commits into from Aug 1, 2022

Conversation

bulanovk
Copy link
Contributor

@bulanovk bulanovk commented Jul 5, 2022

See JENKINS-66896 and JENKINS-68930

Use WA to work with modifiers field filtered since JDK12.
--add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.lang.reflect=ALL-UNNAMED are required.

@bulanovk bulanovk requested a review from a team as a code owner July 5, 2022 11:41
--add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.lang.reflect=ALL-UNNAMED
@NotMyFault NotMyFault changed the title Fix JENKINS-68930 Java17 Error on slave start Fix JENKINS-68930 Java17 Error on agent start Jul 5, 2022
@NotMyFault NotMyFault added the bug label Jul 5, 2022
@NotMyFault NotMyFault changed the title Fix JENKINS-68930 Java17 Error on agent start [JENKINS-68930] Fix Java17 Error on agent start Jul 5, 2022
@NotMyFault
Copy link
Member

Untested, but looks good so far. Thanks for filing the PR!

…tMasterEnvVarsSetter.java

Co-authored-by: Alexander Brandes <brandes.alexander@web.de>
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! It will likely fail SpotBugs, because the return value of the new method might be null. In the end you should check for null and throw NoSuchMethodException to retain the behavior

@bulanovk
Copy link
Contributor Author

Any chance to get this PR merged?

@bulanovk bulanovk requested a review from NotMyFault July 25, 2022 06:02
Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

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

Looks good, do you mind taking another look @oleg-nenashev ?

@limeman40
Copy link

Can someone review the change request so this could possibly be merged?

@limeman40
Copy link

@oleg-nenashev Can you please review this so it can possible get merged?

@NotMyFault NotMyFault merged commit 9b9e962 into jenkinsci:master Aug 1, 2022
@bulanovk bulanovk deleted the JENKINS-68930 branch August 1, 2022 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants