[jira] [Commented] (ZOOKEEPER-2377) zkServer.sh should resolve canonical path from symlinks
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2377?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15177363#comment-15177363
]
Siddhartha commented on ZOOKEEPER-2377:
---
2) Haha yes, left over from my testing sessions, sorry.
4) Good point.
Keeping in mind that this feature did not even exist till now, we can choose to
support one degree symlinks only. In that case, we do not even have to use
readline as ls -l will be sufficient.
Alternatively, if we want to now start supporting multiple symlink traversals,
then we can retain readlink -f usage. Though of course as you pointed out, this
will lead to difference in behaviour across different platforms.
If Zookeeper has a policy to behave strictly the same across all platforms,
then I think we should go with simple one step symlink support only. Otherwise
readline -f is a neat feature to support and I think we should have that. So
your call :)
Implementing our own readlink function sounds overkill to me :P
{quote}
Last but not least, I personally like to append a ".n", where n > 1, just
before the ".patch" suffix. It makes easy to quickly glance over which patch is
the latest one and doesn't break Jenkins. Just a suggestion.
{quote}
Thanks, will follow henceforth. I was just following
https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute, where it
says:
bq. In this case the updated patch should be re-attached to the Jira with the
same name. Jira will archive the older version of the patch and make the new
patch the active patch.
This page should be updated with above instructions :)
Thanks for testing and the review.
> zkServer.sh should resolve canonical path from symlinks
> ---
>
> Key: ZOOKEEPER-2377
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2377
> Project: ZooKeeper
> Issue Type: Improvement
> Components: scripts
>Affects Versions: 3.4.8
> Environment: Centos 6
>Reporter: Siddhartha
>Assignee: Siddhartha
>Priority: Minor
> Attachments: ZOOKEEPER-2377.patch, ZOOKEEPER-2377.patch
>
>
> If zkServer.sh is started from a symlink, it is not able to correctly source
> the other scripts because it looks in the wrong path.
> Attached patch fixes this by first resolving absolute path to the script.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2377) zkServer.sh should resolve canonical path from symlinks
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2377?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15175586#comment-15175586 ] Edward Ribeiro commented on ZOOKEEPER-2377: --- Addedum: it worked with "ls -l" on OSX, as implicit by the answer at 3) > zkServer.sh should resolve canonical path from symlinks > --- > > Key: ZOOKEEPER-2377 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2377 > Project: ZooKeeper > Issue Type: Improvement > Components: scripts >Affects Versions: 3.4.8 > Environment: Centos 6 >Reporter: Siddhartha >Priority: Minor > Attachments: ZOOKEEPER-2377.patch, ZOOKEEPER-2377.patch > > > If zkServer.sh is started from a symlink, it is not able to correctly source > the other scripts because it looks in the wrong path. > Attached patch fixes this by first resolving absolute path to the script. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2377) zkServer.sh should resolve canonical path from symlinks
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2377?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15175579#comment-15175579
]
Edward Ribeiro commented on ZOOKEEPER-2377:
---
Hi [~sdh],
{quote}
Does not look like the failed tests are because of my patch, is it?
{quote}
1. Nope, I don't think so. Both the Findbugs warning and the failed test are
related to SASL and SSL classes that your patch didn't touch at all. Right,
[~rgs]?
2. In your latest patch I've seen the following lines, that I am assuming are
left over debug statements, right? ;)
{code}
echo $ZOOBINDIR
exit 0
{code}
3. Your latest patch worked on OSX (after I removed the code snippet above,
lol). :)
4. I agree with you wrt to the "hack". But, if I understood it right, now we
have two scenarios: on Linux it will follow the chain of symbolic links until
the real file while on OSX/FreeBSD it will stop short on the first scenario.
For example, the following code works on OSX now:
{code}
$ mkdir tmp; cd tmp
[tmp] $ ln -s /Users/edward/IdeaProjects/zookeeper/bin/zkServer.sh zkServer.sh
[tmp] $ ./zkServer.sh start
{code}
But if I add the following commands
{code}
[tmp]$ ln -s ./zkServer.sh zkServer2
[tmp]$ ./zkServer2
{code}
Then it will not on OSX/FreeBSD/OpenBSD/NetBSD, but *it will work on Linux*
because it follows the symlinks chain zkServer2 -> zkServer ->
/Users/edward/IdeaProjects/zookeeper/bin/zkServer.sh, right?
So... My question (and I really don't have an option now) is: this dual
behaviour is desirable? Would it not be better to only follow the first symlink
and fail it is has further symlink??? [~rgs], [~cnauroth]?
Last but not least, I personally like to append a ".n", where n > 1, just
before the ".patch" suffix. It makes easy to quickly glance over which patch is
the latest one and doesn't break Jenkins. Just a suggestion.
Cheers!
> zkServer.sh should resolve canonical path from symlinks
> ---
>
> Key: ZOOKEEPER-2377
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2377
> Project: ZooKeeper
> Issue Type: Improvement
> Components: scripts
>Affects Versions: 3.4.8
> Environment: Centos 6
>Reporter: Siddhartha
>Priority: Minor
> Attachments: ZOOKEEPER-2377.patch, ZOOKEEPER-2377.patch
>
>
> If zkServer.sh is started from a symlink, it is not able to correctly source
> the other scripts because it looks in the wrong path.
> Attached patch fixes this by first resolving absolute path to the script.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2377) zkServer.sh should resolve canonical path from symlinks
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2377?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15175329#comment-15175329 ] Siddhartha commented on ZOOKEEPER-2377: --- Does not look like the failed tests are because of my patch, is it? > zkServer.sh should resolve canonical path from symlinks > --- > > Key: ZOOKEEPER-2377 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2377 > Project: ZooKeeper > Issue Type: Improvement > Components: scripts >Affects Versions: 3.4.8 > Environment: Centos 6 >Reporter: Siddhartha >Priority: Minor > Attachments: ZOOKEEPER-2377.patch, ZOOKEEPER-2377.patch > > > If zkServer.sh is started from a symlink, it is not able to correctly source > the other scripts because it looks in the wrong path. > Attached patch fixes this by first resolving absolute path to the script. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2377) zkServer.sh should resolve canonical path from symlinks
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2377?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15175315#comment-15175315 ] Hadoop QA commented on ZOOKEEPER-2377: -- -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12790890/ZOOKEEPER-2377.patch against trunk revision 1733221. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3056//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3056//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3056//console This message is automatically generated. > zkServer.sh should resolve canonical path from symlinks > --- > > Key: ZOOKEEPER-2377 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2377 > Project: ZooKeeper > Issue Type: Improvement > Components: scripts >Affects Versions: 3.4.8 > Environment: Centos 6 >Reporter: Siddhartha >Priority: Minor > Attachments: ZOOKEEPER-2377.patch, ZOOKEEPER-2377.patch > > > If zkServer.sh is started from a symlink, it is not able to correctly source > the other scripts because it looks in the wrong path. > Attached patch fixes this by first resolving absolute path to the script. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2377) zkServer.sh should resolve canonical path from symlinks
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2377?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15174732#comment-15174732
]
Edward Ribeiro commented on ZOOKEEPER-2377:
---
Hey, [~rgs], thanks for inviting me to check. :)
LGTM. One annoyance is that MacOS doesn't recognize the "-e" switch, so I had
to remove it to make the patch to run on my box, as below:
{code}
- ZOOBIN=$(readlink -e "$ZOOBIN" 2>/dev/null)
+ ZOOBIN=$(readlink "$ZOOBIN" 2>/dev/null)
{code}
As far as I could grasp, the "-e" resolves all the symbolic links recursively
until the target, but this option is not available on OSX, because it includes
the BSD version of readlink while Linux includes the GNU version. Following
this logic, this would make this patch fail on BSD-like systems too (that I
don't have installed here, yet).
The man pages are below:
https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man1/readlink.1.html
http://linux.die.net/man/1/readlink
[~sdh], IMO, we can either remove this switch (if it's safe to assume that ZK
dir will be only one level deep into symlinks) or use the "-e" switch only on
Linux systems. If you choose the latter then it would require to use some code
snippet as below to identify the OS (or the use of "$OSTYPE" env variable) and
maybe some "hack" as the link below (see the "rreadlink" function):
{code}
uname -a | awk '{print $1}'
{code}
https://sourceware.org/ml/crossgcc/2009-11/msg00108.html
Raul, Siddhartha and Chris, wdyt?
> zkServer.sh should resolve canonical path from symlinks
> ---
>
> Key: ZOOKEEPER-2377
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2377
> Project: ZooKeeper
> Issue Type: Improvement
> Components: scripts
>Affects Versions: 3.4.8
> Environment: Centos 6
>Reporter: Siddhartha
>Priority: Minor
> Attachments: ZOOKEEPER-2377.patch
>
>
> If zkServer.sh is started from a symlink, it is not able to correctly source
> the other scripts because it looks in the wrong path.
> Attached patch fixes this by first resolving absolute path to the script.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2377) zkServer.sh should resolve canonical path from symlinks
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2377?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15171408#comment-15171408 ] Raul Gutierrez Segales commented on ZOOKEEPER-2377: --- Thanks for the patch [~sdh]! Lgtm, +1. [~cnauroth], [~eribeiro]: could I get some additional eyes? Would this work for all platforms? > zkServer.sh should resolve canonical path from symlinks > --- > > Key: ZOOKEEPER-2377 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2377 > Project: ZooKeeper > Issue Type: Improvement > Components: scripts >Affects Versions: 3.4.8 > Environment: Centos 6 >Reporter: Siddhartha >Priority: Minor > Attachments: ZOOKEEPER-2377.patch > > > If zkServer.sh is started from a symlink, it is not able to correctly source > the other scripts because it looks in the wrong path. > Attached patch fixes this by first resolving absolute path to the script. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2377) zkServer.sh should resolve canonical path from symlinks
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2377?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15168462#comment-15168462 ] Hadoop QA commented on ZOOKEEPER-2377: -- -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12790057/ZOOKEEPER-2377.patch against trunk revision 1729259. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3050//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3050//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3050//console This message is automatically generated. > zkServer.sh should resolve canonical path from symlinks > --- > > Key: ZOOKEEPER-2377 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2377 > Project: ZooKeeper > Issue Type: Improvement > Components: scripts >Affects Versions: 3.4.8 > Environment: Centos 6 >Reporter: Siddhartha >Priority: Minor > Attachments: ZOOKEEPER-2377.patch > > > If zkServer.sh is started from a symlink, it is not able to correctly source > the other scripts because it looks in the wrong path. > Attached patch fixes this by first resolving absolute path to the script. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
