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

Fix GraalVM Native Image compatibility. #2172

Merged
merged 3 commits into from
Jul 14, 2022
Merged

Fix GraalVM Native Image compatibility. #2172

merged 3 commits into from
Jul 14, 2022

Conversation

maciejwalkowiak
Copy link
Contributor

@maciejwalkowiak maciejwalkowiak commented Jul 14, 2022

📜 Description

Remove reflect-config.json file as without GSON, reflection is not used anymore in the project.

💡 Motivation and Context

Fixes #2155

💚 How did you test it?

While having invalid reflect-config.json does not cause issues with GraalVM 22 and results just in a warning, it does break the build for GraalVM 19. This is likely due to --allow-incomplete-classpath parameter enabled by default in GraalVM 22.

It's been tested by running a simplest console application with Sentry integration against different versions of Sentry SDK, GraalVM and present/absent native image related configuration files.

https://github.com/maciej-scratches/sentry-graalvm-native-image-test

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

Spring Native integration is still in TODO but this may wait for Spring Framework 6

Remove reflect-config.json file as without GSON, reflection is not used anymore in the project.
@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2022

Codecov Report

Merging #2172 (e248bd4) into main (8c459df) will not change coverage.
The diff coverage is n/a.

❗ Current head e248bd4 differs from pull request most recent head 72d52b3. Consider uploading reports for the commit 72d52b3 to get more accurate results

@@            Coverage Diff            @@
##               main    #2172   +/-   ##
=========================================
  Coverage     80.95%   80.95%           
  Complexity     3289     3289           
=========================================
  Files           233      233           
  Lines         12048    12048           
  Branches       1596     1596           
=========================================
  Hits           9753     9753           
  Misses         1712     1712           
  Partials        583      583           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c459df...72d52b3. Read the comment docs.

Copy link
Member

@adinauer adinauer left a comment

Choose a reason for hiding this comment

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

LGTM

@adinauer adinauer merged commit 1c65cc8 into main Jul 14, 2022
@adinauer adinauer deleted the gh-2155 branch July 14, 2022 12:23
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.

Invalid reflection config referencing a legacy constructor
3 participants