kbendick commented on a change in pull request #2938:
URL: https://github.com/apache/iceberg/pull/2938#discussion_r683956233
##########
File path: dev/source-release.sh
##########
@@ -20,72 +20,137 @@
set -e
-if [ -z "$1" ]; then
- echo "Usage: $0 <version-num> <rc-num>"
- exit
-fi
-
-if [ -z "$2" ]; then
- echo "Usage: $0 <version-num> <rc-num>"
- exit
+usage () {
+ echo "usage: $0 [-k <key_id>] [-r <git_remote>] <version_num> <rc_num>"
+ echo " -k Specify signing key. Defaults to \"GPG default key\""
+ echo " -r Specify Git remote name. Defaults to \"origin\""
+ echo " -d Turn on DEBUG output"
+ exit 1
+}
+
+# Default repository remote name
+remote="origin"
+
+while getopts "k:r:d" opt; do
Review comment:
Huge improvement using `getopts` 👍
##########
File path: dev/source-release.sh
##########
@@ -20,72 +20,137 @@
set -e
-if [ -z "$1" ]; then
- echo "Usage: $0 <version-num> <rc-num>"
- exit
-fi
-
-if [ -z "$2" ]; then
- echo "Usage: $0 <version-num> <rc-num>"
- exit
+usage () {
+ echo "usage: $0 [-k <key_id>] [-r <git_remote>] <version_num> <rc_num>"
+ echo " -k Specify signing key. Defaults to \"GPG default key\""
+ echo " -r Specify Git remote name. Defaults to \"origin\""
+ echo " -d Turn on DEBUG output"
+ exit 1
+}
+
+# Default repository remote name
+remote="origin"
+
+while getopts "k:r:d" opt; do
+ case "${opt}" in
+ k)
+ keyid="${OPTARG}"
+ ;;
+ r)
+ remote="${OPTARG}"
+ ;;
+ d)
+ set -x
+ ;;
+ *)
+ usage
+ ;;
+ esac
+done
+
+shift $((OPTIND-1))
+
+version="$1"
+rc="$2"
+
+if [ -z "$version" ] || [ -z "$rc" ]; then
+ usage
Review comment:
Non-blocking / open-ended question:
Can / should we add a block comment at the top of the script, specifying
that this is for release candidates only?
The script name was a bit confusing for me for a while, as `rc` is required
and it seems this is only for pushing release candidates. Some small indicator
might be a good idea.
I often see shell script top level comments like this, though I don't have a
huge preference. My follow up question would be possibly documenting `version`
and `rc`, though those are arguably self-documenting from the usage function
(I'd still love to see them as `getopts` but I'm open to discussion on that /
doing that later).
```bash
#
# Generate a release-candidate and a text file containing the release
announcement
#
```
##########
File path: dev/source-release.sh
##########
@@ -20,72 +20,137 @@
set -e
-if [ -z "$1" ]; then
- echo "Usage: $0 <version-num> <rc-num>"
- exit
-fi
-
-if [ -z "$2" ]; then
- echo "Usage: $0 <version-num> <rc-num>"
- exit
+usage () {
+ echo "usage: $0 [-k <key_id>] [-r <git_remote>] <version_num> <rc_num>"
+ echo " -k Specify signing key. Defaults to \"GPG default key\""
+ echo " -r Specify Git remote name. Defaults to \"origin\""
+ echo " -d Turn on DEBUG output"
+ exit 1
+}
+
+# Default repository remote name
+remote="origin"
+
+while getopts "k:r:d" opt; do
+ case "${opt}" in
+ k)
Review comment:
(Non-blocking) Should we allow for `--key-id` etc? I think the syntax is
just like this for each case: `-k | --key-id)`?
##########
File path: dev/source-release.sh
##########
@@ -20,72 +20,137 @@
set -e
-if [ -z "$1" ]; then
- echo "Usage: $0 <version-num> <rc-num>"
- exit
-fi
-
-if [ -z "$2" ]; then
- echo "Usage: $0 <version-num> <rc-num>"
- exit
+usage () {
+ echo "usage: $0 [-k <key_id>] [-r <git_remote>] <version_num> <rc_num>"
+ echo " -k Specify signing key. Defaults to \"GPG default key\""
+ echo " -r Specify Git remote name. Defaults to \"origin\""
+ echo " -d Turn on DEBUG output"
+ exit 1
+}
+
+# Default repository remote name
+remote="origin"
+
+while getopts "k:r:d" opt; do
+ case "${opt}" in
+ k)
+ keyid="${OPTARG}"
+ ;;
+ r)
+ remote="${OPTARG}"
+ ;;
+ d)
+ set -x
+ ;;
+ *)
+ usage
+ ;;
+ esac
+done
+
+shift $((OPTIND-1))
+
+version="$1"
+rc="$2"
Review comment:
Should we just make these part of the `getops` workflow or would
deviating too much from the current structure be undesirable?
##########
File path: dev/source-release.sh
##########
@@ -20,72 +20,137 @@
set -e
-if [ -z "$1" ]; then
- echo "Usage: $0 <version-num> <rc-num>"
- exit
-fi
-
-if [ -z "$2" ]; then
- echo "Usage: $0 <version-num> <rc-num>"
- exit
+usage () {
+ echo "usage: $0 [-k <key_id>] [-r <git_remote>] <version_num> <rc_num>"
+ echo " -k Specify signing key. Defaults to \"GPG default key\""
+ echo " -r Specify Git remote name. Defaults to \"origin\""
+ echo " -d Turn on DEBUG output"
+ exit 1
+}
+
+# Default repository remote name
+remote="origin"
+
+while getopts "k:r:d" opt; do
+ case "${opt}" in
+ k)
+ keyid="${OPTARG}"
+ ;;
+ r)
+ remote="${OPTARG}"
+ ;;
+ d)
+ set -x
+ ;;
+ *)
+ usage
+ ;;
+ esac
+done
+
+shift $((OPTIND-1))
+
+version="$1"
+rc="$2"
+
+if [ -z "$version" ] || [ -z "$rc" ]; then
+ usage
fi
-version=$1
-rc=$2
+scriptdir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
+projectdir="$(dirname "$scriptdir")"
+tmpdir=$projectdir/tmp
-if [ -d tmp/ ]; then
- echo "Cannot run: tmp/ exists"
- exit
+if [ -d $tmpdir ]; then
+ echo "Cannot run: $tmpdir already exists"
+ exit 1
fi
-tag=apache-iceberg-$version
-tagrc=${tag}-rc${rc}
+tag="apache-iceberg-$version"
+tagrc="${tag}-rc${rc}"
echo "Preparing source for $tagrc"
-# create version.txt for this release
-echo $version > version.txt
-git add version.txt
-git commit -m "Add version.txt for release $version" version.txt
+echo "Adding version.txt and tagging release..."
+echo $version > $projectdir/version.txt
+git add $projectdir/version.txt
+git commit -m "Add version.txt for release $version" $projectdir/version.txt
set_version_hash=`git rev-list HEAD 2> /dev/null | head -n 1 `
-
git tag -am "Apache Iceberg $version" $tagrc $set_version_hash
-git push apache $tagrc
+
+echo "Pushing $tagrc to $remote..."
+git push $remote $tagrc
release_hash=`git rev-list $tagrc 2> /dev/null | head -n 1 `
if [ -z "$release_hash" ]; then
- echo "Cannot continue: unknown git tag: $tag"
+ echo "Cannot continue: unknown Git tag: $tag"
exit
fi
-echo "Using commit $release_hash"
-
-tarball=$tag.tar.gz
-
# be conservative and use the release hash, even though git produces the same
# archive (identical hashes) using the scm tag
-git archive $release_hash --worktree-attributes --prefix $tag/ -o $tarball
+echo "Creating tarball ${tarball} using commit $release_hash"
+tarball=$tag.tar.gz
+git archive $release_hash --worktree-attributes --prefix $tag/ -o
$projectdir/$tarball
+
+echo "Signing the tarball..."
+[[ -z "$keyid" ]] && keyopt="-u $keyid"
+gpg --detach-sig $keyopt --armor --output ${projectdir}/${tarball}.asc
${projectdir}/$tarball
+sha512sum ${projectdir}/$tarball > ${projectdir}/${tarball}.sha512
Review comment:
Should we assert on the existence of `sha512sum` and other built-ins?
Probably overkill for this script in my opinion as an error will appear.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
