You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/16 10:38:46 UTC
[GitHub] [doris] wolfboys opened a new pull request, #10918: start|stop script files improvements
wolfboys opened a new pull request, #10918:
URL: https://github.com/apache/doris/pull/10918
## Further comments
start|stop script files improvements:
1) compatible with the case where the script is a soft link ()
2) optimize script readability
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wolfboys commented on a diff in pull request #10918: [Improvement] start|stop script files improvements
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #10918:
URL: https://github.com/apache/doris/pull/10918#discussion_r922996895
##########
bin/start_be.sh:
##########
@@ -16,11 +16,31 @@
# specific language governing permissions and limitations
# under the License.
-curdir=$(dirname "$0")
-curdir=$(
- cd "$curdir"
- pwd
-)
+# resolve links - $0 may be a softlink
+PRG="$0"
+
+while [ -h "$PRG" ] ; do
Review Comment:
yes. this method has been used in my project for a long time, you can see: https://github.com/apache/tomcat/blob/main/bin/startup.sh
##########
bin/start_be.sh:
##########
@@ -16,11 +16,31 @@
# specific language governing permissions and limitations
# under the License.
-curdir=$(dirname "$0")
-curdir=$(
- cd "$curdir"
- pwd
-)
+# resolve links - $0 may be a softlink
+PRG="$0"
+
+while [ -h "$PRG" ] ; do
+ ls=`ls -ld "$PRG"`
+ link=`expr "$ls" : '.*-> \(.*\)$'`
+ if expr "$link" : '/.*' > /dev/null; then
+ PRG="$link"
+ else
+ PRG=`dirname "$PRG"`/"$link"
+ fi
+done
+
+PRGDIR=`dirname "$PRG"`
+
+export DORIS_HOME=`cd "$PRG_DIR/.." >/dev/null; pwd`
+export PID_DIR=`cd "$PRGDIR" >/dev/null; pwd`
Review Comment:
oh, sorry. it's my fault
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] yiguolei commented on pull request #10918: [Improvement] start|stop script files improvements
Posted by GitBox <gi...@apache.org>.
yiguolei commented on PR #10918:
URL: https://github.com/apache/doris/pull/10918#issuecomment-1197544993
please rebase master and submit again.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on pull request #10918: [Improvement] start|stop script files improvements
Posted by GitBox <gi...@apache.org>.
morningman commented on PR #10918:
URL: https://github.com/apache/doris/pull/10918#issuecomment-1197547030
@wolfboys
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wolfboys commented on pull request #10918: [Improvement] start|stop script files improvements
Posted by GitBox <gi...@apache.org>.
wolfboys commented on PR #10918:
URL: https://github.com/apache/doris/pull/10918#issuecomment-1198902068
> @wolfboys
ok
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wolfboys commented on pull request #10918: [Improvement] start|stop script files improvements
Posted by GitBox <gi...@apache.org>.
wolfboys commented on PR #10918:
URL: https://github.com/apache/doris/pull/10918#issuecomment-1193512879
@compiletheworld PTAL
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wolfboys commented on pull request #10918: [Improvement] start|stop script files improvements
Posted by GitBox <gi...@apache.org>.
wolfboys commented on PR #10918:
URL: https://github.com/apache/doris/pull/10918#issuecomment-1196697080
@compiletheworld @morningman PTAL
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] compiletheworld commented on a diff in pull request #10918: [Improvement] start|stop script files improvements
Posted by GitBox <gi...@apache.org>.
compiletheworld commented on code in PR #10918:
URL: https://github.com/apache/doris/pull/10918#discussion_r922924574
##########
bin/stop_be.sh:
##########
@@ -16,14 +16,26 @@
# specific language governing permissions and limitations
# under the License.
-curdir=`dirname "$0"`
-curdir=`cd "$curdir"; pwd`
+# resolve links - $0 may be a softlink
+PRG="$0"
-export DORIS_HOME=`cd "$curdir/.."; pwd`
-export PID_DIR=`cd "$curdir"; pwd`
+while [ -h "$PRG" ] ; do
+ ls=`ls -ld "$PRG"`
+ link=`expr "$ls" : '.*-> \(.*\)$'`
+ if expr "$link" : '/.*' > /dev/null; then
+ PRG="$link"
+ else
+ PRG=`dirname "$PRG"`/"$link"
+ fi
+done
+
+PRGDIR=`dirname "$PRG"`
+
+export DORIS_HOME=`cd "$PRG_DIR/.." >/dev/null; pwd`
+export PID_DIR=`cd "$PRGDIR" >/dev/null; pwd`
signum=9
-if [ "x"$1 = "x--grace" ]; then
+if [ x"$1" != x"--grace" ]; then
Review Comment:
This change may break the original semantic, which seems not correct.
##########
bin/start_be.sh:
##########
@@ -16,11 +16,31 @@
# specific language governing permissions and limitations
# under the License.
-curdir=$(dirname "$0")
-curdir=$(
- cd "$curdir"
- pwd
-)
+# resolve links - $0 may be a softlink
+PRG="$0"
+
+while [ -h "$PRG" ] ; do
+ ls=`ls -ld "$PRG"`
+ link=`expr "$ls" : '.*-> \(.*\)$'`
+ if expr "$link" : '/.*' > /dev/null; then
+ PRG="$link"
+ else
+ PRG=`dirname "$PRG"`/"$link"
+ fi
+done
+
+PRGDIR=`dirname "$PRG"`
+
+export DORIS_HOME=`cd "$PRG_DIR/.." >/dev/null; pwd`
+export PID_DIR=`cd "$PRGDIR" >/dev/null; pwd`
Review Comment:
Is `cd` necessary to evaluate `PID_DIR`? `cd` changes the current directory, which may lead to some subtle bugs.
##########
bin/start_be.sh:
##########
@@ -16,11 +16,31 @@
# specific language governing permissions and limitations
# under the License.
-curdir=$(dirname "$0")
-curdir=$(
- cd "$curdir"
- pwd
-)
+# resolve links - $0 may be a softlink
+PRG="$0"
+
+while [ -h "$PRG" ] ; do
Review Comment:
Consider command `readlink` to resolve the soft link problem?
##########
bin/start_be.sh:
##########
@@ -16,11 +16,31 @@
# specific language governing permissions and limitations
# under the License.
-curdir=$(dirname "$0")
-curdir=$(
- cd "$curdir"
- pwd
-)
+# resolve links - $0 may be a softlink
+PRG="$0"
+
+while [ -h "$PRG" ] ; do
+ ls=`ls -ld "$PRG"`
+ link=`expr "$ls" : '.*-> \(.*\)$'`
+ if expr "$link" : '/.*' > /dev/null; then
+ PRG="$link"
+ else
+ PRG=`dirname "$PRG"`/"$link"
+ fi
+done
+
+PRGDIR=`dirname "$PRG"`
+
+export DORIS_HOME=`cd "$PRG_DIR/.." >/dev/null; pwd`
Review Comment:
Where is `PRG_DIR` defined and what's the different between `PRG_DIR` and `PRGDIR`?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wolfboys closed pull request #10918: [Improvement] start|stop script files improvements
Posted by GitBox <gi...@apache.org>.
wolfboys closed pull request #10918: [Improvement] start|stop script files improvements
URL: https://github.com/apache/doris/pull/10918
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] compiletheworld commented on pull request #10918: [Improvement] start|stop script files improvements
Posted by GitBox <gi...@apache.org>.
compiletheworld commented on PR #10918:
URL: https://github.com/apache/doris/pull/10918#issuecomment-1192104219
LGTM
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wolfboys commented on pull request #10918: [Improvement] start|stop script files improvements
Posted by GitBox <gi...@apache.org>.
wolfboys commented on PR #10918:
URL: https://github.com/apache/doris/pull/10918#issuecomment-1198901941
see #11325
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] compiletheworld commented on pull request #10918: [Improvement] start|stop script files improvements
Posted by GitBox <gi...@apache.org>.
compiletheworld commented on PR #10918:
URL: https://github.com/apache/doris/pull/10918#issuecomment-1186672511
The changes of this PR seem not robust and not well tested, please resolve the comments and resubmit, thank you.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org