#30149 closed defect (fixed)

Cygwin: problem with DLL search order when using system Python

Reported by: embray Owned by:
Priority: major Milestone: sage-9.2
Component: porting: Cygwin Keywords:
Cc: dimpase, mkoeppe Merged in:
Authors: Erik Bray Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: f7213c5 (Commits, GitHub, GitLab) Commit: f7213c57463ab5af8441cd8410d26cca6ef95d2f
Dependencies: Stopgaps:

Status badges

Description

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.

Change History (13)

comment:1 Changed 13 months ago by embray

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

Last edited 13 months ago by embray (previous) (diff)

comment:2 Changed 13 months ago by mkoeppe

Thanks for catching this! Are you preparing a branch?

comment:3 Changed 13 months ago by embray

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.

Last edited 13 months ago by embray (previous) (diff)

comment:4 Changed 13 months ago by embray

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

comment:5 Changed 13 months ago by mkoeppe

Tried with strace?

comment:6 Changed 13 months ago by dimpase

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

cf https://github.com/rpy2/rpy2/issues/505

comment:7 Changed 13 months ago by mkoeppe

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

comment:8 Changed 13 months ago by embray

This doesn't have anything to directly with rpy2

comment:9 Changed 13 months ago by embray

See #30157

comment:10 Changed 13 months ago by embray

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

comment:11 Changed 13 months ago by embray

  • Authors set to Erik Bray
  • Branch set to u/embray/ticket-30149
  • Commit set to f7213c57463ab5af8441cd8410d26cca6ef95d2f
  • Status changed from new to needs_review

New commits:

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

comment:12 Changed 13 months ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

comment:13 Changed 12 months ago by vbraun

  • Branch changed from u/embray/ticket-30149 to f7213c57463ab5af8441cd8410d26cca6ef95d2f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.