Patch attached. Tested running a daemon out of the bin dir w/ absolute and relative paths.

Does this work on OSX, BSD, and Solaris? I seem to remember this being proposed before, but killed for compatibility reasons.

Right patch this time.

Does this work on OSX, BSD, and Solaris? I seem to remember this being proposed before, but killed for compatibility reasons.

I checked that -m and -n were supported on Solaris. Looks like from ZOOKEEPER-303 though that OSX doesn't have this option, think the same workaround is acceptable here?

Not even all Linux versions use GNU readlink. The (BSD-derived?) version accepts no options.

...and looks like older versions of GNU readlink don't accept the -m option. This is the --help output from version 5.2.1:

-f, --canonicalize canonicalize by following every symlink in every
component of the given path recursively
-n, --no-newline do not output the trailing newline
-q, --quiet,
-s, --silent suppress most error messages
-v, --verbose report error messages
--help display this help and exit
--version output version information and exit

readlink is definitely tricky from a portability perspective; it is fairly recent as far as the commercial OSes go. [I think it was added in AIX 7, but maybe Stephen or someone else can confirm since I no longer have access to an AIX machine. On Solaris, GNU readlink was added in either 10 or NV, IIRC.]

Also, with the change to use BASH_SOURCE, is cd+pwd still super fragile? It may not get it 100% correct, but does it get it 'correct enough'?

Another option might be to use file -h, but I'm not certain how portable that is either. Since we require bash, there is also the (built-in only!) pwd -P.

Yea, looks like readlink isn't going to work portably. I think Allen's suggestion of using pwd -P is most portable, should work on Linux, BSD, OSX and Solaris, and is much cleaner than the current code. I'll update the patch.

The motivation is that there's a bug in the manual symlink resolution code, it fails when the script is called using a symlink from /, which is how Debian runs init scripts.

/ $ ls -al /home/eli/test2
total 56
drwxr-xr-x   2 eli eli  4096 2011-01-07 10:07 .
drwxr-xr-x 136 eli eli 49152 2011-01-07 10:07 ..
lrwxrwxrwx   1 eli eli    17 2011-01-07 09:58 testl1.sh -> ../test1/test1.sh
lrwxrwxrwx   1 eli eli    17 2011-01-07 10:00 testl2.sh -> ../test1/test2.sh
/ $ cat /home/eli/test1/test1.sh 
#!/bin/bash
set -e
this="${BASH_SOURCE-$0}"
while [ -h "$this" ]; do
  ls=`ls -ld "$this"`
  link=`expr "$ls" : '.*-> \(.*\)$'`
  if expr "$link" : '.*/.*' > /dev/null; then
    this="$link"
  else
    this=`dirname "$this"`/"$link"
  fi
done
bin=`dirname "$this"`
script=`basename "$this"`
bin=`cd "$bin"; pwd`
this="$bin/$script"
echo $this

Called w/o a symlink:

/ $ /home/eli/test1/test1.sh
/home/eli/test1/test1.sh

Called via a symlink:

/ $ /home/eli/test2/testl1.sh 
/home/eli/test2/testl1.sh: line 15: cd: ../test1: No such file or directory
//test1.sh

Re-written to use pwd -P

/ $ cat /home/eli/test2/testl2.sh 
#!/bin/bash
bin=$(cd -P -- "$(dirname -- "${BASH_SOURCE-$0}")" && pwd -P)
script="$(basename -- "${BASH_SOURCE-$0}")"
this="$bin/$script"
echo $this

Called via a symlink:

/ $ /home/eli/test2/testl2.sh 
/home/eli/test2/testl2.sh

Patch attached. Tested running from / via a symlink works and that setting HADOOP_CORE_HOME from HDFS/MR root can start the daemons.

Some extra explanation of the rationale: the current code only resolves the final symlink in the path, ie only starts if the file that sources hadoop-config.sh is a symlink, where the patch here unconditionally resolves all the symlinks. So the current code sometimes does, and sometimes does not, fully resolve links in the path to HADOOP_HOME. Therefore the current code should be agnostic as to whether you fully resolve HADOOP_HOME (ie it should be OK to always resolve the symlinks).

I think it makes sense to resolve links (and this probably was the intent of the original symlink resolution code) as that way the logic is uniform, ie the way HADOOP_HOME is determined is the same regardless of whether hadoop-config.sh is sourced from a symlink.

Eli asked me to double check it. This is a change in behavior, but I think it will be safe as well as portable so long as the bash built-ins are used. It might be worthwhile adding that in the comments that we're depending upon the built-ins in case someone does something like port Hadoop to busybox. [Think appliance.]

So, +1 if some changes are made to the comments.

Thanks for the review Allen.

According to the following Posix includes -P for cd and pwd so I think this change will work even on systems w/o bash built ins. On my linux host both the built-in and pwd binary support -P.

IEEE Std 1003.1, 2004 Edition
http://www.opengroup.org/onlinepubs/000095399/utilities/cd.html
http://www.opengroup.org/onlinepubs/000095399/utilities/pwd.html

Per Allen's suggestion, updated the patch to add a comment indicating -P requires bash built-ins or POSIX:2001 cd and pwd.

Committed to trunk and branch 22. Thanks Allen.

Integrated in Hadoop-Common-22-branch #15 (See https://hudson.apache.org/hudson/job/Hadoop-Common-22-branch/15/)
HADOOP-7089. svn merge -c 1058881 from trunk

Integrated in Hadoop-Common-trunk #576 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/576/)
HADOOP-7089. Fix link resolution logic in hadoop-config.sh. Contributed by Eli Collins

Integrated in Hadoop-Common-trunk-Commit #479 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/479/)