Opened 7 years ago

Closed 7 years ago

#14380 closed defect (fixed)

Add $SAGE_LOCAL/bin to LD_LIBRARY_PATH on Cygwin

Reported by: jpflori Owned by: tbd
Priority: major Milestone: sage-5.9
Component: porting: Cygwin Keywords: cygwin dlopen LD_LIBRARY_PATH
Cc: kcrisman, dimpase Merged in: sage-5.9.beta3
Authors: Jean-Pierre Flori Reviewers: Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

It's needed so that dlopen finds some Sage libraries before system-wide ones. See http://trac.sagemath.org/sage_trac/ticket/6743#comment:219.

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

Attachments (1)

trac_14380.patch (934 bytes) - added by jpflori 7 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 7 years ago by dimpase

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.

comment:2 Changed 7 years ago by jpflori

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.

comment:3 Changed 7 years ago by jpflori

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...

comment:4 Changed 7 years ago by leif

/join

comment:5 Changed 7 years ago by jpflori

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

comment:6 follow-up: Changed 7 years ago by jpflori

  • Authors set to Jean-Pierre Flori
  • Status changed from new to needs_review

Needs some testing now.

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

comment:7 follow-up: Changed 7 years ago by leif

s/PATH/LD_LIBRARY_PATH/ in the second export.

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

Changed 7 years ago by jpflori

comment:8 in reply to: ↑ 7 Changed 7 years ago by jpflori

Replying to leif:

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...

comment:9 in reply to: ↑ 6 ; follow-up: Changed 7 years ago by kcrisman

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?

comment:10 in reply to: ↑ 9 Changed 7 years ago by leif

Replying to kcrisman:

Any other PATH experts want to chime in?

You mean Cygwin/Windows pathologists?

comment:11 Changed 7 years ago by jpflori

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.

comment:12 Changed 7 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman
  • Status changed from needs_review to positive_review

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

comment:13 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.9.beta3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.