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

build(pom): pull -core dep from GitHub Packages #18

Merged
merged 6 commits into from
Dec 22, 2022

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Dec 21, 2022

Replaces #17
Related to cryostatio/cryostat-core#92
Consume cryostat-core from GitHub Packages Maven repository. Released versions of -core built by CI can be pulled and use as dependency for builds. This requires a GH pkgs authentication token secret to be configured in the repository secrets. Pull requests from forks will not be passed this token and so CI will fail on them - this means that all PRs in this repo, and any others that consume dependencies in this way, need to be created from upstream branches, not fork branches. See #4 vs #19.

@andrewazores andrewazores added build chore Refactor, rename, cleanup, etc. labels Dec 21, 2022
@github-actions
Copy link

Hi @andrewazores! Add at least one of the required labels to this PR

Required labels are : chore,ci,cleanup,docs,feat,fix,perf,refactor,style,test

@mergify
Copy link

mergify bot commented Dec 21, 2022

⚠️ The sha of the head commit of this PR conflicts with #17. Mergify cannot evaluate rules on this PR. ⚠️

@andrewazores
Copy link
Member Author

Other repos using pull_request_target events do have access to secrets on PRs from forks and that's what the additional automation we have around those PRs is for (ensuring that the tests don't run until they're inspected and deemed safe/trusted). The same setup could be applied here as well. For now I expect the development to be slower and unlikely to have as many external contributors, so those additional PR CI setups can be foregone for the time being.

Copy link
Member

@ebaron ebaron left a comment

Choose a reason for hiding this comment

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

Testing locally, it seems like the previous model of installing core into your local Maven repo still works? I thought we might want a profile for CI to use that connected with the GitHub Maven repo, but that doesn't seem to be necessary.

@andrewazores
Copy link
Member Author

Right, if you manually check out and install the -core dep (or any other I suppose) into ~/.m2, then Maven will just pick that up and use it and not bother trying to download it from the remote repository, so this bypasses the need to configure a PAT for accessing the GitHub Packages repository. If we were using -SNAPSHOT versions and you tried to do ex. mvn -U package then this would necessitate checking the remote(s) for updates and you'd hit the authentication snag.

Putting the config behind a -Pgithub or -Pci might still prove to be a smoother or at least less surprising experience for developers, especially if we do enable -SNAPSHOT builds, but it does look like the current setup works well enough with little to no changes to the existing developer workflow.

Copy link
Member

@ebaron ebaron left a comment

Choose a reason for hiding this comment

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

Sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build chore Refactor, rename, cleanup, etc.
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants