RATIS-1669. Combine shell lib folder and root jars folder#711
Merged
szetszwo merged 3 commits intoapache:masterfrom Aug 12, 2022
xleoken:shell
Merged
RATIS-1669. Combine shell lib folder and root jars folder#711szetszwo merged 3 commits intoapache:masterfrom xleoken:shell
szetszwo merged 3 commits intoapache:masterfrom
xleoken:shell
Conversation
szetszwo
requested changes
Aug 11, 2022
Contributor
szetszwo
left a comment
szetszwo
left a comment
There was a problem hiding this comment.
@leo65535 , thanks for working on this. We should avoid adding log4j dependency since it is known to have security risks; see https://logging.apache.org/log4j/1.2/
BTW, let's exclude everything from log4j's chainsaw, jdbc and net packages as below.
diff --git a/ratis-examples/pom.xml b/ratis-examples/pom.xml
index ecf5afb6..7dae2e1c 100644
--- a/ratis-examples/pom.xml
+++ b/ratis-examples/pom.xml
@@ -147,9 +147,8 @@
<exclude>META-INF/*.DSA</exclude>
<exclude>META-INF/*.RSA</exclude>
<exclude>**/org/apache/log4j/chainsaw/**</exclude>
- <exclude>**/org/apache/log4j/jdbc/JDBCAppender.class</exclude>
- <exclude>**/org/apache/log4j/net/JMSAppender.class</exclude>
- <exclude>**/org/apache/log4j/net/JMSSink.class</exclude>
+ <exclude>**/org/apache/log4j/jdbc/**</exclude>
+ <exclude>**/org/apache/log4j/net/**</exclude>
</excludes>
</filter>
</filters>
diff --git a/ratis-experiments/pom.xml b/ratis-experiments/pom.xml
index d621fe5e..319b03ef 100644
--- a/ratis-experiments/pom.xml
+++ b/ratis-experiments/pom.xml
@@ -130,9 +130,8 @@
<exclude>META-INF/*.DSA</exclude>
<exclude>META-INF/*.RSA</exclude>
<exclude>**/org/apache/log4j/chainsaw/**</exclude>
- <exclude>**/org/apache/log4j/jdbc/JDBCAppender.class</exclude>
- <exclude>**/org/apache/log4j/net/JMSAppender.class</exclude>
- <exclude>**/org/apache/log4j/net/JMSSink.class</exclude>
+ <exclude>**/org/apache/log4j/jdbc/**</exclude>
+ <exclude>**/org/apache/log4j/net/**</exclude>
</excludes>
</filter>
</filters>
diff --git a/ratis-shell/pom.xml b/ratis-shell/pom.xml
index bc66ba5a..2a14708f 100644
--- a/ratis-shell/pom.xml
+++ b/ratis-shell/pom.xml
@@ -71,9 +71,8 @@
<exclude>META-INF/*.DSA</exclude>
<exclude>META-INF/*.RSA</exclude>
<exclude>**/org/apache/log4j/chainsaw/**</exclude>
- <exclude>**/org/apache/log4j/jdbc/JDBCAppender.class</exclude>
- <exclude>**/org/apache/log4j/net/JMSAppender.class</exclude>
- <exclude>**/org/apache/log4j/net/JMSSink.class</exclude>
+ <exclude>**/org/apache/log4j/jdbc/**</exclude>
+ <exclude>**/org/apache/log4j/net/**</exclude>
</excludes>
</filter>
</filters>
Member
Author
|
hi @szetszwo, take a final check. I forget to exclude the Do we need to exclude it in this patch currently?
|
szetszwo
reviewed
Aug 11, 2022
Contributor
szetszwo
left a comment
szetszwo
left a comment
There was a problem hiding this comment.
@leo65535 , thanks for the update!
- With the current approach, we should have ratis-shell-3.0.0-SNAPSHOT.jar instead of ratis-shell-3.0.0-SNAPSHOT-jar-with-dependencies.jar since there are already dependency jars.
- The script files does not have execute permission
$ls -l ./bin/ratis
-rw-r--r-- 1 szetszwo staff 2152 Jan 22 2020 ./bin/ratis
$ls -l ./examples/bin/*
-rw-r--r-- 1 szetszwo staff 1138 Jan 22 2020 ./examples/bin/client.sh
-rw-r--r-- 1 szetszwo staff 1859 Jan 22 2020 ./examples/bin/common.sh
-rw-r--r-- 1 szetszwo staff 921 Jan 22 2020 ./examples/bin/server.sh
-rw-r--r-- 1 szetszwo staff 1598 Jan 22 2020 ./examples/bin/start-all.sh
-rw-r--r-- 1 szetszwo staff 922 Jan 22 2020 ./examples/bin/stop-all.sh```
Contributor
Your choice. We may do it here or in the next change. |
Member
Author
|
hi @szetszwo, take a final check, keep the the latest pkg structure in local dev environment. |
szetszwo
reviewed
Aug 11, 2022
Contributor
szetszwo
left a comment
szetszwo
left a comment
There was a problem hiding this comment.
@leo65535 , thanks for the update! There are some classes in ratis-shell-3.0.0-SNAPSHOT.jar duplicated with the classes in other jars. Below is an example:
- javassist
apache-ratis-3.0.0-SNAPSHOT-bin$jar -tf jars/javassist-3.28.0-GA.jar | grep javassist | head -10
META-INF/maven/org.javassist/
META-INF/maven/org.javassist/javassist/
META-INF/maven/org.javassist/javassist/pom.properties
META-INF/maven/org.javassist/javassist/pom.xml
javassist/
javassist/ByteArrayClassPath$1.class
javassist/ByteArrayClassPath$BytecodeURLConnection.class
javassist/ByteArrayClassPath$BytecodeURLStreamHandler.class
javassist/ByteArrayClassPath.class
javassist/CannotCompileException.class- shell
apache-ratis-3.0.0-SNAPSHOT-bin$jar -tf jars/ratis-shell-3.0.0-SNAPSHOT.jar | grep javassist | head -10
META-INF/maven/org.javassist/
META-INF/maven/org.javassist/javassist/
META-INF/maven/org.javassist/javassist/pom.properties
META-INF/maven/org.javassist/javassist/pom.xml
javassist/
javassist/ByteArrayClassPath$1.class
javassist/ByteArrayClassPath$BytecodeURLConnection.class
javassist/ByteArrayClassPath$BytecodeURLStreamHandler.class
javassist/ByteArrayClassPath.class
javassist/CannotCompileException.class
Member
Author
Nice catch, I had remove the shade plugin from the shell module, the patch has been updated, thanks. Also checked the |
szetszwo
reviewed
Aug 12, 2022
Contributor
szetszwo
left a comment
szetszwo
left a comment
There was a problem hiding this comment.
@leo65535 , thanks a lot for the update!
- There is a syntax error; see the comment inlined.
- The scripts somehow do not have execute 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| else | ||
| RATIS_SHELL_CLASSPATH="$RATIS_SHELL_CLASSPATH":"$jarfile" | ||
| fi | ||
| done < <(find "$RATIS_SHELL_LIB_DIR" ! -type d -name '*.jar' -print0 | sort -z) |
Contributor
There was a problem hiding this comment.
There is a syntax error:
$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.
Member
Author
There was a problem hiding this 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
Contributor
There was a problem hiding this comment.
@leo65535 , bash worked file. Thanks!
It seems that tar somehow have not preserved the permissions
$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
$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
szetszwo
approved these changes
Aug 12, 2022
Contributor
szetszwo
left a comment
szetszwo
left a comment
There was a problem hiding this comment.
+1 the change looks good.
Let's merge the current change first and then fix the permission problem separately.
Member
Author
|
Nice, will ask someone else check the permission issuce again. |
Contributor
|
@leo65535 , FYI, the scripts in our 2.3.0 release also do not have execute permission after untar: $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.shIn the project dir, the original scripts do have execute permission $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 |
Contributor
|
The script files inside apache-ratis-2.3.0-bin.tar.gz also do not have execute permission $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 |
codings-dan
pushed a commit
that referenced
this pull request
Aug 16, 2022
(cherry picked from commit 8b69f74)
symious
pushed a commit
to symious/ratis
that referenced
this pull request
Mar 6, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
After patch
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1669
How was this patch tested?
run
bin/ratiscommand.