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

Add $SAGE_LOCAL/bin to LD_LIBRARY_PATH on Cygwin #14380

Closed
jpflori opened this issue Mar 29, 2013 · 15 comments
Closed

Add $SAGE_LOCAL/bin to LD_LIBRARY_PATH on Cygwin #14380

jpflori opened this issue Mar 29, 2013 · 15 comments

Comments

@jpflori
Copy link

jpflori commented Mar 29, 2013

It's needed so that dlopen finds some Sage libraries before system-wide ones.
See #6743 comment:219.

We should also prepend the Cygwin specific stuff to PATH rather than appending it somewhere further in sage-env.

CC: @kcrisman @dimpase

Component: porting: Cygwin

Keywords: cygwin dlopen LD_LIBRARY_PATH

Author: Jean-Pierre Flori

Reviewer: Karl-Dieter Crisman

Merged: sage-5.9.beta3

Issue created by migration from https://trac.sagemath.org/ticket/14380

@jpflori jpflori added this to the sage-5.9 milestone Mar 29, 2013
@dimpase
Copy link
Member

dimpase commented Mar 29, 2013

comment:1

Is it possible to clarify exactly what's going on? That is,

  • if a dll is invoked via dlopen() then is is looked up in LD_LIBRARY_PATH,
  • and if it is invoked otherwise (how???) then in PATH.

@jpflori
Copy link
Author

jpflori commented Mar 29, 2013

comment:2

Not really sure what exactly is going on.

Here is what I seem to have observed:

  • let's say I launch python.exe, then it needs some libraries which have been specified at link time, Cygwin, Windows or whatever will look for them in $PATH (and I think in . first whatever value has $PATH); I guess those libraries are written somewhere in the exe format (PE* or whatever its name is), although there does not seem that encoding RPATHs is possible as with ELF format (but not required there).
  • now from some piece of C code, I use dlopen with just a library name (no absolute path, no slashes in the name), then the implem is the same one as on Linux: look in $LD_LIBRARY_PATH first, and then from what I've experienced, some default pathes including /usr/{bin,lib} and finally $PATH.

So these two library loading system do not look in the same places in the same order.

@jpflori
Copy link
Author

jpflori commented Mar 29, 2013

comment:3

Maybe that putting everything in LD_LIBRARY_PATH would be cleaner and enough, I'll give it a shot tonight.

But the paragraph

The LD_LIBRARY_PATH environment variable is used by the Cygwin function dlopen () as a list of directories to search for .dll files to load. This environment variable is converted from Windows format to UNIX format when a Cygwin process first starts. Most Cygwin applications do not make use of the dlopen () call and do not need this variable. 

from http://cygwin.com/cygwin-ug-net/setup-env.html gives little hope.
I fear setting PATH is also needed.
And so we should also reconsidere prepending stuff to PATH rather than appending so that it comes before system-wide thing.

For our current setup I guess that when Sage's python is launched it loads correctly the Sage's libpython rather than a system wide one because they both are in the very same directory and "." seems to take precedence on all env variables and default pathes.
But that's not to be the case for dlopen...

@nexttime
Copy link
Mannequin

nexttime mannequin commented Mar 29, 2013

comment:4

/join

@jpflori
Copy link
Author

jpflori commented Mar 29, 2013

comment:5

I propose to change in sage-env

if [ "$UNAME" = "CYGWIN" ]; then
    PATH="$PATH:$SAGE_LOCAL/lib:$SAGE_LOCAL/lib/R/lib" && export PATH
fi

to

if [ "$UNAME" = "CYGWIN" ]; then
    # Cygwin needs pathnames in PATH to resolve runtime dependencies
    PATH="$SAGE_LOCAL/lib/R/lib:$SAGE_LOCAL/lib:$PATH" && export PATH
    # And "dlopen" needs them in LD_LIBRARY_PATH, just as on Linuces,
    # except that on Cygwin shared libraries are usually stored in "bin"
    # and not in "lib"
    LD_LIBRARY_PATH="$SAGE_LOCAL/bin:$LD_LIBRARY_PATH" && export PATH
fi

@jpflori
Copy link
Author

jpflori commented Mar 29, 2013

Author: Jean-Pierre Flori

@jpflori
Copy link
Author

jpflori commented Mar 29, 2013

comment:6

Needs some testing now.

Karl-Dieter could you check it resolves your "fork" error?

@nexttime
Copy link
Mannequin

nexttime mannequin commented Mar 29, 2013

comment:7

s/PATH/LD_LIBRARY_PATH/ in the second export.

Also, you should perhaps check LD_LIBRARY_PATH isn't empty, to avoid unintentionally adding ..

@jpflori
Copy link
Author

jpflori commented Mar 29, 2013

comment:8

Attachment: trac_14380.patch.gz

Replying to @nexttime:

s/PATH/LD_LIBRARY_PATH/ in the second export.

Oops.

Also, you should perhaps check LD_LIBRARY_PATH isn't empty, to avoid unintentionally adding ..

I thought about that, but we already explicitely add stuff in LD_LIBRARY_PATH some lines above.
Of course, if that previous part gets removed, then we can mess things up...

@kcrisman
Copy link
Member

comment:9

Karl-Dieter could you check it resolves your "fork" error?

My fork error wasn't about libpython or anything -mtrand.dll. In fact, I don't even have the system-wide Python. A rebase solved it this time, I don't know why it didn't. Or I suppose it could have had something to do with applying this patch.

But this patch makes a lot of sense anyway, so I think we should keep it. Any other PATH experts want to chime in?

@nexttime
Copy link
Mannequin

nexttime mannequin commented Mar 29, 2013

comment:10

Replying to @kcrisman:

Any other PATH experts want to chime in?

You mean !Cygwin/Windows pathologists?

@jpflori
Copy link
Author

jpflori commented Mar 30, 2013

comment:11

I've just check any simpler combination of PATH/LD_LIBRARY_PATH is not sufficient, i.e. we at least need what's in PATH and LD_... with this patch.

@kcrisman
Copy link
Member

comment:12

My XP does have Python installed, and sure enough that seems to have caused the problem.

@kcrisman
Copy link
Member

Reviewer: Karl-Dieter Crisman

@jdemeyer
Copy link

jdemeyer commented Apr 2, 2013

Merged: sage-5.9.beta3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants