You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2022/08/10 11:48:09 UTC
[GitHub] [ratis] leo65535 opened a new pull request, #711: RATIS-1669. Combine shell lib folder and root jars folder
leo65535 opened a new pull request, #711:
URL: https://github.com/apache/ratis/pull/711
## What changes were proposed in this pull request?
The shell jar in the lib folder, while other jars in jars folder, we can put them together.
```
[dcadmin@dcadmin-work ratis]$ ll -a ratis-assembly/target/apache-ratis-3.0.0-SNAPSHOT
total 36
drwxr-xr-x 11 dcadmin dcadmin 178 Jan 22 2020 .
drwxrwxr-x 7 dcadmin dcadmin 264 Aug 8 11:31 ..
drwxr-xr-x 2 dcadmin dcadmin 19 Jan 22 2020 bin
drwxr-xr-x 2 dcadmin dcadmin 30 Jan 22 2020 conf
drwxr-xr-x 7 dcadmin dcadmin 212 Jan 22 2020 dev-support
drwxr-xr-x 2 dcadmin dcadmin 4096 Jan 22 2020 jars
drwxr-xr-x 3 dcadmin dcadmin 19 Jan 22 2020 lib
drwxr-xr-x 2 dcadmin dcadmin 35 Jan 22 2020 libexec
-rw-r--r-- 1 dcadmin dcadmin 13394 Jan 22 2020 LICENSE
-rw-r--r-- 1 dcadmin dcadmin 5246 Jan 22 2020 NOTICE
-rw-r--r-- 1 dcadmin dcadmin 1201 Jan 22 2020 README.md
drwxr-xr-x 21 dcadmin dcadmin 4096 Jan 22 2020 src
```
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1669
## How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #711:
URL: https://github.com/apache/ratis/pull/711#issuecomment-1212634573
> @leo65535 , thanks for the update! There are some classes in ratis-shell-3.0.0-SNAPSHOT.jar duplicated with the classes in other jars.
Nice catch, I had remove the shade plugin from the shell module, the patch has been updated, thanks.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r944235614
##########
ratis-shell/src/main/libexec/ratis-shell-config.sh:
##########
@@ -58,6 +59,16 @@ if [[ ${JAVA_MAJORMINOR} != 001008 && ${JAVA_MAJOR} != 011 ]]; then
exit 1
fi
+local RATIS_SHELL_CLASSPATH
+
+while read -d '' -r jarfile ; do
+ if [[ "$RATIS_SHELL_CLASSPATH" == "" ]]; then
+ RATIS_SHELL_CLASSPATH="$jarfile";
+ else
+ RATIS_SHELL_CLASSPATH="$RATIS_SHELL_CLASSPATH":"$jarfile"
+ fi
+done < <(find "$RATIS_SHELL_LIB_DIR" ! -type d -name '*.jar' -print0 | sort -z)
Review Comment:
@leo65535 , bash worked file. Thanks!
It seems that tar somehow have not preserved the permissions
```shell
$tar tvzf ratis-assembly/target/apache-ratis-3.0.0-SNAPSHOT-bin.tar.gz
...
-rw-r--r-- 0 root root 2152 Jan 22 2020 apache-ratis-3.0.0-SNAPSHOT-bin/bin/ratis
-rw-r--r-- 0 root root 1580 Jan 22 2020 apache-ratis-3.0.0-SNAPSHOT-bin/conf/log4j.properties
-rw-r--r-- 0 root root 7118 Jan 22 2020 apache-ratis-3.0.0-SNAPSHOT-bin/examples/README.md
-rw-r--r-- 0 root root 1138 Jan 22 2020 apache-ratis-3.0.0-SNAPSHOT-bin/examples/bin/client.sh
-rw-r--r-- 0 root root 1859 Jan 22 2020 apache-ratis-3.0.0-SNAPSHOT-bin/examples/bin/common.sh
-rw-r--r-- 0 root root 921 Jan 22 2020 apache-ratis-3.0.0-SNAPSHOT-bin/examples/bin/server.sh
-rw-r--r-- 0 root root 1598 Jan 22 2020 apache-ratis-3.0.0-SNAPSHOT-bin/examples/bin/start-all.sh
-rw-r--r-- 0 root root 922 Jan 22 2020 apache-ratis-3.0.0-SNAPSHOT-bin/examples/bin/stop-all.sh
...
```
The permissions are correct in ratis-assembly/target dir
```shell
$ls -l ratis-assembly/target/apache-ratis-3.0.0-SNAPSHOT-bin/apache-ratis-3.0.0-SNAPSHOT-bin/bin/
total 8
-rwxr-xr-x 1 szetszwo staff 2152 Jan 22 2020 ratis
$ls -l ratis-assembly/target/apache-ratis-3.0.0-SNAPSHOT-bin/apache-ratis-3.0.0-SNAPSHOT-bin/examples/bin/
total 40
-rwxr-xr-x 1 szetszwo staff 1138 Jan 22 2020 client.sh
-rwxr-xr-x 1 szetszwo staff 1859 Jan 22 2020 common.sh
-rwxr-xr-x 1 szetszwo staff 921 Jan 22 2020 server.sh
-rwxr-xr-x 1 szetszwo staff 1598 Jan 22 2020 start-all.sh
-rwxr-xr-x 1 szetszwo staff 922 Jan 22 2020 stop-all.sh
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #711:
URL: https://github.com/apache/ratis/pull/711#issuecomment-1211498534
hi @szetszwo, take a final check.
I forget to exclude the `example` from the pkg, but it seems not a good way to resolve it. Trying to think a best way to solve it.
Do we need to exclude it in this patch?
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
szetszwo commented on PR #711:
URL: https://github.com/apache/ratis/pull/711#issuecomment-1213372443
The script files inside apache-ratis-2.3.0-bin.tar.gz also do not have execute permission
```shell
$tar tvf apache-ratis-2.3.0-bin.tar.gz | grep sh
-rw-r--r-- 0 root root 3396 Jan 22 2020 apache-ratis-2.3.0/dev-support/checks/_mvn_unit_report.sh
-rw-r--r-- 0 root root 1189 Jan 22 2020 apache-ratis-2.3.0/dev-support/checks/author.sh
-rw-r--r-- 0 root root 1000 Jan 22 2020 apache-ratis-2.3.0/dev-support/checks/build.sh
-rw-r--r-- 0 root root 1937 Jan 22 2020 apache-ratis-2.3.0/dev-support/checks/checkstyle.sh
-rw-r--r-- 0 root root 1695 Jan 22 2020 apache-ratis-2.3.0/dev-support/checks/findbugs.sh
-rw-r--r-- 0 root root 1268 Jan 22 2020 apache-ratis-2.3.0/dev-support/checks/rat.sh
-rw-r--r-- 0 root root 1384 Jan 22 2020 apache-ratis-2.3.0/dev-support/checks/shellcheck.sh
-rw-r--r-- 0 root root 1197 Jan 22 2020 apache-ratis-2.3.0/dev-support/checks/sonar.sh
-rw-r--r-- 0 root root 1249 Jan 22 2020 apache-ratis-2.3.0/dev-support/checks/unit.sh
-rw-r--r-- 0 root root 1690 Jan 22 2020 apache-ratis-2.3.0/dev-support/ci/common.sh
-rw-r--r-- 0 root root 2103 Jan 22 2020 apache-ratis-2.3.0/dev-support/ci/nightly-build.sh
-rw-r--r-- 0 root root 2113 Jan 22 2020 apache-ratis-2.3.0/dev-support/ci/precommit-build.sh
-rw-r--r-- 0 root root 1091 Jan 22 2020 apache-ratis-2.3.0/dev-support/intellij/install-runconfig.sh
-rw-r--r-- 0 root root 6319 Jan 22 2020 apache-ratis-2.3.0/dev-support/make_rc.sh
-rw-r--r-- 0 root root 1382 Jan 22 2020 apache-ratis-2.3.0/dev-support/run-test-repeatedly.sh
-rw-r--r-- 0 root root 1345 Jan 22 2020 apache-ratis-2.3.0/dev-support/vagrant/bin/start_ratis_load_gen.sh
-rw-r--r-- 0 root root 1066 Jan 22 2020 apache-ratis-2.3.0/dev-support/vagrant/bin/start_ratis_server.sh
-rw-r--r-- 0 root root 2509 Jan 22 2020 apache-ratis-2.3.0/dev-support/vagrant/run_all_tests.sh
-rw-r--r-- 0 root root 1594 Jan 22 2020 apache-ratis-2.3.0/dev-support/yetus-personality.sh
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #711:
URL: https://github.com/apache/ratis/pull/711#issuecomment-1212892867
>
Nice, will ask someone else check the permission issuce 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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
leo65535 commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r944209026
##########
ratis-shell/src/main/libexec/ratis-shell-config.sh:
##########
@@ -58,6 +59,16 @@ if [[ ${JAVA_MAJORMINOR} != 001008 && ${JAVA_MAJOR} != 011 ]]; then
exit 1
fi
+local RATIS_SHELL_CLASSPATH
+
+while read -d '' -r jarfile ; do
+ if [[ "$RATIS_SHELL_CLASSPATH" == "" ]]; then
+ RATIS_SHELL_CLASSPATH="$jarfile";
+ else
+ RATIS_SHELL_CLASSPATH="$RATIS_SHELL_CLASSPATH":"$jarfile"
+ fi
+done < <(find "$RATIS_SHELL_LIB_DIR" ! -type d -name '*.jar' -print0 | sort -z)
Review Comment:
hi @szetszwo, I test several commands, `bash` is different from `sh`, their grammar is different.
| command | result |
| ---- | ---- |
| /bin/bash ./bin/ratis | ok |
| /bin/bash ./bin/ratis -e | ok |
| /bin/bash ./bin/ratis sh | ok |
| /bin/sh ./bin/ratis sh | error |
| /bin/sh ./bin/ratis -e | error |
We should use `bash` as the same as the head in script files.
## The scripts somehow do not have execute again.
I checked the assemble file, we alreadly set the fileMode `0755`, and my local evn is ok.
Can you ask someone else check these files again?
```
$ls -l bin/
total 8
-rw-r--r-- 1 szetszwo staff 2152 Jan 22 2020 ratis
$ls -l examples/bin/
total 40
-rw-r--r-- 1 szetszwo staff 1138 Jan 22 2020 client.sh
-rw-r--r-- 1 szetszwo staff 1859 Jan 22 2020 common.sh
-rw-r--r-- 1 szetszwo staff 921 Jan 22 2020 server.sh
-rw-r--r-- 1 szetszwo staff 1598 Jan 22 2020 start-all.sh
-rw-r--r-- 1 szetszwo staff 922 Jan 22 2020 stop-all.sh
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
szetszwo commented on PR #711:
URL: https://github.com/apache/ratis/pull/711#issuecomment-1211557470
> Do we need to exclude it in this patch currently?
Your choice. We may do it here or in the next change.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
leo65535 commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r944209026
##########
ratis-shell/src/main/libexec/ratis-shell-config.sh:
##########
@@ -58,6 +59,16 @@ if [[ ${JAVA_MAJORMINOR} != 001008 && ${JAVA_MAJOR} != 011 ]]; then
exit 1
fi
+local RATIS_SHELL_CLASSPATH
+
+while read -d '' -r jarfile ; do
+ if [[ "$RATIS_SHELL_CLASSPATH" == "" ]]; then
+ RATIS_SHELL_CLASSPATH="$jarfile";
+ else
+ RATIS_SHELL_CLASSPATH="$RATIS_SHELL_CLASSPATH":"$jarfile"
+ fi
+done < <(find "$RATIS_SHELL_LIB_DIR" ! -type d -name '*.jar' -print0 | sort -z)
Review Comment:
hi @szetszwo, I test several commands, `bash` is different from `sh`.
| command | result |
| ---- | ---- |
| /bin/bash ./bin/ratis | ok |
| /bin/bash ./bin/ratis -e | ok |
| /bin/bash ./bin/ratis sh | ok |
| /bin/sh ./bin/ratis sh | error |
| /bin/sh ./bin/ratis -e | error |
We should use `bash` as the same as the head in script files.
## The scripts somehow do not have execute again.
I checked the assemble file, we alreadly set the fileMode `0755`, and my local evn is ok.
Can you ask someone else check these files again?
```
$ls -l bin/
total 8
-rw-r--r-- 1 szetszwo staff 2152 Jan 22 2020 ratis
$ls -l examples/bin/
total 40
-rw-r--r-- 1 szetszwo staff 1138 Jan 22 2020 client.sh
-rw-r--r-- 1 szetszwo staff 1859 Jan 22 2020 common.sh
-rw-r--r-- 1 szetszwo staff 921 Jan 22 2020 server.sh
-rw-r--r-- 1 szetszwo staff 1598 Jan 22 2020 start-all.sh
-rw-r--r-- 1 szetszwo staff 922 Jan 22 2020 stop-all.sh
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r943090453
##########
ratis-shell/pom.xml:
##########
@@ -74,6 +94,8 @@
<exclude>**/org/apache/log4j/jdbc/JDBCAppender.class</exclude>
<exclude>**/org/apache/log4j/net/JMSAppender.class</exclude>
<exclude>**/org/apache/log4j/net/JMSSink.class</exclude>
Review Comment:
Same as before, please remove them.
##########
ratis-examples/pom.xml:
##########
@@ -150,6 +150,8 @@
<exclude>**/org/apache/log4j/jdbc/JDBCAppender.class</exclude>
<exclude>**/org/apache/log4j/net/JMSAppender.class</exclude>
<exclude>**/org/apache/log4j/net/JMSSink.class</exclude>
Review Comment:
Please remove these three lines.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
leo65535 commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r943113419
##########
ratis-examples/pom.xml:
##########
@@ -150,6 +150,8 @@
<exclude>**/org/apache/log4j/jdbc/JDBCAppender.class</exclude>
<exclude>**/org/apache/log4j/net/JMSAppender.class</exclude>
<exclude>**/org/apache/log4j/net/JMSSink.class</exclude>
Review Comment:
Make sense.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r944192133
##########
ratis-shell/src/main/libexec/ratis-shell-config.sh:
##########
@@ -58,6 +59,16 @@ if [[ ${JAVA_MAJORMINOR} != 001008 && ${JAVA_MAJOR} != 011 ]]; then
exit 1
fi
+local RATIS_SHELL_CLASSPATH
+
+while read -d '' -r jarfile ; do
+ if [[ "$RATIS_SHELL_CLASSPATH" == "" ]]; then
+ RATIS_SHELL_CLASSPATH="$jarfile";
+ else
+ RATIS_SHELL_CLASSPATH="$RATIS_SHELL_CLASSPATH":"$jarfile"
+ fi
+done < <(find "$RATIS_SHELL_LIB_DIR" ! -type d -name '*.jar' -print0 | sort -z)
Review Comment:
There is a syntax error:
```shell
$sh ./bin/ratis -e
/Users/szetszwo/Downloads/2.3.0/apache-ratis-3.0.0-SNAPSHOT-bin/bin/../libexec/ratis-shell-config.sh: line 70: syntax error near unexpected token `<'
/Users/szetszwo/Downloads/2.3.0/apache-ratis-3.0.0-SNAPSHOT-bin/bin/../libexec/ratis-shell-config.sh: line 70: `done < <(find "$RATIS_SHELL_LIB_DIR" ! -type d -name '*.jar' -print0 | sort -z)'
Unsupported command -e
Usage: ratis COMMAND [GENERIC_COMMAND_OPTIONS] [COMMAND_ARGS]
COMMAND is one of:
-e sh Command line tool for ratis
GENERIC_COMMAND_OPTIONS supports:
-e -D<property=value> Use a value for a given ratis-shell property
Commands print help when invoked without parameters.
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo merged pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
szetszwo merged PR #711:
URL: https://github.com/apache/ratis/pull/711
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r943046654
##########
pom.xml:
##########
@@ -398,6 +398,11 @@
<scope>test</scope>
<version>1.7.29</version>
</dependency>
+ <dependency>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ <version>1.2.17</version>
+ </dependency>
Review Comment:
Or we may try `slf4j-simple.jar`.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
leo65535 commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r943043823
##########
pom.xml:
##########
@@ -398,6 +398,11 @@
<scope>test</scope>
<version>1.7.29</version>
</dependency>
+ <dependency>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ <version>1.2.17</version>
+ </dependency>
Review Comment:
Make sense, but when we run the command, always output
```
[dcadmin@dcadmin-work apache-ratis-3.0.0-SNAPSHOT-bin]$ bin/ratis sh
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
Usage: ratis sh [generic options]
[election [transfer] [stepDown] [pause] [resume]]
[group [info] [list]]
[peer [add] [remove] [setPriority]]
[snapshot [create]]
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #711:
URL: https://github.com/apache/ratis/pull/711#issuecomment-1211577015
hi @szetszwo, take a final check, keep the `example` in the pkg.
the latest pkg structure in local dev environment.
```
[dcadmin@dcadmin-work apache-ratis-3.0.0-SNAPSHOT-bin]$ ll
total 32
drwxr-xr-x 2 dcadmin dcadmin 19 Jan 22 2020 bin
drwxr-xr-x 2 dcadmin dcadmin 30 Jan 22 2020 conf
drwxrwxr-x 5 dcadmin dcadmin 40 Aug 11 13:48 examples
drwxrwxr-x 2 dcadmin dcadmin 4096 Aug 11 13:48 jars
drwxr-xr-x 2 dcadmin dcadmin 35 Jan 22 2020 libexec
-rw-r--r-- 1 dcadmin dcadmin 13419 Jan 22 2020 LICENSE
-rw-r--r-- 1 dcadmin dcadmin 5246 Jan 22 2020 NOTICE
-rw-r--r-- 1 dcadmin dcadmin 1201 Jan 22 2020 README.md
[dcadmin@dcadmin-work apache-ratis-3.0.0-SNAPSHOT-bin]$ ll bin
total 4
-rwxr-xr-x 1 dcadmin dcadmin 2152 Jan 22 2020 ratis
[dcadmin@dcadmin-work apache-ratis-3.0.0-SNAPSHOT-bin]$ ll examples/bin/
total 20
-rwxr-xr-x 1 dcadmin dcadmin 1138 Jan 22 2020 client.sh
-rwxr-xr-x 1 dcadmin dcadmin 1859 Jan 22 2020 common.sh
-rwxr-xr-x 1 dcadmin dcadmin 921 Jan 22 2020 server.sh
-rwxr-xr-x 1 dcadmin dcadmin 1598 Jan 22 2020 start-all.sh
-rwxr-xr-x 1 dcadmin dcadmin 922 Jan 22 2020 stop-all.sh
[dcadmin@dcadmin-work apache-ratis-3.0.0-SNAPSHOT-bin]$ ll jars/
total 23124
-rw-r--r-- 1 dcadmin dcadmin 58284 Jan 22 2020 commons-cli-1.5.0.jar
-rw-r--r-- 1 dcadmin dcadmin 851531 Jan 22 2020 javassist-3.28.0-GA.jar
-rw-r--r-- 1 dcadmin dcadmin 19936 Jan 22 2020 jsr305-3.0.2.jar
-rw-r--r-- 1 dcadmin dcadmin 116713 Jan 22 2020 ratis-client-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 333476 Jan 22 2020 ratis-common-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 127010 Jan 22 2020 ratis-grpc-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 27006 Jan 22 2020 ratis-metrics-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 111493 Jan 22 2020 ratis-netty-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 1419395 Jan 22 2020 ratis-proto-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 7264 Jan 22 2020 ratis-replicated-map-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 46660 Jan 22 2020 ratis-resource-bundle-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 417972 Jan 22 2020 ratis-server-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 90832 Jan 22 2020 ratis-server-api-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 1119277 Jan 22 2020 ratis-shell-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 6825 Jan 22 2020 ratis-test-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 18685030 Jan 22 2020 ratis-thirdparty-misc-1.0.1.jar
-rw-r--r-- 1 dcadmin dcadmin 11615 Jan 22 2020 ratis-tools-3.0.0-SNAPSHOT.jar
-rw-r--r-- 1 dcadmin dcadmin 130398 Jan 22 2020 reflections-0.10.2.jar
-rw-r--r-- 1 dcadmin dcadmin 41424 Jan 22 2020 slf4j-api-1.7.29.jar
-rw-r--r-- 1 dcadmin dcadmin 15240 Jan 22 2020 slf4j-simple-1.7.29.jar
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
leo65535 commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r944209026
##########
ratis-shell/src/main/libexec/ratis-shell-config.sh:
##########
@@ -58,6 +59,16 @@ if [[ ${JAVA_MAJORMINOR} != 001008 && ${JAVA_MAJOR} != 011 ]]; then
exit 1
fi
+local RATIS_SHELL_CLASSPATH
+
+while read -d '' -r jarfile ; do
+ if [[ "$RATIS_SHELL_CLASSPATH" == "" ]]; then
+ RATIS_SHELL_CLASSPATH="$jarfile";
+ else
+ RATIS_SHELL_CLASSPATH="$RATIS_SHELL_CLASSPATH":"$jarfile"
+ fi
+done < <(find "$RATIS_SHELL_LIB_DIR" ! -type d -name '*.jar' -print0 | sort -z)
Review Comment:
hi @szetszwo, I test several commands, `bash` is different from `sh`.
| command | result |
| ---- | ---- |
| /bin/bash ./bin/ratis | ok |
| /bin/bash ./bin/ratis -e | ok |
| /bin/bash ./bin/ratis sh | ok |
| /bin/sh ./bin/ratis sh | error |
| /bin/sh ./bin/ratis -e | error |
We should use `bash` as the same as the head in script files.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r943024986
##########
pom.xml:
##########
@@ -398,6 +398,11 @@
<scope>test</scope>
<version>1.7.29</version>
</dependency>
+ <dependency>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ <version>1.2.17</version>
+ </dependency>
Review Comment:
Please don't add log4j dependency since it is known to have security risks. Indeed, we should not bundle in our jars. Otherwise, our software also have security risks.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r943046064
##########
pom.xml:
##########
@@ -398,6 +398,11 @@
<scope>test</scope>
<version>1.7.29</version>
</dependency>
+ <dependency>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ <version>1.2.17</version>
+ </dependency>
Review Comment:
@leo65535 , Indeed, ratis-shell should not use log4j at all since it won't print any log message. We should disable it.
We should have `slf4j-nop.jar` as described in https://www.slf4j.org/codes.html#StaticLoggerBinder
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
leo65535 commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r943052486
##########
pom.xml:
##########
@@ -398,6 +398,11 @@
<scope>test</scope>
<version>1.7.29</version>
</dependency>
+ <dependency>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ <version>1.2.17</version>
+ </dependency>
Review Comment:
Got, done.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
szetszwo commented on PR #711:
URL: https://github.com/apache/ratis/pull/711#issuecomment-1213371038
@leo65535 , FYI, the scripts in our 2.3.0 release also do not have execute permission after untar:
```shell
$ls -l apache-ratis-2.3.0/dev-support/checks
total 72
-rw-r--r--@ 1 szetszwo staff 3396 Jan 22 2020 _mvn_unit_report.sh
-rw-r--r--@ 1 szetszwo staff 1189 Jan 22 2020 author.sh
-rw-r--r--@ 1 szetszwo staff 1000 Jan 22 2020 build.sh
-rw-r--r--@ 1 szetszwo staff 1937 Jan 22 2020 checkstyle.sh
-rw-r--r--@ 1 szetszwo staff 1695 Jan 22 2020 findbugs.sh
-rw-r--r--@ 1 szetszwo staff 1268 Jan 22 2020 rat.sh
-rw-r--r--@ 1 szetszwo staff 1384 Jan 22 2020 shellcheck.sh
-rw-r--r--@ 1 szetszwo staff 1197 Jan 22 2020 sonar.sh
-rw-r--r--@ 1 szetszwo staff 1249 Jan 22 2020 unit.sh
```
In the project dir, the original scripts do have execute permission
```shell
$ls -l dev-support/checks
total 72
-rwxr-xr-x 1 szetszwo staff 3396 May 19 08:14 _mvn_unit_report.sh
-rwxr-xr-x 1 szetszwo staff 1189 May 19 08:14 author.sh
-rwxr-xr-x 1 szetszwo staff 1005 May 19 08:14 build.sh
-rwxr-xr-x 1 szetszwo staff 1937 May 19 08:14 checkstyle.sh
-rwxr-xr-x 1 szetszwo staff 1695 May 19 08:14 findbugs.sh
-rwxr-xr-x 1 szetszwo staff 1268 May 19 08:14 rat.sh
-rwxr-xr-x 1 szetszwo staff 1384 May 19 08:14 shellcheck.sh
-rwxr-xr-x 1 szetszwo staff 1197 May 19 08:14 sonar.sh
-rwxr-xr-x 1 szetszwo staff 1249 May 19 08:14 unit.sh
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on a diff in pull request #711: RATIS-1669. Combine shell lib folder and root jars folder
Posted by GitBox <gi...@apache.org>.
leo65535 commented on code in PR #711:
URL: https://github.com/apache/ratis/pull/711#discussion_r944209026
##########
ratis-shell/src/main/libexec/ratis-shell-config.sh:
##########
@@ -58,6 +59,16 @@ if [[ ${JAVA_MAJORMINOR} != 001008 && ${JAVA_MAJOR} != 011 ]]; then
exit 1
fi
+local RATIS_SHELL_CLASSPATH
+
+while read -d '' -r jarfile ; do
+ if [[ "$RATIS_SHELL_CLASSPATH" == "" ]]; then
+ RATIS_SHELL_CLASSPATH="$jarfile";
+ else
+ RATIS_SHELL_CLASSPATH="$RATIS_SHELL_CLASSPATH":"$jarfile"
+ fi
+done < <(find "$RATIS_SHELL_LIB_DIR" ! -type d -name '*.jar' -print0 | sort -z)
Review Comment:
welcome @szetszwo, I test several commands, `bash` is different from `sh`, their grammar is different.
| command | result |
| ---- | ---- |
| /bin/bash ./bin/ratis | ok |
| /bin/bash ./bin/ratis -e | ok |
| /bin/bash ./bin/ratis sh | ok |
| /bin/sh ./bin/ratis sh | error |
| /bin/sh ./bin/ratis -e | error |
We should use `bash` as the same as the head in script files.
## The scripts somehow do not have execute again.
I checked the assemble file, we alreadly set the fileMode `0755`, and my local evn is ok.
Can you ask someone else check these files again?
```
$ls -l bin/
total 8
-rw-r--r-- 1 szetszwo staff 2152 Jan 22 2020 ratis
$ls -l examples/bin/
total 40
-rw-r--r-- 1 szetszwo staff 1138 Jan 22 2020 client.sh
-rw-r--r-- 1 szetszwo staff 1859 Jan 22 2020 common.sh
-rw-r--r-- 1 szetszwo staff 921 Jan 22 2020 server.sh
-rw-r--r-- 1 szetszwo staff 1598 Jan 22 2020 start-all.sh
-rw-r--r-- 1 szetszwo staff 922 Jan 22 2020 stop-all.sh
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org