-
Notifications
You must be signed in to change notification settings - Fork 99
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
chore(dist): improve validate-release.sh #329
Conversation
dist/validate-release.sh
Outdated
|
||
ls -lh | ||
pushd ./*hugegraph-incubating*src/hugegraph-server/*hugegraph*"${RELEASE_VERSION}" || exit | ||
bin/init-store.sh || exit | ||
pushd "./*hugegraph-incubating*src/hugegraph-server/*hugegraph*${RELEASE_VERSION}" |
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.
The command should still be pushd ./*hugegraph-incubating*src/hugegraph-server/*hugegraph*"${RELEASE_VERSION}"
otherwise the wildcard *
won't work for matching 🤔
./validate-release.sh: line 172: pushd: ./*hugegraph-incubating*src/hugegraph-server/*hugegraph*1.2.0: No such file or directory
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.
The command should still be
pushd ./*hugegraph-incubating*src/hugegraph-server/*hugegraph*"${RELEASE_VERSION}"
otherwise the wildcard*
won't work for matching 🤔./validate-release.sh: line 172: pushd: ./*hugegraph-incubating*src/hugegraph-server/*hugegraph*1.2.0: No such file or directory
yep, "str"
will override the *
in shell, and also don't put !
in the end of the "str" like echo "run $param !"
(it may occur some bugs in centos)
cd "$(dirname "$0")" || exit | ||
cd "$(dirname "$0")" | ||
pwd | ||
) | ||
|
||
cd "${WORK_DIR}" || exit |
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.
if we remove || exit
, when the input/exec path is not as expected, some files may be deleted or move to wrong space?
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.
we can remove all the || exit
since added set -e
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.
we can remove all the
|| exit
since addedset -e
Get it, thanks, I check & test the usage about it, find some points we need to care (when use set -e
):
set -e
ls not-exist | echo 2 # set -e won't influence the '|"(pipe)
echo "Not Exit"
but use set -eo pipefail
we could also solve the problem:
set -eo pipefail
ls not-exist | echo 2 # exit after "echo 2"
echo "Not Exit"
And set -e
will influence the whole script, if some cmd return non-zero
but we don't want it exit, could use set +e
& set -e
to deactivate it temporarily~
set -e
.....
set +e
cmd_that_shouldnt_exit()
set -e
# Or we could use "cmd || ture"
cmd_that_shouldnt_exit() || ture # bypass -e
....
And in our case, use set -euxo pipefail
seems better? (Remember to Test it)
cc @VGalaxies @Pengzna @liuxiaocs7 @msgui
Refer: https://www.ruanyifeng.com/blog/2017/11/bash-set.html (recommend)
dist/validate-release.sh
Outdated
|
||
ls -lh | ||
pushd ./*hugegraph-incubating*src/hugegraph-server/*hugegraph*"${RELEASE_VERSION}" || exit | ||
bin/init-store.sh || exit | ||
pushd "./*hugegraph-incubating*src/hugegraph-server/*hugegraph*${RELEASE_VERSION}" |
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.
The command should still be
pushd ./*hugegraph-incubating*src/hugegraph-server/*hugegraph*"${RELEASE_VERSION}"
otherwise the wildcard*
won't work for matching 🤔./validate-release.sh: line 172: pushd: ./*hugegraph-incubating*src/hugegraph-server/*hugegraph*1.2.0: No such file or directory
yep, "str"
will override the *
in shell, and also don't put !
in the end of the "str" like echo "run $param !"
(it may occur some bugs in centos)
dist/validate-release.sh
Outdated
USER=$3 | ||
|
||
# this path is just valid in release-ing progress |
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.
# this path is just valid in release-ing progress | |
# this url(dev path) is only valid in the progress of release |
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.
Look almost fine to me, could test it in the release 1.3
@@ -21,11 +21,12 @@ | |||
# 3. Compile the source package & run server & toolchain | |||
# 4. Run server & toolchain in binary package | |||
|
|||
set -e | |||
# if we don't want to exit after '|', remove "-o pipefail" | |||
set -euxo pipefail |
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.
-o pipefail
will exit when COUNT=$(grep -E "$CATEGORY_X" LICENSE NOTICE | wc -l)
Start to check the package content: apache-hugegraph-ai-incubating-1.3.0-src
+ [[ ! -f LICENSE ]]
+ [[ ! -f NOTICE ]]
+ [[ ! -f DISCLAIMER ]]
++ grep -E '\bGPL|\bLGPL|Sleepycat License|BSD-4-Clause|\bBCL\b|JSR-275|Amazon Software License|\bRSAL\b|\bQPL\b|\bSSPL|\bCPOL|\bNPL1|Creative Commons Non-Commercial' LICENSE NOTICE
++ wc -l
+ COUNT=' 0'
continue | ||
fi | ||
# TODO: consider using commands that are entirely consistent with building binary packages | ||
mvn package -DskipTests -Papache-release -ntp -e || exit | ||
mvn package -DskipTests -Papache-release -ntp -e |
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.
compile hugegraph-core error:
Caused by: org.eclipse.aether.resolution.DependencyResolutionException:
Could not find artifact org.apache.hugegraph:hugegraph-common:jar:1.3.0 in central (https://repo.maven.apache.org/maven2)
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.
need use mvn xxx -P stage
to get the latest dependency (this has been done in ci by replacing settings.xml so the script doesn't do it)
@@ -268,57 +277,56 @@ for i in *.tar.gz; do | |||
echo "The package $i shouldn't include empty file: $EMPTY_FILE is empty" && exit 1 | |||
done | |||
|
|||
popd || exit | |||
popd |
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.
The package apache-hugegraph-commons-incubating-1.3.0-src.tar.gz shouldn't include empty directory: ./hugegraph-rpc/target/generated-sources/annotations is empty
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-gpg-plugin:1.6:sign (sign-release-artifacts) on project hugegraph-commons: Exit code: 2 -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-gpg-plugin:1.6:sign (sign-release-artifacts) on project hugegraph-commons: Exit code: 2
[ERROR] Failed to execute goal on project hugegraph-loader: Could not resolve dependencies for project org.apache.hugegraph:hugegraph-loader:jar:1.3.0: Failed to collect dependencies at org.apache.hbase:hbase-mapreduce:jar:2.2.3 -> org.apache.hbase:hbase-server:jar:2.2.3 -> org.glassfish.web:javax.servlet.jsp:jar:2.3.2 -> org.glassfish:javax.el:jar:3.0.1-b06-SNAPSHOT: Failed to read artifact descriptor for org.glassfish:javax.el:jar:3.0.1-b06-SNAPSHOT: Could not transfer artifact org.glassfish:javax.el:pom:3.0.1-b06-SNAPSHOT from/to jvnet-nexus-snapshots (https://maven.java.net/content/repositories/snapshots): transfer failed for https://maven.java.net/content/repositories/snapshots/org/glassfish/javax.el/3.0.1-b06-SNAPSHOT/javax.el-3.0.1-b06-SNAPSHOT.pom: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target -> [Help 1]
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.
same reason
related to #197