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

feat: do not create /bin/java symlink #752

Merged
merged 27 commits into from
Dec 4, 2024

Conversation

vpa1977
Copy link
Contributor

@vpa1977 vpa1977 commented Nov 18, 2024

This PR overrides maven and ant plugin to avoid creating /bin/java symlink : canonical/craft-parts#813

This allows to use those plugins to build rocks.
The plugins still need to set JAVA_HOME manually which is addressed by canonical/craft-parts#895

Raised #754 to address the linter error.

  • Have you signed the CLA?

Maven and Ant plugin should not create /bin/java symlink
Ant and Maven plugins override build commands of craft-parts Ant and Maven plugins.
- add smoke tests for maven and ant plugins.
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Nov 18, 2024
@vpa1977 vpa1977 marked this pull request as ready for review November 18, 2024 20:10
@linostar linostar self-requested a review November 19, 2024 09:50
@tigarmo
Copy link
Collaborator

tigarmo commented Nov 19, 2024

@vpa1977 I'll review this on Thursday (I'm OOO until then)

- use package installation to ensure /usr/bin/java is present
- run jlink to replace modules
- we have not merged JAVA_HOME fix yet, so do basic tests that plugin works
- we still have to create java symlink in rockcraft yaml
-  the test image is fixed for amd64, so its ok to hardcode path in task.yaml
Copy link
Collaborator

@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! Some minor comments but this is pretty close to approved

docs/reference/plugins/ant_plugin.rst Show resolved Hide resolved
docs/reference/plugins/maven_plugin.rst Show resolved Hide resolved
tests/spread/rockcraft/plugin-ant/task.yaml Outdated Show resolved Hide resolved
tests/spread/rockcraft/plugin-ant/task.yaml Outdated Show resolved Hide resolved
tests/spread/rockcraft/plugin-maven/task.yaml Outdated Show resolved Hide resolved
@tigarmo
Copy link
Collaborator

tigarmo commented Nov 28, 2024

@linostar please take a look

@vpa1977 vpa1977 requested a review from tigarmo November 28, 2024 19:11
Reuse common docs for maven and ant plugins.
ant test had part named 'maven-sample'.
Validate that /bin/java is a valid command
craft-parts ant plugin description has too long lines.
@vpa1977
Copy link
Contributor Author

vpa1977 commented Dec 1, 2024

I have added: canonical/craft-parts#916 so that I could make documentation override less horrible in the followup PR.

Copy link
Collaborator

@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!

Copy link
Collaborator

@linostar linostar left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@tigarmo tigarmo merged commit b7ea743 into canonical:main Dec 4, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants