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

Make jre folder platform dependent #632

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

chippmann
Copy link
Contributor

This makes the jre folder platform dependent.

This makes it possible for a project to have multiple jre folder simultaneously on the file system. The module then automatically picks the correct one for the current platform.
The main advantage of this is that on a project with multiple people working on it, the embedded jre can be committed to the repository for each platform together with the built jar and a project member does not need to have java locally installed and JAVA_HOME set. Which is especially useful if one is working with non programmers.

Note: Breaking change. Needs special notes in release notes!

@chippmann chippmann marked this pull request as ready for review May 12, 2024 17:27
piiertho
piiertho previously approved these changes May 18, 2024
Copy link
Member

@piiertho piiertho left a comment

Choose a reason for hiding this comment

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

I think we need to test in exports to be sure we don't break them.

@chippmann
Copy link
Contributor Author

I tested linux editor and export templates. Could you test windows and mac? (I don't think testing on mobile is necessary but you can if you want to :-) )

@@ -65,8 +65,19 @@ static constexpr const char* GRAAL_NATIVE_IMAGE_FILE {IOS_GRAAL_NATIVE_IMAGE_FIL

#endif

#ifdef X11_ENABLED
Copy link
Member

Choose a reason for hiding this comment

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

The else statement doesn't make sense to have anymore.
Also I would simply make all those paths not unique to an OS because if you want to go the extra mile, it should be possible to do cross platform exports ( For example exporting for Linux on a Windows editor).
I recommend doing like I did with all the other paths above, with the specialised path available to everyone, and then a preprocessor that pick the one specific to your system. That way, you can freely use the right path in the export plugin.

@chippmann
Copy link
Contributor Author

chippmann commented May 26, 2024

Tested the exports and editors for linux amd64 with the changes @CedNaru requested.
Will test on macos later today

@piiertho need a review again from you as these were quite significant changes again

@chippmann chippmann force-pushed the feature/jre-folder-per-platform-and-arch branch from 96984d5 to d51d4b9 Compare May 26, 2024 10:43
@chippmann
Copy link
Contributor Author

Tested editor and export on macOS as well.

If you @CedNaru or @piiertho could test on windows, that would be appreciated.

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