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

Do not warn if sun.reflect.Reflection.getCallerClass() is unavailable #1760

Merged
merged 3 commits into from
Oct 5, 2023

Conversation

Flowdalic
Copy link
Contributor

Modern Java releases disallow the access to internal sun.* API. There is not much the user can do about it, hence this warning is not actionable and therefore unneeded.

It also does not tag its origin, causing confusion among users about its whereabouts.

Copy link
Contributor

@ppkarwasz ppkarwasz left a comment

Choose a reason for hiding this comment

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

@Flowdalic,

The log4j-api artifact is a multi-release JAR, so the code in your PR should only run if you are running on JRE 8. On JRE 9+ this class should run.

The typical reason why the Java 8 StackLocator runs on JRE 9+ is shading: a user copies the classes from log4j-api, but is not aware of other setting of the JAR (like the Multi-Release: true entry in the manifest).

@Flowdalic
Copy link
Contributor Author

Thanks for your reply, much appreciated. :)

The typical reason why the Java 8 StackLocator runs on JRE 9+ is shading

That it probably what happened for me. I am using the mill build tool for Scala, which is using Jar Jar Abrams for shading. Probably the shading does not support multi release jars.

BTW, see also #1761.

Note that I like you suggestions, but I will probably not have the resources to implement them.

@ppkarwasz ppkarwasz force-pushed the remove-warning branch 2 times, most recently from be5dc5a to 39ec672 Compare September 6, 2023 17:59
Flowdalic and others added 2 commits September 6, 2023 21:03
Modern Java releases disallow the access to internal sun.* API. There
is not much the user can do about it, hence this warning is not
actionable and therefore unneeded.

It also does not tag its origin, causing confusion among users about
its whereabouts.
@vy
Copy link
Member

vy commented Oct 5, 2023

@Flowdalic, thank so much for the report and the PR, much appreciated! 🙇 Merging it...

@vy vy merged commit 9886625 into apache:2.x Oct 5, 2023
4 checks passed
@vy vy added the enhancement Additions or updates to features label Oct 5, 2023
Flowdalic added a commit to Flowdalic/logging-log4j2 that referenced this pull request Oct 5, 2023
This is a follow up from my pervious PR [1]. Unfortunatly, it only
occured to me after it was merged that the warning message is
incomplete. In my case it was not the runtime environment which did
not support multi-release JARs, but the build system. This adjusts the
message accordingly.

1: apache#1760
@Flowdalic
Copy link
Contributor Author

@Flowdalic, thank so much for the report and the PR, much appreciated! 🙇 Merging it...

Thanks for working on an improved solution and merging this. Unfortunately, I only discovered now that the commit message is incomplete and may blames the wrong entity for missing multi-release JAR support. See my follow up PR at #1833

@Flowdalic Flowdalic deleted the remove-warning branch October 5, 2023 13:26
ppkarwasz added a commit that referenced this pull request Oct 5, 2023
Co-authored-by: Piotr P. Karwasz <[email protected]>
Co-authored-by: Volkan Yazıcı <[email protected]>
vy added a commit that referenced this pull request Oct 6, 2023
Co-authored-by: Piotr P. Karwasz <[email protected]>
Co-authored-by: Volkan Yazıcı <[email protected]>
vy added a commit that referenced this pull request Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Additions or updates to features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants