- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.6k
Make jdk.jfr import optional in OSGi manifest #5092
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
Make jdk.jfr import optional in OSGi manifest #5092
Conversation
Co-authored-by: laeubi <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, I think it's good to make this consistent with the module descriptor. Please note that we might make it mandatory in the future at which point the JDK used needs to include the jdk.jfr module or add jfr-polyfill to their classpath.
| Import-Package: \ | ||
| ${importAPIGuardian},\ | ||
| ${importJSpecify},\ | ||
| ${importJdkJfr},\ | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move these changes to the build script of junit-platform-launcher as they're specific to that module/bundle (see junit-platform-commons for an example).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcphilipp I'm sadly not very familiar with gradle build scripts and looking at that build-script I'm not completely sure how to archive this, as that one adds an additional header and here we want to modify an existing definition. I could repeat the definition there but it seems a bit odd to maintain then.
Any suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries! I pushed a few commits. I agree that modifying an existing definition is not as DRY as one would wish.
For consistency with its module descriptor. --------- Co-authored-by: Marc Philipp <[email protected]> (cherry picked from commit 729fc28)
| Cherry-picked to  | 
With the recent adoption of JUnit 6 we noticed that JUnit no longer can run on stripped down JVMs that are missing the flight recorder module because it has a mandatory import in the OSGi manifest.
As the flightrecorder integration seems optional, one might want to make the import of the package optional as well
I hereby agree to the terms of the JUnit Contributor License Agreement.
The content was generated by copilote here I squashed the commits, reviewed the result and found it being sane compared to how it was done for other optional imports. Please let me know if any adjustments are necessary as I'm not familiar with gradle.
Definition of Done
@APIannotations