#28829 closed defect (fixed)

suitesparse does not build on Cygwin

Reported by: embray Owned by:
Priority: blocker Milestone: sage-9.0
Component: packages: standard Keywords: cygwin windows cvxopt suitesparse
Cc: dimpase, fbissey 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 22 months ago by dimpase

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 22 months ago by embray

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

comment:3 follow-up: Changed 22 months ago by dimpase

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

comment:4 Changed 22 months ago by embray

  • Dependencies set to #28832

comment:5 Changed 22 months ago by embray

  • Authors set to Erik Bray
  • Branch set to u/embray/ticket-28829
  • Commit set to 0e9580c549ee640339418cae3820cc398a107236
  • Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.
  • Status changed from new to needs_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 22 months ago by embray

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 22 months ago by git

  • Commit changed from 0e9580c549ee640339418cae3820cc398a107236 to fa68739e619b645501bed7fe34a749fe7762fb47

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 22 months ago by embray

  • Cc fbissey added

Rebased to incorporate fbissey's updates to #28832.

comment:9 Changed 22 months ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_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 22 months ago by embray

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 22 months ago by dimpase

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 22 months ago by vbraun

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