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

Cygwin: problem with DLL search order when using system Python #30149

Closed
embray opened this issue Jul 15, 2020 · 16 comments
Closed

Cygwin: problem with DLL search order when using system Python #30149

embray opened this issue Jul 15, 2020 · 16 comments

Comments

@embray
Copy link
Contributor

embray commented Jul 15, 2020

I noticed a problem when trying to run the tests for the latest Sage, that the wrong version of libR.dll was being loaded, because I have R installed via Sage, but I also have the R package for Cygwin installed.

My path starts with:

/home/embray/src/sagemath/sage/local/lib/R/lib:/home/embray/src/sagemath/sage/local/lib:/home/embray/src/sagemath/sage/build/bin:/home/embray/src/sagemath/sage/src/bin:/home/embray/src/sagemath/sage/local/bin:/usr/local/bin:/usr/bin

so when importing rpy2 it should link with the libR.dll that's in $SAGE_LOCAL/lib/R/lib. Normally this has been the case.

But when using the system Python this is not so. This is because according to the standard DLL search path the first place it looks is:

The directory where the executable module for the current process is located.

Well, when running the system Python that's /usr/bin where python3.7m.exe lives. However, that's also where the system's libR.dll lives.

This is just an example, but it's a broader problem: For any DLL linked to by a compiled Python module, it will always privilege the one in /usr/bin over a copy provided by Sage.

What we might have to do is, at least on Cygwin, when creating the venv it should actually copy the Python executable instead of just symlinking to it. I believe venv has an option for this.

CC: @dimpase @mkoeppe

Component: porting: Cygwin

Author: Erik Bray

Branch/Commit: f7213c5

Reviewer: Matthias Koeppe

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

@embray embray added this to the sage-9.2 milestone Jul 15, 2020
@embray
Copy link
Contributor Author

embray commented Jul 15, 2020

comment:1

In build/bin/sage-venv there are some lines:

if os.name == 'nt':
    # default for Windows
    use_symlinks = False
else:
    # default for posix
    # On macOS, definitely need symlinks=True (which matches what we test in build/pkgs/spkg-configure.m4)
    # or it may fail with 'dyld: Library not loaded: @executable_path/../Python3' on macOS.
    use_symlinks = True

But this first check is useless because we currently don't support native Windows. It should say if sys.platform == 'cygwin', or if we really want to be optimistic if sys.platform in ['win32', 'cygwin'].

@mkoeppe
Copy link
Member

mkoeppe commented Jul 15, 2020

comment:2

Thanks for catching this! Are you preparing a branch?

@embray
Copy link
Contributor Author

embray commented Jul 15, 2020

comment:3

This seems to have partially fixed the problem, but only partially. Now there's something really bizarre happening, which is that if I run ./sage -python and do from sage.all import r; r('"abc"') it loads the correct libR.dll. But if I do just ./sage and then r('"abc"') in the Sage shell, it reverts back to the bad behavior.

This shouldn't be, since in both cases it's still running the same Python interpreter executable.

@embray
Copy link
Contributor Author

embray commented Jul 15, 2020

comment:4

I cannot for the life of me find any logical explanation for this discrepancy.

@mkoeppe
Copy link
Member

mkoeppe commented Jul 15, 2020

comment:5

Tried with strace?

@dimpase
Copy link
Member

dimpase commented Jul 15, 2020

comment:6

rpy2 does weird things with loading libR and blas/lapack, they need a correct order of this.

cf rpy2/rpy2#505

@mkoeppe
Copy link
Member

mkoeppe commented Jul 15, 2020

comment:7

Before getting into the depths of what rpy2 may be doing on Cygwin, I'd suggest to try the rpy2 update first - #29441 is waiting for review

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

comment:8

This doesn't have anything to directly with rpy2

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

comment:9

See #30157

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

comment:10

For now I'll create a patch for this issue. It's actually a totally separate issue from #30157.

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

New commits:

f7213c5Trac #30149: When creating the Python venv on Cygwin make sure to copy

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

Branch: u/embray/ticket-30149

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

Commit: f7213c5

@embray
Copy link
Contributor Author

embray commented Jul 16, 2020

Author: Erik Bray

@mkoeppe
Copy link
Member

mkoeppe commented Jul 16, 2020

Reviewer: Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented Jul 28, 2020

Changed branch from u/embray/ticket-30149 to f7213c5

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