-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-24318 Create-release scripts fixes and enhancements #1643
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
Conversation
on hbase project only, while retaining special behaviors for hbase sub-projects which share the hbase dist and jira locations.
… from tag step by keeping the tagged git repo.
to <publish-dist|publish-release>. In fact, what was formerly called "build" built the distribution tarballs AND published them to dist, while "publish" built the maven artifacts AND published them to Nexus. The new terminology clarifies what's happening, and removes the appearance of order dependendency.
…. Remove unnecessary use of "-Dmaven.repo.local", put it in settings.xml instead. Stop putting password literals in settings.xml. (It's therefore no longer necessary to delete settings.xml in cleanup, but left that since there is no harm.)
…xed a couple bugs in the current release-tag.sh.
- Fix publish-snapshot so it does same checks as publish-release. - Factor out common maven usages, and move them to build-util.sh. - Change default polarity of DRY_RUN to default to true. Change -n ("no-publish") to -f ("force publish") to actually publish. - Fix problems in do-release.sh so it runs correctly outside of docker, including DRY_RUN being exported. - Have do-release.sh set REPO (shared maven local repository) if doing all three stages. - Cleaned up REPO directory creation. - General cleanup of comments and usage.
- use ${BASH_SOURCE[0]} instead of $0 to determine script directory path - smarter way to read version from pom with mvn - minor bug fixes
…word strings - do maven-gpg-plugin config settings in maven settings file correctly as documented - fix gpg signing failure on Mac due to gpg-agent timeout - remove gpg debug code
…ish-release to work correctly as individual steps and/or without docker - fix another gpg timeout - improve log reporting from publish-release step
…ON with GIT_REF - demote "PACKAGE_VERSION" to "package_version_name" and undocument it in favor of RELEASE_TAG. Still enable appropriate defaulting in case RELEASE_TAG is undefined. - unify RELEASE_VERSION with VERSION, to remove ambiguity and allow it to be set when only running 'publish' step without 'tag' - query confirm RELEASE_TAG - emphasize that release-build.sh is called for a single action at a time, and should be called from do-release.sh. - add '-s' option to do-release.sh - suppress maven "Download from central:" messages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a look through. Looks really good. This stuff is hard. Actually run it is best test. A few comments below. Otherwise, I think we should put it in place for the next RM to exercise.
echo "It may take some time for the tag to be synchronized to github." | ||
echo "Press enter when you've verified that the new tag ($RELEASE_TAG) is available." | ||
read -r | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know where this comes from. The tag is immediate in my experience. A short wait and just move on would be nice future-to-have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently it was in the original code swiped from the Spark project. I think it would be pretty easy to replace it with a little wait loop; I'll do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No hurry. Nice-to-have. Can come later too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. See commit db4bab6 just added.
It waits in 30-second increments up to 5 minutes.
increment version for ongoing dev, and publish to Apache git repo. | ||
publish-dist Build and publish distribution packages (tarballs) to Apache dist repo | ||
publish-snapshot Build and publish maven artifacts snapshot release to Apache snapshots repo | ||
publish-release Build and publish maven artifacts release to Apache release repo, and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this what I do when I want to run a release candidate? Release candidate goes into staging repo first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What one should do to run a release candidate, is invoke do-release-docker.sh
(preferably) or do-release.sh
(if there's a reason not to use docker). I agree with your comments here and in the next item below, that this script must clearly be documented as NOT the entrance script. And yet, the many env variables that get passed around as inter-script communication do need to be documented somewhere. Let me see if I can move them to do-release.sh
, and make the comments and the README more useful.
On a completely unrelated note, you mention, as a question,
Release candidate goes into staging repo first.
That's correct, for non-snapshot releases/release candidates. The business with staging repo in Nexus is part of the publish-release
step. I did not change its basic logic, which is now in release-util.sh::maven_deploy():
- Use mvn to set the version in pom.xml
- Do
mvn deploy
to Nexus, using-P apache-release,release
profiles, which use the maven-release-plugin, nexus-staging-maven-plugin, and configs from the HBase top-level pom and root ASF pom, to manage Nexus deployment. - Afterward (back to release-build.sh, at the end of
publish-release
step), the code expects to find that the Staging Repo has been closed (made immutable), and returns the repo ID to the user. Note that for snapshot releases, the Staging Repo intermediate appears not to be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k. This sounds like it works as it used to which sort of basically worked.
|
||
All other inputs are environment variables. Please use do-release-docker.sh or | ||
do-release.sh to set up the needed environment variables. This script, release-build.sh, | ||
is not intended to be called stand-alone, and such use is untested. The env variables used are: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we need an entrance script or a rename of the one of these scripts as main.sh or some such. As is, operator has to consult the README. I suppose this ok given how specialized this all is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was letting it go due to how specialized it is, but you're right; there's no reason not to make it nice and clear. Will try to improve this; see comment immediately above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting late, will tackle this in the morning.
GPG_PASSPHRASE - Passphrase for GPG key | ||
REPO - Set to full path of a directory to use as maven local repo (dependencies cache) | ||
to avoid re-downloading dependencies for each stage. It is automatically set if you | ||
request full sequence of stages (tag, publish-dist, publish-release) in do-release.sh. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good doc.
I have been working through trying to use the release scripts to roll 3.0.0-alpha-1. unless it is pressing I would like to incorporate this PR into me doing that as a review |
@saintstack , thank you for the thorough review, I will respond to all comments momentarily. @busbey , I'd be delighted to have you use it. This can be done two ways:
In either case, I'd be happy to respond promptly to any problems you find. |
@busbey and @ndimiduk You both are about to RM. You could review this as sub-task of RM'ing but may I suggest that you'll have your hands full RM'ing anyways; just take what is here and build on its benefit? It might be a bit hard to review because it does some renaming toward improved clarity and moves some actions around to make more sensible grouping. Just a suggestion. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
As I said previously, if this is pressing for someone go ahead. I am already on a forked branch in order to fix problems with the release creation tooling as I find them getting towards RC0. As I become sure that the things I am fixing are actually fixed, I'll post them up for reviews. I'm trying to minimize repeated updates or addenda as I figure out that something was only fixed for a part of the RC generation process. I had wanted to incorporate this change on the branch I'm on (https://github.com/busbey/hbase/tree/HBASE-23339) so that I could ensure they work prior to HBase 3 alpha-1 rather then when I'm trying to maintain a regular cadence of alpha releases. If you are not willing to wait for that for some reason then by all means go ahead. I am likely to ignore them until after alpha-1 RCs are done in that case. |
Thanks for working through all of this @mattf-apache! |
Thanks, @busbey ! |
Clean up
dev-support/create-release
code, fix bugs, remove ambiguities, and clarify usage, to enable:tag
,publish-dist
,publish-snapshot
, andpublish-release
(the latter two being mutually exclusive alternatives).shellcheck
errors in the scripts, and removed ambiguities and redundancies in the many environment variables used.There is more detail in the commit logs attached.
In addition, the changes move the code toward being more general / less HBase-specific, so it can be run on any Apache project (while still accommodating HBase-specific features regarding how sub-projects are named and organized in Jira and release repos). In future I propose to take it further along that path, and move create-release into Yetus (recognizing that this create-release code has been passed between a couple other projects already).
These changes have NO IMPACT on HBase functionality.
TESTING: The scripts have been tested manually on Docker and Mac environments, singly and together, in dry run mode, on branch-2.3 builds. Testing in "live" mode (not dry run) will require cooperation from a Release Manager, as we'd like to avoid putting test material into the actual repos and then have to clean it up.