acelyc111 commented on code in PR #2034:
URL:
https://github.com/apache/incubator-pegasus/pull/2034#discussion_r1618507929
##########
scripts/pack_server.sh:
##########
@@ -109,8 +109,28 @@ while [[ $# > 0 ]]; do
shift
done
+if [[ $separate_servers == "false" ]] && [[ ! -f
${BUILD_LATEST_DIR}/output/bin/pegasus_server/pegasus_server ]]; then
+ echo "ERROR: ${BUILD_LATEST_DIR}/output/bin/pegasus_server/pegasus_server
not found"
+ exit 1
+fi
+
+if [[ $separate_servers == "true" ]] && [[ ! -f
${BUILD_LATEST_DIR}/output/bin/pegasus_meta_server/pegasus_meta_server ]]; then
+ echo "ERROR:
${BUILD_LATEST_DIR}/output/bin/pegasus_meta_server/pegasus_meta_server not
found"
+ exit 1
+fi
+
+if [[ $separate_servers == "true" ]] && [[ ! -f
${BUILD_LATEST_DIR}/output/bin/pegasus_replica_server/pegasus_replica_server
]]; then
+ echo "ERROR:
${BUILD_LATEST_DIR}/output/bin/pegasus_replica_server/pegasus_replica_server
not found"
+ exit 1
+fi
Review Comment:
Will `copy_file` throw an error if the source file not exist? You can remove
the manual judgements if it will.
##########
scripts/pack_server.sh:
##########
@@ -27,6 +27,7 @@ function usage() {
echo " -g|--custom-gcc"
echo " -k|--keytab-file"
echo " -j|--use-jemalloc"
+ echo " -s|--separate_servers"
Review Comment:
How about adding the new flag to the BUILD_OPTIONS of the recently added CI
job build_debug_on_centos7 [1] to ensure everything works well as expected.
1.
https://github.com/apache/incubator-pegasus/blob/master/.github/workflows/lint_and_test_cpp.yaml#L421
##########
scripts/pack_server.sh:
##########
@@ -78,6 +74,7 @@ fi
custom_gcc="false"
keytab_file=""
use_jemalloc="off"
+separate_servers="false"
Review Comment:
Could you please unify to use true/false or on/off for all the switches
above?
##########
scripts/pack_client.sh:
##########
@@ -105,6 +103,21 @@ while [[ $# > 0 ]]; do
shift
done
+if [[ $separate_servers == "false" ]] && [[ ! -f
${BUILD_LATEST_DIR}/output/bin/pegasus_server/pegasus_server ]]; then
Review Comment:
Now is packing client libs, why judge the server?
##########
scripts/pack_common.sh:
##########
@@ -20,7 +20,11 @@ set -e
function get_stdcpp_lib()
{
- libname=`ldd ${BUILD_LATEST_DIR}/output/bin/pegasus_server/pegasus_server
2>/dev/null | grep libstdc++`
+ if [[ $2 == "false" ]]; then
+ libname=`ldd
${BUILD_LATEST_DIR}/output/bin/pegasus_server/pegasus_server 2>/dev/null | grep
libstdc++`
+ else
+ libname=`ldd
${BUILD_LATEST_DIR}/output/bin/pegasus_meta_server/pegasus_meta_server
2>/dev/null | grep libstdc++`
Review Comment:
pegasus_meta_server and pegasus_replica_server depends on different libs,
it's needed to find both of the servers' dependencies.
--
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]
