Opened 3 years ago

Closed 3 years ago

#28829 closed defect (fixed)

suitesparse does not build on Cygwin

Reported by: Erik Bray Owned by:
Priority: blocker Milestone: sage-9.0
Component: packages: standard Keywords: cygwin windows cvxopt suitesparse
Cc: Dima Pasechnik, François Bissey Merged in:
Authors: Erik Bray Reviewers: François Bissey
Report Upstream: Not yet reported upstream; Will do shortly. Work issues:
Branch: fa68739 (Commits, GitHub, GitLab) Commit: fa68739e619b645501bed7fe34a749fe7762fb47
Dependencies: #28832 Stopgaps:

Status badges

Description

[suitesparse-5.6.0] gcc -L/home/embray/src/sagemath/sage-python3/local/lib -Wl,-rpath,/home/embray/src/sagemath/sage-python3/local/lib  -L/home/embray/src/sagemath/sage-python3/local/lib -L/home/embray/src/sagemath/sage-python3/local/lib -L/home/embray/src/sagemath/sage-python3/local/lib -shared -Wl,-soname -Wl,libamd.so.2 -Wl,--no-undefined amd_i_aat.o amd_i_1.o amd_i_2.o amd_i_dump.o amd_i_postorder.o amd_i_defaults.o amd_i_post_tree.o amd_i_order.o amd_i_control.o amd_i_info.o amd_i_valid.o amd_i_preprocess.o amd_l_aat.o amd_l_1.o amd_l_2.o amd_l_dump.o amd_l_postorder.o amd_l_defaults.o amd_l_post_tree.o amd_l_order.o amd_l_control.o amd_l_info.o amd_l_valid.o amd_l_preprocess.o -o /home/embray/src/sagemath/sage-python3/local/lib/libamd.so.2.4.6 -lm -lsuitesparseconfig
[suitesparse-5.6.0] /usr/lib/gcc/x86_64-pc-cygwin/7.4.0/../../../../x86_64-pc-cygwin/bin/ld: cannot find -lsuitesparseconfig
[suitesparse-5.6.0] collect2: error: ld returned 1 exit status

The description in #22380 indicates that it was included at one point as part of cvxopt, but I never had any problems related to this before.

Change History (12)

comment:1 Changed 3 years ago by Dima Pasechnik

Do you have local/lib/libsuitesparseconfig.so* present?

I also wonder how many times one can safely repeat -L/home/embray/src/sagemath/sage-python3/local/lib in parameters...

comment:2 Changed 3 years ago by Erik Bray

Shew...this build system is a mess...

comment:3 Changed 3 years ago by Dima Pasechnik

from my dim memories about cygwin, sometimes DLLs end up in local/bin rather than in local/bin, etc...

comment:4 Changed 3 years ago by Erik Bray

Dependencies: #28832

comment:5 Changed 3 years ago by Erik Bray

Authors: Erik Bray
Branch: u/embray/ticket-28829
Commit: 0e9580c549ee640339418cae3820cc398a107236
Report Upstream: N/ANot yet reported upstream; Will do shortly.
Status: newneeds_review

This is the most minimal patch I could come up with for now. It required changes in quite a few files for them to install files properly on Cygwin. I didn't touch the libraries that use CMake (and they might already work better), or any other make targets that don't need to work for Sage's purposes.

A better solution would involve a total reworking of the build system.

Rather than patching, I did consider just updating our spkg-install with some Cygwin-specific commands to move things around after the initial faulty installation. That conceivably could have worked, but might be an even bigger and uglier mess. I'll propose my changes, or something like it, to upstream later.

Still need to test that the rest of the Sage build passes.


New commits:

19212a8Trac #28832: Support staged installation of suitesparse using DESTDIR.
0e9580cTrac #28829: Patch suitesparse to support installing correctly on

comment:6 in reply to:  3 Changed 3 years ago by Erik Bray

Replying to dimpase:

from my dim memories about cygwin, sometimes DLLs end up in local/bin rather than in local/bin, etc...

I think you meant "local/lib rather than in local/bin"? :) And yes, that was part of the problem. It actually didn't even properly detect building on Cygwin, and the implementation of doing so (which in fairness was noted in the comments as "untested") was indeed completely broken. It also didn't properly produce import libs for the DLLs, and a number of other odd issues.

Beyond just the Cygwin issues though the whole thing is a bit of a mess :D

comment:7 Changed 3 years ago by git

Commit: 0e9580c549ee640339418cae3820cc398a107236fa68739e619b645501bed7fe34a749fe7762fb47

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

5b54ee7Re-add some LDFLAGS that are needed in suitesparse
8d79590fix install_name on OS X
efeab2fcorrect the final path in install_name
fa68739Trac #28829: Patch suitesparse to support installing correctly on

comment:8 Changed 3 years ago by Erik Bray

Cc: François Bissey added

Rebased to incorporate fbissey's updates to #28832.

comment:9 Changed 3 years ago by François Bissey

Reviewers: François Bissey
Status: needs_reviewpositive_review

We shouldn't need SuiteSparse_GPURuntime or xerbla. One needs a config flag to be run and the other need to be explicitly build as far as I can see. The rest looks OK to me.

Of course extra useless (for sage) patching doesn't hurt. Keeping as close as you would like upstream to take it is definitely OK.

Let's see how it goes.

comment:10 Changed 3 years ago by Erik Bray

Exactly--some of this patching was done mechanically since their sources have many copies of almost identical Makefiles (annoyingly some of them with minor, unnecessary variations). I still intend to submit this patch (at a minimum) upstream, and maybe offer to develop a more thorough refactoring another time (especially if one of his wealthy sponsors would be willing to fund the work :)

comment:11 Changed 3 years ago by Dima Pasechnik

maybe it's more meaningful to use cmake with https://github.com/jlblancoc/suitesparse-metis-for-windows (despite the name, it's cross-platform, and endorsed by the suitesparse author).

comment:12 Changed 3 years ago by Volker Braun

Branch: u/embray/ticket-28829fa68739e619b645501bed7fe34a749fe7762fb47
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.