Opened 4 years ago

Closed 21 months ago

#9167 closed defect (fixed)

Resolve ecl.dll conflict on Cygwin

Reported by: was Owned by: tbd
Priority: major Milestone: sage-5.7
Component: porting: Cygwin Keywords: cygwin spkg ecl
Cc: mhansen, dimpase, jpflori, jdemeyer Merged in: sage-5.7.beta1
Authors: Jean-Pierre Flori Reviewers: Karl-Dieter Crisman, Jeroen Demeyer
Report Upstream: None of the above - read trac for reasoning. Work issues:
Branch: Commit:
Dependencies: #13324, #13860 Stopgaps:

Description (last modified by jdemeyer)

Though the C-library interface to ECL builds on Cygwin, it does not work at all. All tests fail:

sage: import sage.libs.ecl
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)

/home/wstein/sage-4.4.3/<ipython console> in <module>()

ImportError: No such process
sage: 

The reason of this is a name clash: there are two different libraries called ecl.dll.
One in SAGE_LOCAL/lib/ and one in SAGE_LOCAL/lib/python/site-packages/sage/libs/.
It is the second one whose importing fails because it should be linked to the first one, but cygcheck shows that this dependency is resolved to itself!
This is of course dysfunctional, whence the import failure.

The easiest solution would be to rename sage/libs/ecl.pyx to something else, thus avoiding a name clash.
The solution proposed here is different and more indirect:
Patch ECL build system so that it follows the name convention proposed by Cygwin.
The shared library itself is now in SAGE_LOCAL/bin/cygecl.dll.
In addition, an import file is created in SAGE_LOCAL/lib/libecl.dll.a.

An updated spkg, based on the one in #13324, is available at
http://boxen.math.washington.edu/home/jpflori/ecl-12.12.1.p1.spkg

Attachments (4)

spkg.diff (14.3 KB) - added by jpflori 2 years ago.
Spkg diff, for review only. Not committed in spkg.
spkg-take2.diff (12.9 KB) - added by kcrisman 23 months ago.
For review only - based on JP's spkg
implib.patch (4.9 KB) - added by jpflori 23 months ago.
Patch to upstream build system.
ecl-12.12.1.p1.diff (8.5 KB) - added by jpflori 22 months ago.
Spkg diff, for review only.

Download all attachments as: .zip

Change History (125)

comment:1 Changed 4 years ago by drkirkby

I don't know if it's related, but gcc reports are couple of rather serious looking warning messages with ecl.

/export/home/drkirkby/sage-4.5.1/spkg/build/ecl-10.2.1.p1/src/src/c/dpp.c:678: warning: too few arguments for format
/export/home/drkirkby/sage-4.5.1/spkg/build/ecl-10.2.1.p1/src/src/c/dpp.c:680: warning: too many arguments for format

I'm surprised that gcc does not reject such code and refuse to compile it.

Dave

comment:2 Changed 3 years ago by kcrisman

I can confirm this on the most recent versions of everything at the port wiki page.

That is bad, because now the default Maxima (and hence ECL) for calculus is the library one.

comment:3 Changed 3 years ago by kcrisman

Two possibly irrelevant data points:

  • Tab-completion on from sage.libs.[tab] gives sage.libs.ecl on Mac, not on Cygwin (it also doesn't give .libecm or .ratpoints, though at least ratpoints works).
  • The directory $SAGE_LOCAL/lib/python/site-packages/sage/libs/ DOES include ecl.pyx and ecl.dll, so the library seems to be there, if that is where such imports actually come from (which I'm not sure about).

comment:4 Changed 3 years ago by kcrisman

A possibly useful explanation of something similar from the Cygwin list:

Now that they are in the cygwin dll, libgfortran doesn't need
> to provide them anymore but this has the unfortunate side-effect of breaking
> old executables, since on Windows an imported function reference in an
> executable has to specify not just the function name but also the particular
> DLL from which the import comes.
>
>  I imagine that on ELF platforms where the executable just has a list of
> undefined functions and a list of shared libs to load and the dynamic linker
> just satisfies an undefined symbol from whichever lib it first comes across a
> definition of it, this probably works without anything needing changing.  But
> we're stuck I'm afraid when exports move around like this.

comment:5 Changed 3 years ago by kcrisman

  • Cc mhansen added

This stackoverflow question also might have a useful piece of information. I don't know how to open/read dlls, though, nor exactly how to trace why it is that it's not finding things.

comment:6 follow-up: Changed 3 years ago by kcrisman

Cygwin's cygcheck information was much more helpful, and I find two things.

  • When looking at $SAGE_LOCAL/libs/ecl.dll,
    cygcheck: track_down:  could not find cyggc-1.dll
    
    shows up a lot. I have cyggcc_s-1.dll, cyggcj-*, cyggcr-0.dll and some others, but not this, in /bin/.
  • When looking at devel/sage/build/lib.cygwin.../sage/libs/ecl.dll, I find
    cygcheck: track_down:  could not find csage.dll
    
    This file is in devel/sage/c_lib and local/lib, but maybe that's not enough?

comment:7 in reply to: ↑ 6 Changed 3 years ago by kcrisman

Replying to kcrisman:

Cygwin's cygcheck information was much more helpful, and I find two things.

  • When looking at $SAGE_LOCAL/libs/ecl.dll,
    cygcheck: track_down:  could not find cyggc-1.dll
    
    shows up a lot. I have cyggcc_s-1.dll, cyggcj-*, cyggcr-0.dll and some others, but not this, in /bin/.

Mine, at least, does NOT have cyggc-1.dll anywhere. Apparently it does have libgc (this is a garbage collector) which I've seen connected to cyggc-1.dll on the Cygwin list. But I don't see how I could have libgc without cyggc-1.dll if that were the case... I guess the only thing to try is upgrading my libgc and adding libgc-devel, though I'm a little scared!

comment:8 follow-up: Changed 3 years ago by kcrisman

I can't even figure out where this is created! Unless

cygwin* | mingw* | pw32*)
  version_type=windows
  need_version=no
  need_lib_prefix=no
  case $GCC,$host_os in
  yes,cygwin*)
    library_names_spec='$libname.dll.a'
    soname_spec='`echo ${libname} | sed -e 's/^lib/cyg/'``echo ${release} | sed -e 's/[.]/-/g'`${versuffix}.dll'
    postinstall_cmds='dlpath=`bash 2>&1 -c '\''. $dir/${file}i;echo \$dlname'\''`~
      dldir=$destdir/`dirname \$dlpath`~
      test -d \$dldir || mkdir -p \$dldir~
      $install_prog .libs/$dlname \$dldir/$dlname'
    postuninstall_cmds='dldll=`bash 2>&1 -c '\''. $file; echo \$dlname'\''`~
      dlpath=$dir/\$dldll~
       $rm \$dlpath'

and a few similar things in the configure and aclocal files. I can't quite parse those sed things, though I am pretty sure this wouldn't produce that - but I'm not sure what ${release} would be in this context.

comment:9 in reply to: ↑ 8 Changed 3 years ago by leif

Replying to kcrisman:

...
    soname_spec='`echo ${libname} | sed -e 's/^lib/cyg/'``echo ${release} | sed -e 's/[.]/-/g'`${versuffix}.dll'
...

and a few similar things in the configure and aclocal files.

Where does this come from? ECL?

This might be the cause of the problem, since the sed command also replaces the lib prefix by cyg.

comment:10 Changed 3 years ago by leif

P.S.:

The nm and objdump utilities from the GNU binutils package might be helpful to inspect libraries etc.

I vaguely remember there was also dlltool (perhaps from the MinGW project though).

comment:11 follow-up: Changed 3 years ago by kcrisman

Replying to leif:

Replying to kcrisman:

...
    soname_spec='`echo ${libname} | sed -e 's/^lib/cyg/'``echo ${release} | sed -e 's/[.]/-/g'`${versuffix}.dll'
...

and a few similar things in the configure and aclocal files.

Where does this come from? ECL?

No! This is from the configure file for libgc-6.4.1 or so - the Boehm GC, ported to Cygwin. I have libgc, just not cyggc.

This might be the cause of the problem, since the sed command also replaces the lib prefix by cyg.

Right, that is why I pointed to it. But it doesn't seem to replace the -6.4.1 (or -7.1.1, now) with -1, as far as I could tell. And I didn't have anything like cyggc-* on it.

I'm almost done upgrading and adding libgc-devel on my Cygwin - which meant upgrading basically every single Cygwin package, because this Cygwin hadn't been changed in a year or more. Hopefully won't break anything else, but probably will :(

As to the tools, I think that cygcheck helped a lot, so for now I'm going to stick with that because I actually sort of understand it :)

comment:12 in reply to: ↑ 11 ; follow-up: Changed 3 years ago by kcrisman

Right, that is why I pointed to it. But it doesn't seem to replace the -6.4.1 (or -7.1.1, now) with -1, as far as I could tell. And I didn't have anything like cyggc-* on it.

I'm almost done upgrading and adding libgc-devel on my Cygwin - which meant upgrading basically every single Cygwin package, because this Cygwin hadn't been changed in a year or more. Hopefully won't break anything else, but probably will :(

I think that libgc-devel was what it took to get this file. However, the upgrade upgraded too much - see this Cygwin list thread - so I had to downgrade libgfortran as described there, and gcc, and .... Not for the last time, I have to say that Cygwin definitely is a moving target.

comment:13 in reply to: ↑ 12 Changed 3 years ago by kcrisman

Replying to kcrisman:

Right, that is why I pointed to it. But it doesn't seem to replace the -6.4.1 (or -7.1.1, now) with -1, as far as I could tell. And I didn't have anything like cyggc-* on it.

I'm almost done upgrading and adding libgc-devel on my Cygwin - which meant upgrading basically every single Cygwin package, because this Cygwin hadn't been changed in a year or more. Hopefully won't break anything else, but probably will :(

I think that libgc-devel was what it took to get this file. However, the upgrade upgraded too much - see this Cygwin list thread - so I had to downgrade libgfortran as described there, and gcc, and ...

...and I managed to toast my Cygwin lapack. As far as I can tell I downgraded everything necessary to the right version, rebuilt lapack, rebooted, but still no go.

$ python
>>> import numpy
ImportError <snip>

Nuts. Note that fixing this for the Cygwin Python should fix it for Sage, I think, since we use the same Fortran stuff and even lapack (?), certainly BLAS/ATLAS.

comment:14 follow-up: Changed 3 years ago by kcrisman

After a lot of trouble managing to get Cygwin shell to find lapack again, I definitely have the right files now. We definitely need as at least part of the fix to add libgc-devel as a dependency.

However, there is also still a problem that cygcheck finds all the needed dlls for the ecl.dll in local/lib and local/bin, but not for the ones in devel/sage/build/sage/libs/. It cannot find ecl.dll or csage.dll. Which seems weird, since those files certainly exist.

This might be a PATH problem, judging by some similar issues elsewhere. Unfortunately, ./sage -gdb is no help here, nor is ./sage -ipython.

comment:15 in reply to: ↑ 14 ; follow-up: Changed 3 years ago by kcrisman

However, there is also still a problem that cygcheck finds all the needed dlls for the ecl.dll in local/lib and local/bin, but not for the ones in devel/sage/build/sage/libs/. It cannot find ecl.dll or csage.dll. Which seems weird, since those files certainly exist.

Yeah, after a ./sage -br it still looks like csage.dll is not being found for some reason. Must be a path issue, I think. Where do we set the Sage path? It certainly doesn't include local/lib, but I don't know if that's really the problem.

comment:16 in reply to: ↑ 15 ; follow-up: Changed 3 years ago by leif

Replying to kcrisman:

However, there is also still a problem that cygcheck finds all the needed dlls for the ecl.dll in local/lib and local/bin, but not for the ones in devel/sage/build/sage/libs/. It cannot find ecl.dll or csage.dll. Which seems weird, since those files certainly exist.

Yeah, after a ./sage -br it still looks like csage.dll is not being found for some reason. Must be a path issue, I think. Where do we set the Sage path?

In local/bin/sage-env. You could special-case Cygwin there and add all directories that contain DLLs, as Windows treats them as executables.

It certainly doesn't include local/lib, but I don't know if that's really the problem.

Well, before editing sage-env, you could just try modifying your PATH from the shell accordingly.

comment:17 in reply to: ↑ 16 ; follow-up: Changed 3 years ago by kcrisman

Yeah, after a ./sage -br it still looks like csage.dll is not being found for some reason. Must be a path issue, I think. Where do we set the Sage path?

In local/bin/sage-env. You could special-case Cygwin there and add all directories that contain DLLs, as Windows treats them as executables.

It certainly doesn't include local/lib, but I don't know if that's really the problem.

Well, I have good news and bad news.

Well, before editing sage-env, you could just try modifying your PATH from the shell accordingly.

  • Good news: editing PATH from the shell to include local/lib made cygcheck pass for these files.
  • News: sage-env includes
    if [ "$UNAME" = "CYGWIN" ]; then
        PATH="$PATH:$SAGE_LOCAL/lib" && export PATH
    fi
    
  • Bad news: even with this added to PATH and this being in the sage-env, I still have this problem. I'm still pretty sure it's a path issue, but maybe they are in the wrong order or something? I have no idea how complex this could get...

comment:18 in reply to: ↑ 17 ; follow-up: Changed 3 years ago by kcrisman

  • News: sage-env includes
if [ "$UNAME" = "CYGWIN" ]; then
    PATH="$PATH:$SAGE_LOCAL/lib" && export PATH
fi
  • Bad news: even with this added to PATH and this being in the sage-env, I still have this problem. I'm still pretty sure

For some reason this is not actually in Sage's path, gotten by sys.path in the Sage session. And adding it to sys.path didn't help, either. What the heck is going on?

comment:19 Changed 3 years ago by kcrisman

Another possibility, suggested by this thread, is that there could be two of some file making things hard. Interestingly, there are several ecl.dll's floating around (everywhere a libecl.{dylib,so} would live, I guess) and cygcheck gives different dependencies for each.

comment:20 in reply to: ↑ 18 ; follow-up: Changed 3 years ago by leif

Replying to kcrisman:

  • News: sage-env includes
if [ "$UNAME" = "CYGWIN" ]; then
    PATH="$PATH:$SAGE_LOCAL/lib" && export PATH
fi

Yep, but this should IMHO be moved up in the file (including the definition of UNAME), in any case above

if [ "$1" = "-short" ]; then
    return 0
fi

and $SAGE_LOCAL/lib should be prepended to pick up Sage's versions first.

For some reason this is not actually in Sage's path, gotten by sys.path in the Sage session. And adding it to sys.path didn't help, either. What the heck is going on?

No idea. What does os.environ.get("PATH") give?

I also don't know if we have to add some more or different things to PYTHONPATH on Cygwin.

Note that e.g. $SAGE_LOCAL/lib/python (which is added to PYTHONPATH [if it is a directory] in sage-env) is a symbolic link on Linux etc. (pointing to $SAGE_LOCAL/lib/python2.6); I don't know if it is the real directory on Cygwin instead.

comment:21 in reply to: ↑ 20 Changed 3 years ago by kcrisman

and $SAGE_LOCAL/lib should be prepended to pick up Sage's versions first.

That sounds like a good idea. But should it be before $SAGE_ROOT and $SAGE_LOCAL/bin?

For some reason this is not actually in Sage's path, gotten by sys.path in the Sage session. And adding it to sys.path didn't help, either. What the heck is going on?

No idea. What does os.environ.get("PATH") give?

This gives what I would expect - Sage root, Sage local bin, usr/bin, some Cygwin stuff, a Lapack thing I had to add due to my carelessness, and then Sage local lib.

I also don't know if we have to add some more or different things to PYTHONPATH on Cygwin.

This is very sparse. It is just the directory below.

Note that e.g. $SAGE_LOCAL/lib/python (which is added to PYTHONPATH [if it is a directory] in sage-env) is a symbolic link on Linux etc. (pointing to $SAGE_LOCAL/lib/python2.6); I don't know if it is the real directory on Cygwin instead.

It looks like it's the same on Cygwin.

Another interesting thing is that there are libntl.dll files in /local/bin and /local/lib. Moving just one doesn't seem to do much - note that the bin one is the one imported usually, as in your comment above. Furthermore, apparently only the ecl.dll in devel/sage/build/sage/libs needs libntl.dll and cyggmp, while the one in local/{bin,lib} just wants cyggmp-3.dll.

Anyway, I guess I can move files around all day but I'm not getting any nearer.

comment:22 follow-up: Changed 3 years ago by kcrisman

  • Cc dimpase added

Question for Dima or others; is it possible that we have too many copies of the dlls? Either that the ecl.dll files are in too many places - local/lib and local/bin - or that the extra libntl.dll files also are causing problems? See #11635 for where this started - perhaps this is the cause of all the trouble?

comment:23 in reply to: ↑ 22 ; follow-up: Changed 3 years ago by dimpase

Replying to kcrisman:

Question for Dima or others; is it possible that we have too many copies of the dlls? Either that the ecl.dll files are in too many places - local/lib and local/bin - or that the extra libntl.dll files also are causing problems? See #11635 for where this started - perhaps this is the cause of all the trouble?

for the record, 2 different copies of ecl.dll in SAGE_ROOT/local/ are created; one in local/lib (and/or local/bin), created from ecl spkg, the other in local/lib/python2.6/site-packages/sage/libs/, which contains Sage/Python? interface to ecl (I don't know details about how and when it is built).

comment:24 in reply to: ↑ 23 Changed 3 years ago by kcrisman

for the record, 2 different copies of ecl.dll in SAGE_ROOT/local/ are created; one in local/lib (and/or local/bin), created from ecl spkg, the other in local/lib/python2.6/site-packages/sage/libs/, which contains Sage/Python? interface to ecl (I don't know details about how and when it is built).

Right, and the third one is the one which which yields "No such process".

Although just by chance I tried (in ./sage -ipython)

from sage.matrix import matrix_integer_dense_hnf
NameError: ZZ

from the import from sage.libs.ntl.ntl_ZZ even though

from sage.libs.ntl import *
ntl_ZZ

works fine. Still going to take a while to track all this down, sigh...

What do you think about the possibility that it's just a path problem suggested above? I just don't know enough about how all this works to be sure.

comment:25 Changed 3 years ago by kcrisman

Here's something interesting.

User 1@GC02635 /home/SageUser/sage-4.7.2
$ cygcheck local/bin/ecl.dll
C:\cygwin\home\SageUser\sage-4.7.2\local\bin\ecl.dll
  C:\cygwin\bin\cyggc-1.dll
    C:\cygwin\bin\cygwin1.dll
      C:\WINDOWS\system32\ADVAPI32.DLL
        C:\WINDOWS\system32\KERNEL32.dll
          C:\WINDOWS\system32\ntdll.dll
        C:\WINDOWS\system32\RPCRT4.dll
          C:\WINDOWS\system32\Secur32.dll
    C:\cygwin\bin\cyggcc_s-1.dll
  C:\cygwin\bin\cyggmp-3.dll
  C:\cygwin\bin\cygffi-4.dll

User 1@GC02635 /home/SageUser/sage-4.7.2
$ cygcheck local/bin/ecl.exe
C:\cygwin\home\SageUser\sage-4.7.2\local\bin\ecl.exe
  C:\cygwin\bin\cygwin1.dll
    C:\WINDOWS\system32\ADVAPI32.DLL
      C:\WINDOWS\system32\KERNEL32.dll
        C:\WINDOWS\system32\ntdll.dll
      C:\WINDOWS\system32\RPCRT4.dll
        C:\WINDOWS\system32\Secur32.dll
cygcheck: track_down: could not find ecl.dll

Or maybe it's boring. At any rate, I find this weird.

And here is the cygcheck for the offending dll.

User 1@GC02635 /home/SageUser/sage-4.7.2/local/lib/python2.6/site-packages/sage/libs
$ cygcheck ./ecl.dll
C:\cygwin\home\SageUser\sage-4.7.2\devel\sage-main\build\sage\libs\ecl.dll
  C:\cygwin\bin\cygwin1.dll
    C:\WINDOWS\system32\ADVAPI32.DLL
      C:\WINDOWS\system32\KERNEL32.dll
        C:\WINDOWS\system32\ntdll.dll
      C:\WINDOWS\system32\RPCRT4.dll
        C:\WINDOWS\system32\Secur32.dll
  C:\cygwin\home\SageUser\sage-4.7.2\devel\sage-main\build\sage\libs\ecl.dll
cygcheck: track_down: could not find libpython2.6.dll

cygcheck: track_down: could not find libpython2.6.dll

cygcheck: track_down: could not find csage.dll

cygcheck: track_down: could not find csage.dll

That's a lot of things not to find.

comment:26 follow-up: Changed 3 years ago by kcrisman

Scratch that - in private comm. Dima points out that we should be in the subshell.

SAGE_ROOT=/home/SageUser/sage-4.7.2
(sage subshell) GC02635:sage-4.7.2 User 1$ cd local/lib/python2.6/site-packages/sage/libs/
SAGE_ROOT=/home/SageUser/sage-4.7.2
(sage subshell) GC02635:libs User 1$ cygcheck ./ecl.dll
C:\cygwin\home\SageUser\sage-4.7.2\devel\sage-main\build\sage\libs\ecl.dll
  C:\cygwin\home\SageUser\sage-4.7.2\local\bin\libpython2.6.dll
    C:\cygwin\bin\cygwin1.dll
      C:\WINDOWS\system32\ADVAPI32.DLL
        C:\WINDOWS\system32\KERNEL32.dll
          C:\WINDOWS\system32\ntdll.dll
        C:\WINDOWS\system32\RPCRT4.dll
          C:\WINDOWS\system32\Secur32.dll
  C:\cygwin\home\SageUser\sage-4.7.2\devel\sage-main\build\sage\libs\ecl.dll
    C:\cygwin\home\SageUser\sage-4.7.2\local\lib\csage.dll
      C:\cygwin\home\SageUser\sage-4.7.2\local\bin\cyggmp-3.dll
      C:\cygwin\bin\cyggcc_s-1.dll
      C:\cygwin\bin\cygstdc++-6.dll
      C:\cygwin\home\SageUser\sage-4.7.2\local\lib\libntl.dll
SAGE_ROOT=/home/SageUser/sage-4.7.2

But doing it outside its own directory yields the same "could not find" message as before. It only finds it if I'm already in site-packages/sage/.

comment:27 in reply to: ↑ 26 Changed 3 years ago by dimpase

Replying to kcrisman:

Scratch that - in private comm. Dima points out that we should be in the subshell.

still, we need to know details of Sage/Python? extension implementation on Cygwin to solve this. It could be just an artefact of broken Python/Cython? (fork() failures when running sage -b are a pain...)

comment:28 Changed 2 years ago by jpflori

  • Cc jpflori added

comment:29 Changed 2 years ago by jpflori

Just for info, on my Cygwin 1.7.16/Sage 5.2 install cygchecking all the ecl.* things shows no problem.
So the problem looks more subtle...

comment:30 Changed 2 years ago by jpflori

I found it quite strange that the problematic ecl.dll links to itself.
Maybe it's wanting the other ecl.dll, but gets itself because of the name clash.

I'll try to rename ecl.pyx to ecl_blah.pyx, regenerate it and import it to see what happens.

comment:31 Changed 2 years ago by jpflori

No problem importing ecl_blah (after removing the former ecl.dll in the same directory sage/libs/)!

comment:32 follow-up: Changed 2 years ago by jpflori

And the question is now, why do we get ecl.dll in local/lib/ rather than libecl.dll.

comment:33 in reply to: ↑ 32 ; follow-up: Changed 2 years ago by dimpase

Replying to jpflori:

And the question is now, why do we get ecl.dll in local/lib/ rather than libecl.dll.

the following Cygwin doc seems to imply that lib prefix is merely a matter of convenience; it recommends that there should be no libecl.dll; it should rather be cygecl.dll and libecl.dll.a.

comment:34 Changed 2 years ago by jpflori

Ok, that's because in ecl build system, the shared library is named
${SHAREDPREFIX}ecl.${SHAREDEXT}
and these two variables are set to and 'dll' by configure on Cygwin.

comment:35 in reply to: ↑ 33 Changed 2 years ago by jpflori

Replying to dimpase:

Replying to jpflori:

And the question is now, why do we get ecl.dll in local/lib/ rather than libecl.dll.

the following Cygwin doc seems to imply that lib prefix is merely a matter of convenience; it recommends that there should be no libecl.dll; it should rather be cygecl.dll and libecl.dll.a.

I agree with you.
The question is now if it's trivial enough to modify the build of the shared library on Cygwin.
Unfortunately, ecl does not use libtool.

comment:36 Changed 2 years ago by jpflori

I think the changes could be quite easy, I'll give it a try.

comment:37 follow-up: Changed 2 years ago by jpflori

I based my patch on the two following naming schemes:
http://www.mingw.org/wiki/sampleDLL for MinGW
and:
http://cygwin.com/cygwin-ug-net/dll.html#dll-build for CYGWIN
I'll report that upstream.

As I had to run autoconf which modified a lot of data in config*, the patch included and the hg history will be quite big, so it would be better to update to an upstream release including such modifications (if upstream thinks its a good idea of course).
Unfortunately, we are quite behind, and I'm not really ready to do both an update and having this fix at the same time.
Another solution would be to included a patched src directory in the spkg, but I can already hear people ranting.
Or directly patch configure in a minimal way, but I'm not so inclined to doing so.

comment:38 Changed 2 years ago by jpflori

  • Authors set to Jean-Pierre Flori
  • Dependencies set to #13324
  • Description modified (diff)
  • Keywords cygwin spkg ecl added
  • Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.
  • Status changed from new to needs_review

comment:39 in reply to: ↑ 37 ; follow-up: Changed 2 years ago by leif

Replying to jpflori:

As I had to run autoconf which modified a lot of data in config*, the patch included and the hg history will be quite big ...

You may try to use the exact same versions of automake and autoconf... ;-)

Another solution would be to included a patched src directory in the spkg, but I can already hear people ranting.

Well, I'm personally ok with just "autoreconfing" src/, but such disturbes the move to git.

Or directly patch configure in a minimal way, but I'm not so inclined to doing so.

If the patch isn't that large, that's perhaps the best solution until upstream includes a fix. If they quickly include it into some devel version, we could try to upgrade to that.

comment:40 in reply to: ↑ 39 Changed 2 years ago by jpflori

Replying to leif:

If the patch isn't that large, that's perhaps the best solution until upstream includes a fix. If they quickly include it into some devel version, we could try to upgrade to that.

I agree with both these ideas, see also https://groups.google.com/d/topic/sage-devel/Ikwfh6PXJnQ/discussion .

The problem with patching configure directly is that it needs more dirty work and I'm lazy to do it.

comment:41 Changed 2 years ago by jpflori

And the problem with the upstream solution is that upstream is quite far away from our version and that would also need much more work (if things break, which can be expected).

comment:42 Changed 2 years ago by jpflori

  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.

comment:43 follow-up: Changed 2 years ago by dimpase

  • Status changed from needs_review to needs_work

the new spkg installs OK, but gives the following linking error during the build of various Sage dlls:

...
gcc -I/usr/include/ncurses -fno-strict-aliasing -fwrapv -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I/usr/local/src/sage/sage-5.2/local/include/ecl/ -I/usr/local/src/sage/sage-5.2/local/include -I/usr/local/src/sage/sage-5.2/local/include/csage -I/usr/local/src/sage/sage-5.2/devel/sage/sage/ext -I/usr/local/src/sage/sage-5.2/local/include/python2.7 -c sage/libs/ecl.c -o build/temp.cygwin-1.7.16-i686-2.7/sage/libs/ecl.o -w
gcc -shared -Wl,--enable-auto-image-base -L/usr/local/src/sage/sage-5.2/local/lib build/temp.cygwin-1.7.16-i686-2.7/sage/libs/ecl.o -L/usr/local/src/sage/sage-5.2/local/lib -L/usr/local/src/sage/sage-5.2/local/lib/python2.7/config -lcsage -lecl -lgmp -lstdc++ -lntl -lpython2.7 -o build/lib.cygwin-1.7.16-i686-2.7/sage/libs/ecl.dll
build/temp.cygwin-1.7.16-i686-2.7/sage/libs/ecl.o: In function `__pyx_pf_4sage_4libs_3ecl_4shutdown_ecl':
/usr/local/src/sage/sage-5.2/devel/sage/sage/libs/ecl.c:3134: undefined reference to `_cl_shutdown'
build/temp.cygwin-1.7.16-i686-2.7/sage/libs/ecl.o: In function `__pyx_pf_4sage_4libs_3ecl_9EclObject_12__hash__':
/usr/local/src/sage/sage-5.2/devel/sage/sage/libs/ecl.c:4906: undefined reference to `_cl_sxhash'
.....and lots of siimilar errors...........

comment:44 in reply to: ↑ 43 Changed 2 years ago by dimpase

Replying to dimpase:

the new spkg installs OK, but gives the following linking error during the build of various Sage dlls:

please aslo see Redhat's docs for more details on linking on Cygwin. It seems that we might misunderstand the need for and the purpose of libecl.dll.a

comment:45 Changed 2 years ago by jpflori

The purpose of libecl.dll.a is to be able to link to Cygwin dlls from outside of the Cygwin world, with the Windows linker for example.
So for Sage on Cygwin, this is not needed.

The point here was to use a more generic installation for ECL, which would also work for linking from outside Cygwin, and which would as a side effect solves the name clash problem of this ticket.

The link problem is myabe something with that we do not -L$SAGE_LOCAL/bin when building/linking?
Libraries generated with libtool do not have this problem because libtool also creates a .la file, which is just a text file containing some info, and in particular where to look for the real shared library file, which is usually in ../cyg<libname>-<version info>.dll

comment:46 Changed 2 years ago by jpflori

What's strange is that copying back bin/cygecl.dll to lib/[lib]ecl.dll does not solve the problem.
And that bin/ecl.exe which links to cygecl.dll works fine.

I may have broken something while tweaking the Makefile...

comment:47 follow-up: Changed 2 years ago by jpflori

Running nm on both libraries from this spkg and the previous one shows the same symbols exactly, but with a different address base.

comment:48 in reply to: ↑ 47 Changed 2 years ago by dimpase

Replying to jpflori:

Running nm on both libraries from this spkg and the previous one shows the same symbols exactly, but with a different address base.

the culprit is the new libecl.dll.a, which gets picked up by the linker first, what which is not what is needed (as this is for other purposes).
And this makes perfect sense, that's exactly what happens according to the Redhat docs cited above.

I have overcome this by moving libecl.dll.a out of SAGELOCAL/lib, and creating there a symbolic link named cygecl.dll
to ../bin/cygecl.dll

Hopefully will get Sage that can at least start up some time soon...

comment:49 follow-up: Changed 2 years ago by jpflori

I don't agree with the point that libecl.dll.a cshould not be picked up by the linker.
It is not necessary and you can directly link to cygecl.dll, that I agree with, but it should be possible to go through libecl.dll.a as well.
Of course, there might be a problem with the produced libecl.dll.a, but using import files should not be impossible.

Or then I don't understand how any piece of Sage can link with MPIR, MPFR and any other library which uses libtool and which generates as well import files and put them into SAGE_LOCAL/lib/ where they get picked up before anything else.
And I don't think we have any piece of code which makes sure these dll.a files are not used.

comment:50 in reply to: ↑ 49 Changed 2 years ago by dimpase

Replying to jpflori:

I don't agree with the point that libecl.dll.a cshould not be picked up by the linker.
It is not necessary and you can directly link to cygecl.dll, that I agree with, but it should be possible to go through libecl.dll.a as well.

Redhat docs recommend direct linking over the import library linking, as more efficient.
Further than that, no Idea. I don't have much (positive :-)) experience with Win32 dlls.

Of course, there might be a problem with the produced libecl.dll.a,

this could well be the case, e.g. __declspec(dllexport) declarations missing in the source when compiling.
(and there are arcane rules about using -export-all-symbols and the above declarations at the same time)

Or some options to the linker are wrong/missing?

but using import files should not be impossible.
Or then I don't understand how any piece of Sage can link with MPIR, MPFR and any other library which uses libtool and which generates as well import files and put them into SAGE_LOCAL/lib/ where they get picked up before anything else.

note that there are also these .la files produced along the way in the case the libtool is used.

And I don't think we have any piece of code which makes sure these dll.a files are not used.

comment:51 Changed 2 years ago by jpflori

I don't think the .la files are used by ld at all, but I might be wrong.

The dllexport thing might be the problem, I'll check that.

comment:52 Changed 2 years ago by jpflori

The dll.a file is indeed broken.
Running nm on it should return much more symbols.

Maybe the dll[import|export] magic is broken.

For the dllimport this is sure.
In config.h, the dllimport maic is turned on if cygwin is defined, whereas gcc on cygwin defines _ _ CYGWIN _ _.

For the dllexport magic, I'm not sure.
During the build process there is indeed a cygwin variable defined which triggers the use of dllexport (or seems to do).

Last edited 2 years ago by jpflori (previous) (diff)

comment:53 Changed 2 years ago by jpflori

And I'm able to generate a correct dll.a file using nm and dlltool.
So the linking command which generates the dll.a file must be problematic.

comment:54 Changed 2 years ago by jpflori

Ok, think I got it.

I've put the implid stuff into LDFLAGS which are used several times, so libecl.dll.a keeps being regenerated which various stuff in it.

comment:55 Changed 2 years ago by jpflori

  • Status changed from needs_work to needs_review

The spkg should be fixed now.

Nothing is committed yet, but please give it a try!

comment:56 follow-up: Changed 2 years ago by dimpase

well, I have finished the previous build, but then Sage was refusing to start, throwing the usual fork() problems, even after repeated rebasealls and reboots. I've decided to update Cygwin and uninstall as much as possible of unneeded parts of Cygwin, in hope that it will help. But I am not optimistic.

comment:57 Changed 2 years ago by jpflori

  • Status changed from needs_review to needs_work
  • Work issues set to configure regeneration history issue

comment:58 Changed 2 years ago by jpflori

See discussion at https://groups.google.com/d/topic/sage-devel/Ikwfh6PXJnQ/discussion for autotools regeneration issue.

comment:59 in reply to: ↑ 56 Changed 2 years ago by dimpase

Replying to dimpase:

well, I have finished the previous build, but then Sage was refusing to start, throwing the usual fork() problems, even after repeated rebasealls and reboots. I've decided to update Cygwin and uninstall as much as possible of unneeded parts of Cygwin, in hope that it will help. But I am not optimistic.

no, still no luck. I start to suspect that 2GB of RAM are not enough on 32-bit Windows 7 to run Sage. When I examine the location of Sage dlls which produce these fork() failures, I see that

their preferred base addresses (set up by rebaseall) have nothing to do with the actual places they are allocated;

these actual places may be already used by Win32 system dlls.

It could also be that it's just the 32-bit system is to blame, not the relatively low by modern standards amount of RAM. It's pathetic that on Linux 0.5GB of RAM are enough to have a well-running Sage, while here 2GB are not enough.

comment:60 Changed 2 years ago by jpflori

  • Report Upstream changed from Reported upstream. No feedback yet. to None of the above - read trac for reasoning.
  • Status changed from needs_work to needs_review
  • Work issues configure regeneration history issue deleted

I've upped an upgraded spkg based on 12.7.1.p0 spkg from #13324 at
http://perso.telecom-paristech.fr/~flori/sage/ecl-12.7.1.p1.spkg
using the autotools stuff from #13357.

Nothing committed or tagged yet, nor retested on Cygwin.

Upstream is ready to include a similar patch if it does not break anything on any Windows port... which will need some more thorough testing before I can submit such a patch.
As far as Sage on Cygwin is concerned, everything seems fine (as much as possible...) so far.

comment:61 Changed 2 years ago by jpflori

  • Description modified (diff)

comment:62 Changed 2 years ago by jpflori

  • Status changed from needs_review to needs_work

I lost part of my changes during the update to 12.7.1...

comment:63 Changed 2 years ago by jpflori

  • Status changed from needs_work to needs_review

Should be fixed now, patched aclocal.m4 is back and patches are correctly tracked.

Changed 2 years ago by jpflori

Spkg diff, for review only. Not committed in spkg.

comment:64 Changed 2 years ago by jpflori

  • Description modified (diff)

comment:65 Changed 2 years ago by dimpase

I asked our sysadmins to get me a 64-bit Win7 system (VM), so that I have a chance to get a working Sage on Cygwin. It will take few days to get it up and properly running.

comment:66 Changed 2 years ago by kcrisman

This works on Cygwin on XP! Great solution - I was just not quite knowledgeable about this stuff to help, though I spent far too long on it.

I won't have time to look at the spkg itself until Monday, but definitely positive review from actual practice!

comment:67 Changed 2 years ago by jpflori

Thanx for testing this, but anyway I don't think the package here should get merge, let's just wait for upstream to include something similar (which means action from my side, but should happen in a finite amount of time, we first have to deal with other strange things happening with ECL last stable version potentially ignoring signals).

The spkg here is nonetheless useful to get a somehow more useful build on Cygwin!

comment:68 Changed 23 months ago by jpflori

The issue is now discussed upstream at http://sourceforge.net/p/ecls/feature-requests/15/ (after some exchanges on the ecl-devel list).

The spkg here are outdated (although functional) and should be rebased on top of #13324.

The bug mentioned reported here:
http://sourceforge.net/p/ecls/bugs/222/
should as well be integrated here or at #13324.

comment:69 Changed 23 months ago by kcrisman

This is only a little bit off from being based on #13324, cosmetic stuff mostly.

comment:70 follow-up: Changed 23 months ago by kcrisman

Not quite ready for prime-time, but this spkg with the above diff. This is just naively porting JP's diff. Somehow I mixed up something, and also there is the problem with the weird tarring... but an attempt, anyway.

Last edited 23 months ago by kcrisman (previous) (diff)

Changed 23 months ago by kcrisman

For review only - based on JP's spkg

comment:71 in reply to: ↑ 70 Changed 23 months ago by kcrisman

Not quite ready for prime-time, but this spkg with the above diff. This is just naively porting JP's diff. Somehow I mixed up something, and also there is the problem with the weird tarring... but an attempt, anyway.

Okay, I see what I did wrong. I still have the weird unknown extended keyword thing while untarring, but at least the patches apply now! Waiting on the build, hopefully all will be well on Cygwin.

comment:72 Changed 23 months ago by kcrisman

  • Status changed from needs_review to needs_work

After rebuilding the Maxima package, I get weird things on Mac. They pretty much all look like this.

File "/Users/.../sage-5.4.rc2/devel/sage-main/sage/functions/piecewise.py", line 396:
    sage: f.integral(definite=True)
Exception raised:
<snip>
        ecl_eval("(require 'maxima)")
      File "ecl.pyx", line 1236, in sage.libs.ecl.ecl_eval (sage/libs/ecl.c:6990)
      File "ecl.pyx", line 1251, in sage.libs.ecl.ecl_eval (sage/libs/ecl.c:6927)
      File "ecl.pyx", line 257, in sage.libs.ecl.ecl_safe_eval (sage/libs/ecl.c:2805)
    RuntimeError: ECL says: Detected access to an invalid or protected memory address.

So definitely not ready. JP, did I miss something obvious?

comment:73 follow-up: Changed 23 months ago by kcrisman

Eh, and it didn't even work on Cygwin for some reason (that is, same import errors). I'm going to try JP's again.

Last edited 23 months ago by kcrisman (previous) (diff)

comment:74 in reply to: ↑ 73 Changed 23 months ago by kcrisman

Eh, and it didn't even work on Cygwin for some reason (that is, same import errors). I'm going to try JP's again.

And now I get the same bad behavior (Mac and Cygwin). I have a feeling that previous ECL stuff isn't properly destroyed when one does sage -f ecl-x.y.z.spkg. But whatever.

comment:75 follow-up: Changed 23 months ago by jpflori

That's strange, I guess you've rebuilt Maxima as well.

comment:76 in reply to: ↑ 75 Changed 23 months ago by kcrisman

That's strange, I guess you've rebuilt Maxima as well.

Yes. I think there were just leftover pieces around, I don't know how. I think it's due to my own bad creation of the spkg.

So if you can provide an update to this spkg based on #13324, incorporating the same essential changes in the Cygwin-only case - no need to do anything fancy, just the same stuff - I would love to get that in. The spkg you provided here earlier does still work on Cygwin, just checked a brand-new 5.5.rc0 build.

Last edited 23 months ago by kcrisman (previous) (diff)

comment:77 Changed 23 months ago by jpflori

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Summary changed from cygwin: importing sage.libs.ecl yields a "no such process" error to Resolve ecl.dll conflict on Cygwin

Changed 23 months ago by jpflori

Patch to upstream build system.

comment:78 Changed 23 months ago by jpflori

  • Status changed from needs_review to needs_work
  • Work issues set to use matching autoconf

I guess I should have used a matching version of autoconf to generate a smaller diff.

comment:79 Changed 23 months ago by kcrisman

XP report:

I almost forgot to ./sage -b! But it does fix the problem, based off of the p0 spkg. Yes!

And before rebuilding Maxima I get the usual "don't know how to require Maxima" problem, but afterward, with the spkg from #13364, all is well. Yay! So all is well as far as I'm concerned.

comment:80 follow-up: Changed 23 months ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman

By the way, could this be upgraded irrespective of upgrading Maxima? (I mean for Sage proper.) If all we need is to change the location of the Maxima library, maybe we could avoid having to track down the doctest issues in #13364 right now, by providing a minimal Maxima spkg change here.

comment:81 in reply to: ↑ 80 ; follow-ups: Changed 23 months ago by leif

Replying to kcrisman:

By the way, could this be upgraded irrespective of upgrading Maxima? (I mean for Sage proper.) If all we need is to change the location of the Maxima library, maybe we could avoid having to track down the doctest issues in #13364 right now, by providing a minimal Maxima spkg change here.

To ease things, I'd then not hard-code the (new) name, but test whether maxima.system.fasb is present, and if so, copy that, otherwise (try to) copy maxima.fasb, or bail out if none of these is present.

(Something we should probably do at #13364 as well.)

comment:82 in reply to: ↑ 81 Changed 23 months ago by dimpase

Replying to leif:

Replying to kcrisman:

By the way, could this be upgraded irrespective of upgrading Maxima? (I mean for Sage proper.) If all we need is to change the location of the Maxima library, maybe we could avoid having to track down the doctest issues in #13364 right now, by providing a minimal Maxima spkg change here.

indeed, this can and should be done.

To ease things, I'd then not hard-code the (new) name, but test whether maxima.system.fasb is present, and if so, copy that, otherwise (try to) copy maxima.fasb, or bail out if none of these is present.

good idea!

(Something we should probably do at #13364 as well.)

comment:83 follow-up: Changed 23 months ago by jpflori

Feel free to do so in another ticket, change dependencies here, update #13364 to be based on this new ticket, and remove dependency on this ECL ticket form #13364.

comment:84 in reply to: ↑ 83 Changed 23 months ago by dimpase

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Summary changed from Resolve ecl.dll conflict on Cygwin to Update ECL and resolve ecl.dll conflict on Cygwin

Replying to jpflori:

Feel free to do so in another ticket, change dependencies here, update #13364 to be based on this new ticket, and remove dependency on this ECL ticket form #13364.

I'd rather see this ticket brought to completion. I imagine it's just a small autoconf fix, right?
I'm adding to the ticket description a link to updated maxima 5.26 spkg.

Can one close #13324 as a duplicate?

comment:85 Changed 23 months ago by dimpase

  • Status changed from needs_review to needs_info

comment:86 follow-ups: Changed 23 months ago by jpflori

I think that #13324 still needs_review and potential firther changes (no feedback from upstream yet on the uncaught segfault issue).

Closing #13324 would be similar to what I wanted to do for #12115 and #13137 except for the fact that #12115 was kind of trivial and got merged into #13137, whereas here we would merge a big (almost functional?) ticket into a simpler (but potentially more controversial) one.

Furthermore, as this one is only needed on Cygwin, I'm even more inclined to think it might be better to wait for #13324 to be properly merged, and then go with this one.
If you really want to use it, just download it and do so.

And I still think the change to Maxima spkg-install script should be done in an independent (and quite trivial) ticket called something like "let Maxima spkg properly install with different version of ECL", it would be merged really quicly, and then rebase #13364 to update Maxima on top of that new ticket.

With this approach people could just drop in different versions of ECl and ply with them.

Generally I feel that keeping issues, or even "independent" changes, into separated tickets is a good idea.

comment:87 Changed 23 months ago by jpflori

Yeah, and Jeroen ranted when I asked to close #12115 and merge it within #13137 and I understand his point :)

comment:88 in reply to: ↑ 86 Changed 23 months ago by dimpase

Replying to jpflori:

I think that #13324 still needs_review and potential firther changes (no feedback from upstream yet on the uncaught segfault issue).

Closing #13324 would be similar to what I wanted to do for #12115 and #13137 except for the fact that #12115 was kind of trivial and got merged into #13137, whereas here we would merge a big (almost functional?) ticket into a simpler (but potentially more controversial) one.

#13324 is partially duplicating #13364. The segfault one sees with Maxima is due to an infinite recursion in some Maxima code, as
is acknowledged on maxima bug tracker, bug 2520. I am not sure ECL can be blamed for crashing on this.

comment:89 Changed 23 months ago by jpflori

As was demonstrated, other Lisp interpreters catch the error more gracefully.
I must say I don't really care, especially that when Maxima is fixed, there should be nothing to catch anymore, but if a fix is possible and is devised, we could include it in #13324 or a further ECL update where it belongs.

comment:90 in reply to: ↑ 86 Changed 23 months ago by leif

Replying to jpflori:

And I still think the change to Maxima spkg-install script should be done in an independent (and quite trivial) ticket called something like "let Maxima spkg properly install with different version of ECL", it would be merged really quicly, and then rebase #13364 to update Maxima on top of that new ticket.

Yep, that's what I was thinking of as well. (Unless the Maxima guys [and probably we, too] are super-quick and fix all issues within the next few days...)

With this approach people could just drop in different versions of ECl and ply with them.

Although I occasionally hear the contrary (e.g. "Why let LCalc build with different versions of PARI?"), we shouldn't establish unnecessary dependencies, so yes.


Generally I feel that keeping issues, or even "independent" changes, into separated tickets is a good idea.

Generally. Unfortunately often N developers touch the "same" code (or spkgs) such that at least N-1 developers have to rebase their changes MN times, or the fixes just rotten and never get merged.

Last edited 23 months ago by leif (previous) (diff)

comment:91 in reply to: ↑ 81 ; follow-up: Changed 22 months ago by dimpase

Replying to leif:

Replying to kcrisman:

By the way, could this be upgraded irrespective of upgrading Maxima? (I mean for Sage proper.) If all we need is to change the location of the Maxima library, maybe we could avoid having to track down the doctest issues in #13364 right now, by providing a minimal Maxima spkg change here.

To ease things, I'd then not hard-code the (new) name, but test whether maxima.system.fasb is present, and if so, copy that, otherwise (try to) copy maxima.fasb, or bail out if none of these is present.

OK, this is now #13860

Please review!

comment:92 Changed 22 months ago by leif

  • Dependencies changed from #13324 to #13324 #13860
  • Description modified (diff)

comment:93 in reply to: ↑ 91 ; follow-up: Changed 22 months ago by kcrisman

OK, this is now #13860

Please review!

Apparently that has positive review now. JP, do we really need to generate a smaller patch with "use matching autoconf", or is that unnecessary? This ticket is currently "needs info" but it isn't clear to me what info is needed. Is it really sage-pending on #13324, or is there something else we're trying to get feedback on? Thanks.

comment:94 in reply to: ↑ 93 ; follow-up: Changed 22 months ago by jpflori

Replying to kcrisman:

OK, this is now #13860

Please review!

Apparently that has positive review now. JP, do we really need to generate a smaller patch with "use matching autoconf", or is that unnecessary? This ticket is currently "needs info" but it isn't clear to me what info is needed. Is it really sage-pending on #13324, or is there something else we're trying to get feedback on? Thanks.

I'd like to get upstream feedback on the import library changes and include an official patch rather mine.
I guess we'll have to wait for the end of the holidays for that :)

comment:95 follow-up: Changed 22 months ago by jpflori

And inbetween we can already get #13324 merged...

comment:96 in reply to: ↑ 95 Changed 22 months ago by kcrisman

And inbetween we can already get #13324 merged...

And maybe even #13364...

comment:97 Changed 22 months ago by kcrisman

  • Summary changed from Update ECL and resolve ecl.dll conflict on Cygwin to Resolve ecl.dll conflict on Cygwin

comment:98 Changed 22 months ago by kcrisman

  • Description modified (diff)

comment:99 in reply to: ↑ 94 Changed 22 months ago by kcrisman

  • Status changed from needs_info to needs_review

I'd like to get upstream feedback on the import library changes and include an official patch rather mine.
I guess we'll have to wait for the end of the holidays for that :)

My holidays are done :) And it does look like there has been some activity on the ECL list again. But I think that's not important and we can all take a break, because...

Really I think there is no reason to wait on upstream; Sage does contribute upstream, but we still take care of our own. Is there anything else you would want to change here? even the whole "generate smaller patch" thing seems less significant. As far as I'm concerned, the current spkg is fine especially since the "dependencies" in Maxima and ECL are in/positive review.

Last edited 22 months ago by kcrisman (previous) (diff)

comment:100 Changed 22 months ago by jpflori

That's fair, I'll check the spkg is clean cause I don't really remember right now and report back here so someone motivated can put this as positive review (I guess you could as this is really only Cygwin (and MinGW) related and should have no effect on any other system).

comment:101 follow-up: Changed 22 months ago by jpflori

Just a reminder for me, I should add a simple hack to take care of http://sourceforge.net/p/ecls/bugs/222/ in order to produce cleaner libs as well.

comment:102 in reply to: ↑ 101 Changed 22 months ago by kcrisman

Just a reminder for me, I should add a simple hack to take care of http://sourceforge.net/p/ecls/bugs/222/ in order to produce cleaner libs as well.

Maybe that can wait for upstream; I don't think it's necessary here and I don't know if I'll have the chance to test things as much now.

comment:103 follow-up: Changed 22 months ago by jpflori

  • Cc jdemeyer added

Ok, I've looked back at the spkg and it looks clean in fact.
As you suggested I won't include additional (and not absolutely necessary) fixes.

So if you feel inclined, please put this as positive review.
The main points I guess are to:

  • check it works as expected on Cygwin (and makes something somehow sensible),
  • look at the patch to make sure it does not do anything on other platforms,
  • check the spkg is clean.

If #13324 goes in in a previous release, I guess we'll have to properly rebase it because of hg history and tags?
But I feel it is not necessary if this ticket and #13324 get merged at once in 5.7, is that right?
What would be your strategy Jeroen?

comment:104 in reply to: ↑ 103 Changed 22 months ago by jdemeyer

Replying to jpflori:

What would be your strategy Jeroen?

The plan is certainly to merge #13324 in sage-5.7.beta0. Of course, unexpected problems can always appear, but so far the new ECL has passed all buildbot tests.

I don't quite understand how this ticket relates to #13324, as that is also about Cygwin...

comment:105 Changed 22 months ago by jpflori

#13324 was only about building ecl on Cygwin.
This ticket is about resolving a conflict between the main ecl library being called ecl.dll and the one for the ecl interface in Sage having the same name.
Indeed Windows cannot deal will that: the latter one should link to the former one but at runtime it resolves this dependency by linking back to itself because of the name collision...

comment:106 Changed 22 months ago by jdemeyer

  • Dependencies changed from #13324 #13860 to #13324, #13860
  • Description modified (diff)

comment:107 Changed 22 months ago by jdemeyer

Ideally, the hg history would be preserved so I prefer a proper rebase to #13324. But let's wait until sage-5.7.beta0 is released.

In any case, this ticket still needs to be reviewed (without worrying about rebasing for now).

comment:108 Changed 22 months ago by jdemeyer

Concerning "autoconf": the autotools optional package should solve that problem. Install that package in some Sage version (this doesn't have to be Cygwin), run a Sage shell and then run autoconf/automake/autowhatever from within that Sage shell.

comment:109 Changed 22 months ago by jpflori

  • Status changed from needs_review to needs_work

Yeah I'm aware of that... and you just made me realize that's what I originally intended to do and forgot to do this morning although it's written evreywhere in this ticket.

comment:110 Changed 22 months ago by jpflori

  • Status changed from needs_work to needs_review
  • Work issues use matching autoconf deleted

Done (I'm aware about the not changed date in SPKG.txt but whatever).

comment:111 Changed 22 months ago by jdemeyer

Beware with timestamps when patching configure!

You must ensure that the timestamp of configure is more recent than that of the various .in files, otherwise make might want to rerun autoconf, leading to potential failures.

One solution is to edit the patch file such that configure comes last, as patch processes files in order. Or manually touch configure afterwards.

comment:112 Changed 22 months ago by jpflori

True indeed, doing that now, sorry for being that lame on this one.

comment:113 Changed 22 months ago by jpflori

Should be ok now (I've even taken the time to actually try to build it (on Linux)).

Changed 22 months ago by jpflori

Spkg diff, for review only.

comment:114 Changed 22 months ago by kcrisman

Grmph, the train's wifi cut me off...

Short version: is there anything actually different from the previous spkg that did work? I also can't see any configure changes in the patch, maybe that's a good thing.

comment:115 Changed 21 months ago by kcrisman

Okay, all relevant tests pass with this (and a rebuilt Maxima) on Mac. I say positive review, assuming JP clarifies my dumb comment about configure and he confirms that he fixed the timestamp issue (I think he is saying that he did in comment:112).

comment:116 Changed 21 months ago by jpflori

The implib.patch I posted here only touches the autotools file (you then need to somehow regenerate the build system, let's say autoreconf -i).

I've done this for the spkg, so the one included in the spkg (and in the diff posted here) is the same plus the changes to the build system (after runing autoreconf -i using matching versions of autotools).

And I've indeed reordered the spkg's patch hunks so that autotools files are patched before the build system one to ensure that autotools does not decide to regenerate everything on the fly.

comment:117 Changed 21 months ago by kcrisman

  • Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Jeroen Demeyer
  • Status changed from needs_review to positive_review

I can no longer check this due to horrible BLODA issues but my comment:115 stands, and if Jeroen agrees that these are stamped in the correct order, then let's get this in, since it does work.

comment:118 Changed 21 months ago by kcrisman

Is it possible one might get a doctest error on this? I did a weird upgrade where I already had this ticket, from 5.6.rc0 to 5.6, and then got this.

File "/Users/karl.crisman/Downloads/sage-5.6/devel/sage/sage/libs/ecl.pyx", line 247:
    sage: inf_loop()
Expected:
    Traceback (most recent call last):
    ...
    RuntimeError: ECL says: Console interrupt
Got:
    Traceback (most recent call last):
      File "/Users/karl.crisman/Downloads/sage-5.6/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/Users/karl.crisman/Downloads/sage-5.6/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/Users/karl.crisman/Downloads/sage-5.6/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_6[7]>", line 1, in <module>
        inf_loop()###line 247:
    sage: inf_loop()
      File "ecl.pyx", line 704, in sage.libs.ecl.EclObject.__call__ (sage/libs/ecl.c:5038)
      File "ecl.pyx", line 280, in sage.libs.ecl.ecl_safe_apply (sage/libs/ecl.c:3049)
    RuntimeError: ECL says: Console interrupt.

Note the one-character difference (on Mac, anyway). Can someone check this? I think it might be due to my having done the upgrade and thus not important.

comment:119 Changed 21 months ago by jpflori

That's expected, the first patch from http://trac.sagemath.org/sage_trac/ticket/13324 should fix that.

comment:120 Changed 21 months ago by kcrisman

Hmm, that sounds familiar. Great.

comment:121 Changed 21 months ago by jdemeyer

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