You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/07/01 12:36:15 UTC
[GitHub] [pulsar] nicoloboschi opened a new pull request, #16332: [improve][cli] Pulsar shell: include pulsar-shell in released tarballs
nicoloboschi opened a new pull request, #16332:
URL: https://github.com/apache/pulsar/pull/16332
Master Issue: #16250
### Motivation
See master issue under "Distribution" section.
### Changes
New tarball `pulsar-shell` under `distribution` directory. It generates a tarball with:
- `pulsar-client-tools` dependency (and transitive)
- log dependencies
- README + NOTICE as other packages
- bin/pulsar-shell and bin/pulsar-admin-common.sh scripts
- conf/client.conf for default configuration
- [x] `doc-required`
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #16332: [improve][cli] Pulsar shell: include pulsar-shell in released tarballs - part 2
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #16332:
URL: https://github.com/apache/pulsar/pull/16332#discussion_r921836009
##########
bin/pulsar-shell:
##########
@@ -18,7 +18,19 @@
# under the License.
#
-BINDIR=$(dirname "$0")
+# need this for relative symlinks
+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
Review Comment:
@nicoloboschi may I ask a concrete usage of this change? It seems other scripts all use `BINDIR=$(dirname "$0")`.
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #16332: [improve][cli] Pulsar shell: include pulsar-shell in released tarballs - part 2
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #16332:
URL: https://github.com/apache/pulsar/pull/16332#discussion_r922297527
##########
bin/pulsar-shell:
##########
@@ -18,7 +18,19 @@
# under the License.
#
-BINDIR=$(dirname "$0")
+# need this for relative symlinks
+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
Review Comment:
Actually I'm working on providing a root level symbol link to the distribution in our workspace so that developers can easily play with latest Pulsar on their branch. In that scenario other scripts may need to resolve symlink also.
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi merged pull request #16332: [improve][cli] Pulsar shell: include pulsar-shell in released tarballs - part 2
Posted by GitBox <gi...@apache.org>.
nicoloboschi merged PR #16332:
URL: https://github.com/apache/pulsar/pull/16332
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #16332: [improve][cli] Pulsar shell: include pulsar-shell in released tarballs - part 2
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on code in PR #16332:
URL: https://github.com/apache/pulsar/pull/16332#discussion_r922248139
##########
bin/pulsar-shell:
##########
@@ -18,7 +18,19 @@
# under the License.
#
-BINDIR=$(dirname "$0")
+# need this for relative symlinks
+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
Review Comment:
we don't yet. but I already created a draft package for homebrew and it's needed
you can check it out here: https://github.com/nicoloboschi/homebrew-apache-pulsar-shell
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #16332: [improve][cli] Pulsar shell: include pulsar-shell in released tarballs - part 2
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #16332:
URL: https://github.com/apache/pulsar/pull/16332#discussion_r922115354
##########
bin/pulsar-shell:
##########
@@ -18,7 +18,19 @@
# under the License.
#
-BINDIR=$(dirname "$0")
+# need this for relative symlinks
+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
Review Comment:
@nicoloboschi Thanks for your explanation. I get this point.
When do you use pulsar-shell as a symbol link? From this patch we copy the script itself into the tarball:
```
<file>
<outputDirectory>bin</outputDirectory>
<source>${basedir}/../../bin/pulsar-shell</source>
<fileMode>755</fileMode>
</file>
```
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #16332: [improve][cli] Pulsar shell: include pulsar-shell in released tarballs - part 2
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on code in PR #16332:
URL: https://github.com/apache/pulsar/pull/16332#discussion_r921939431
##########
bin/pulsar-shell:
##########
@@ -18,7 +18,19 @@
# under the License.
#
-BINDIR=$(dirname "$0")
+# need this for relative symlinks
+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
Review Comment:
That's a good question.
This is needed when the binary is a symlink otherwise it won't be able to find the actual pulsar home directory.
For example, if you install the shell with homebrew, a symlink is used.
To be honest I didn't invented any here, I just copied it from Apache Maven: https://github.com/apache/maven/blob/f05397a88dc4178fff7ac7d307797cd54db3bef9/apache-maven/src/assembly/shared/mvnvalidate#L1-L13
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #16332: [improve][cli] Pulsar shell: include pulsar-shell in released tarballs - part 2
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #16332:
URL: https://github.com/apache/pulsar/pull/16332#discussion_r922284185
##########
bin/pulsar-shell:
##########
@@ -18,7 +18,19 @@
# under the License.
#
-BINDIR=$(dirname "$0")
+# need this for relative symlinks
+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
Review Comment:
Looks great :)
##########
bin/pulsar-shell:
##########
@@ -18,7 +18,19 @@
# under the License.
#
-BINDIR=$(dirname "$0")
+# need this for relative symlinks
+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
Review Comment:
@nicoloboschi Thanks for your explanation. I get this point.
When do you use pulsar-shell as a symbol link? From this patch we copy the script itself into the tarball:
```xml
<file>
<outputDirectory>bin</outputDirectory>
<source>${basedir}/../../bin/pulsar-shell</source>
<fileMode>755</fileMode>
</file>
```
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #16332: [improve][cli] Pulsar shell: include pulsar-shell in released tarballs - part 2
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #16332:
URL: https://github.com/apache/pulsar/pull/16332#discussion_r922297527
##########
bin/pulsar-shell:
##########
@@ -18,7 +18,19 @@
# under the License.
#
-BINDIR=$(dirname "$0")
+# need this for relative symlinks
+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
Review Comment:
Actually I'm working on providing a root level symbol link to the distribution in our workspace so that developers can easily play with latest Pulsar on their branch. In that scenario other scripts may need to resolve symlink also.
But if we just symlink the parent directory, it seems not need for resolve the script itself. This script works when the script is directly symlinked, doesn't it?
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on pull request #16332: [improve][cli] Pulsar shell: include pulsar-shell in released tarballs - part 2
Posted by GitBox <gi...@apache.org>.
momo-jun commented on PR #16332:
URL: https://github.com/apache/pulsar/pull/16332#issuecomment-1188502570
Hi, @nicoloboschi, will you add or update relevant docs in a follow-up PR?
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org