Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KARAF-4564 Can't start karaf using symbolic link #196

Closed
wants to merge 4 commits into from

Conversation

adetalhouet
Copy link

@adetalhouet adetalhouet commented Jun 8, 2016

When executing the karaf script, it gets the DIRNAME based on $0 which is the path used to start the script. This DIRNAME is then used to set the KARAF_HOME and multiple other KARAF_* env variables.
Using a symbolic link, you would have, for instance, usr/bin/karaf redirecting to /opt/opendaylight/bin/karaf. So $0 would be usr/bin and not /opt/opendaylight/bin so the locateHome function isn't setting the right path for the KARAF_HOME.

This ends up failing to start ODL with following
ERROR: Error: Could not find or load main class org.apache.karaf.main.Main

Signed-off-by: Alexis de Talhouët adetalhouet@inocybe.com

Alexis de Talhouët added 2 commits June 8, 2016 17:28
Signed-off-by: Alexis de Talhouët <adetalhouet@inocybe.com>
Signed-off-by: Alexis de Talhouët <adetalhouet@inocybe.com>
@adetalhouet
Copy link
Author

The pull request isn't complete as it should take care of the .bat scripts a well. But I wasn't able to figure it out for the batch script.

@jbonofre
Copy link
Member

jbonofre commented Jun 9, 2016

Unfortunately, I don't think it's a good idea to use realpath as it's not available on most of Unix systems.
For instance, on Debian/Ubuntu, it requires:

$ sudo apt-get install realpath

Maybe readlink is more appropriate ?

@jbonofre
Copy link
Member

jbonofre commented Jun 9, 2016

Even better, we can test if readlink is available on the system, then use it, else fallback to the current behavior.

@adetalhouet
Copy link
Author

Sure, would you like me to test it to make sure it solves the issue the same way?

On Jun 9, 2016, at 9:27 AM, Jean-Baptiste Onofré notifications@github.com wrote:

Unfortunately, I don't think it's a good idea to use realpath as it's not available on most of Unix systems.
For instance, on Debian/Ubuntu, it requires:

$ sudo apt-get install realpath
Maybe readlink is more appropriate ?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #196 (comment), or mute the thread https://github.com/notifications/unsubscribe/ADMIKeLO9sBAwkttmuwfx-HZY3tGIkPoks5qKBTHgaJpZM4IxbLj.

@jbonofre
Copy link
Member

jbonofre commented Jun 9, 2016

That would be great ! Thanks !

@adetalhouet
Copy link
Author

adetalhouet commented Jun 9, 2016

I confirm readlink is working fine with the three environments mentioned in the JIRA

Alexis de Talhouët added 2 commits June 9, 2016 09:39
Signed-off-by: Alexis de Talhouët <adetalhouet@inocybe.com>
@jbonofre
Copy link
Member

As said in my previous comment, even if readlink is available by default on most of system, it's not necessarily the case. I would add a test to check if readlink command is available: if yes, then we use it for basedir, else we "fallback" to the current behavior.

For instance, something like type readlink and check if it returns 0 (readlink is found on the system) or 1 (readlink is not found on the system).

@adetalhouet
Copy link
Author

adetalhouet commented Jun 10, 2016

So would this be more appropriate?

READLINK=`type readlink`
if [ "x$READLINK" = "x" ]; then
    REALNAME=$0
else
    REALNAME=`readlink "$0"`
fi

DIRNAME=`dirname "$REALNAME"`
PROGNAME=`basename "$REALNAME"`

But if readlink doesn't exist it will trigger a warning:
/usr/bin/karaf: line 19: type: readlink: not found

@jbonofre
Copy link
Member

I would do more something like:

REALNAME=`readlink "$0"`
if [ $? != 0 ]; then
  REALNAME=$0
fi

@jbonofre
Copy link
Member

I updated the PR with:

REALNAME=`readlink -e "$0" > /dev/null 2>&1`
if [ $? != 0 ]; then
    REALNAME=$0
fi
DIRNAME=`dirname "$REALNAME"`
PROGNAME=`basename "$REALNAME"`

It works fine. Rebasing and merging.

asfgit pushed a commit that referenced this pull request Jun 12, 2016
@asfgit asfgit closed this in 8228782 Jun 13, 2016
asfgit pushed a commit that referenced this pull request Jun 13, 2016
@adetalhouet adetalhouet deleted the symbolic-link branch June 14, 2016 13:00
@adetalhouet
Copy link
Author

@jbonofre Thank you for such a great support!

@jbonofre
Copy link
Member

@adetalhouet you are welcome ! My pleasure !

@vorburger
Copy link
Member

asfgit pushed a commit that referenced this pull request Aug 22, 2016
frouleau pushed a commit to frouleau/karaf that referenced this pull request Jan 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants