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.

Sign up for GitHub

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

Jump to bottom

feat: implement Jlink plugin #951

Open
wants to merge 48 commits into
base: main
Choose a base branch
from
Open

feat: implement Jlink plugin #951

wants to merge 48 commits into from
+495 −1

Conversation

vpa1977
Copy link
Contributor

@vpa1977 vpa1977 commented Jan 6, 2025 •
edited
Loading

This MR resolves #891

It adds jlink plugin along with unit and integration tests and plugin documentation.

  • Have you signed the CLA?
  • Have you added an entry to the changelog (docs/reference/changelog.rst)?

vpa1977 added 2 commits January 6, 2025 16:13
@vpa1977
feat: add jlink plugin
c12302c 
Introduce jlink plugin to create Java runtime
for the application.
canonical#891
@vpa1977
doc: add jlink plugin documentation
cf7a732 
Add initial documentation for the jlink plugin.
@vpa1977 vpa1977 marked this pull request as ready for review January 8, 2025 03:55
@tigarmo tigarmo requested review from tigarmo and mr-cal January 14, 2025 12:48
tigarmo
tigarmo reviewed Jan 16, 2025
View reviewed changes
.github/workflows/tests.yaml Show resolved Hide resolved
craft_parts/plugins/jlink_plugin.py Outdated Show resolved Hide resolved
craft_parts/plugins/jlink_plugin.py Outdated Show resolved Hide resolved
@vpa1977
feat: use JAVA_HOME and deduce version from it
44db443 
The JAVA_HOME is prepended to the classpath and
if defined overrides JDK that is used by jlink plugin.

The jlink_java_version option is dropped as the version is determined by the jar version.
@vpa1977
doc: update documentation
8af0327 
- Add JAVA_HOME usage
- drop jlink_java_version
@vpa1977
Merge branch 'jlink-plugin' of github.com:vpa1977/craft-parts into jl…
a59bfb0 
…ink-plugin
@vpa1977
Merge remote-tracking branch 'upstream/main' into jlink-plugin
14ca2d3
@vpa1977
lint: dependant -> dependent
bc74bd6
@vpa1977
test: drop test for java version
2452b20 
The property is dropped from the plugin.
Drop the test.
@vpa1977
doc: add missing words for the spellcheck
213d18f
@vpa1977
test: drop integration test for java_version property
5922bbb
@vpa1977
lint: drop unused import
6251ec3
@vpa1977 vpa1977 requested a review from tigarmo January 20, 2025 05:32
tigarmo
tigarmo requested changes Jan 22, 2025
View reviewed changes
Copy link
Contributor

@tigarmo tigarmo left a comment

Choose a reason for hiding this comment

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

thanks!

craft_parts/plugins/jlink_plugin.py Outdated Show resolved Hide resolved
craft_parts/plugins/jlink_plugin.py Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/jlink_plugin.rst Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/jlink_plugin.rst Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/jlink_plugin.rst Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/jlink_plugin.rst Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/jlink_plugin.rst Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/jlink_plugin.rst Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/jlink_plugin.rst Outdated Show resolved Hide resolved
docs/reference/changelog.rst Outdated Show resolved Hide resolved
@tigarmo tigarmo requested a review from medubelko January 22, 2025 19:54
vpa1977 and others added 10 commits January 23, 2025 15:44
@vpa1977 @medubelko
Update docs/reference/changelog.rst
f3a0c7f 
Co-authored-by: Michael DuBelko <[email protected]>
@vpa1977 @medubelko
Update docs/common/craft-parts/reference/plugins/jlink_plugin.rst
41ecc03 
Co-authored-by: Michael DuBelko <[email protected]>
@vpa1977 @medubelko
Update docs/common/craft-parts/reference/plugins/jlink_plugin.rst
693a1fe 
Co-authored-by: Michael DuBelko <[email protected]>
@vpa1977 @medubelko
Update docs/common/craft-parts/reference/plugins/jlink_plugin.rst
0996063 
Co-authored-by: Michael DuBelko <[email protected]>
@vpa1977
Merge remote-tracking branch 'upstream/main' into jlink-plugin
a8d6168
@vpa1977
fix: set and validate JAVA_HOME
126ee3d 
Instead of altering the runtime path, set JAVA_HOME and validate that JAVA_HOME/bin/java is an executable
@vpa1977
doc: format yaml blocks
789693c
@vpa1977
doc: rename jar -> JAR
972b828
@vpa1977
doc: openjdk -> OpenJDK
62b2dcd
@vpa1977
doc: declare openjdk build-package
05625d3
@vpa1977 vpa1977 requested review from medubelko and tigarmo January 23, 2025 19:08
tigarmo
tigarmo approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@tigarmo tigarmo left a comment

Choose a reason for hiding this comment

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

Thanks a lot, this is great. @medubelko @mr-cal can we get this across the finish line?

mr-cal
mr-cal requested changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@mr-cal mr-cal left a comment

Choose a reason for hiding this comment

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

Nice work, this is looking good.

craft_parts/plugins/jlink_plugin.py Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/jlink_plugin.rst Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/jlink_plugin.rst Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/jlink_plugin.rst Outdated Show resolved Hide resolved
tests/unit/plugins/test_jlink_plugin.py Outdated Show resolved Hide resolved
docs/reference/changelog.rst Outdated Show resolved Hide resolved
medubelko
medubelko requested changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@medubelko medubelko left a comment

Choose a reason for hiding this comment

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

There are some remaining YAML indentation quirks.

docs/common/craft-parts/reference/plugins/jlink_plugin.rst Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/jlink_plugin.rst Outdated Show resolved Hide resolved
docs/reference/changelog.rst Outdated Show resolved Hide resolved
@vpa1977 vpa1977 requested a review from mr-cal January 25, 2025 02:03
mr-cal
mr-cal approved these changes Jan 27, 2025
View reviewed changes
Copy link
Contributor

@mr-cal mr-cal left a comment

Choose a reason for hiding this comment

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

Thanks!

@vpa1977, have you tested building a simple snap, rock, or charm with the jlink plugin? I'm happy to publish a version of snapcraft with this plugin to make it easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mr-cal mr-cal mr-cal approved these changes

@medubelko medubelko medubelko approved these changes

@tigarmo tigarmo tigarmo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Jlink support for Java plugin
4 participants
@vpa1977 @tigarmo @medubelko @mr-cal