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

Add support for local maven deployment #355

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

jerboaa
Copy link
Collaborator

@jerboaa jerboaa commented Jul 20, 2023

Example usage which installs the current version into the local maven repo (~/.m2 on Linux):

$ java -ea ./build.java --maven-deploy-local --mandrel-version $MANDREL_VERSION --verbose --mx-home $MX_HOME --mandrel-repo $MANDREL_REPO --mandrel-home $MANDREL_HOME

Closes: #354

@jerboaa jerboaa requested a review from zakkak July 20, 2023 13:56
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jul 20, 2023
Copy link
Collaborator

@zakkak zakkak left a comment

Choose a reason for hiding this comment

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

The PR looks good to me but I wonder if it's really necessary.

Can't you achieve local deployment with --maven-repo-id local --maven-version 3.6.0 --maven-url file://$HOME/.m2/repository

@jerboaa
Copy link
Collaborator Author

jerboaa commented Jul 21, 2023

Thanks for the review! Don't merge this just yet (still some things coming).

Can't you achieve local deployment with --maven-repo-id local --maven-version 3.6.0 --maven-url file://$HOME/.m2/repository

It's a mvn deploy vs. mvn install call. I'd prefer an install so as to not even attempt to go remote. Also using the internal install logic frees us from trying to re-dediscover the local repo and all that.

@jerboaa jerboaa force-pushed the add-maven-local-deploy branch from ed8e39c to 4323ee1 Compare July 21, 2023 12:48
@jerboaa
Copy link
Collaborator Author

jerboaa commented Jul 21, 2023

@zakkak Please review again. graalvm/mandrel#526 (comment) contains a draft as to where this is going.

Copy link
Collaborator

@zakkak zakkak left a comment

Choose a reason for hiding this comment

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

Still LGTM, just added two minor comments.

build.java Outdated Show resolved Hide resolved
build.java Show resolved Hide resolved
@github-actions
Copy link

This PR appears to be stale because it has been open 30 days with no activity. This PR will be closed in 7 days unless Stale label is removed or a new comment is made.

Comment on lines +967 to +977
"sdk:NATIVEIMAGE," +
"sdk:COLLECTIONS," +
"sdk:POLYGLOT," +
"sdk:WORD," +
Copy link
Collaborator Author

@jerboaa jerboaa Aug 21, 2023

Choose a reason for hiding this comment

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

These four sub-modules need to be added to the deploy artifacts list, since otherwise only GRAAL_SDK gets deployed to the local maven repo and its dependencies won't get resolved properly (as they're not in the local repo). This is in-line with the graal-sdk artefact split.

@jerboaa
Copy link
Collaborator Author

jerboaa commented Aug 21, 2023

This PR depends on #360

Copy link
Collaborator

@zakkak zakkak 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 @jerboaa

@jerboaa jerboaa force-pushed the add-maven-local-deploy branch 3 times, most recently from 04296b8 to ccd8489 Compare September 5, 2023 14:39
@jerboaa jerboaa force-pushed the add-maven-local-deploy branch from ccd8489 to b12cc4f Compare September 6, 2023 16:04
@jerboaa
Copy link
Collaborator Author

jerboaa commented Sep 6, 2023

CI passes with this in the 23.1 branch. See https://github.com/jerboaa/mandrel-packaging/actions/runs/6099726559

I'm merging this.

@jerboaa jerboaa merged commit b2e26df into graalvm:master Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-stale OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for local maven deploy
2 participants