Opened 3 years ago

Closed 3 years ago

#28832 closed defect (fixed)

suitesparse spkg does not properly use a DESTDIR installation

Reported by: Erik Bray Owned by:
Priority: critical Milestone: sage-9.0
Component: packages: standard Keywords: suitesparse cvxopt
Cc: Dima Pasechnik Merged in:
Authors: Erik Bray, François Bissey Reviewers: François Bissey, Erik Bray
Report Upstream: N/A Work issues:
Branch: efeab2f (Commits, GitHub, GitLab) Commit: efeab2f799c7d4d6b2d89e110b9ddaffc77b2712
Dependencies: Stopgaps:

Status badges

Description

This SPKG was introduced in #22380 as a standard package.

However, it does not properly use the the facilities for DESTDIR installation, and thus a file manifest is not produced for it:

$ cat local/var/lib/sage/installed/suitesparse-5.6.0
{
    "package_name": "suitesparse",
    "package_version": "5.6.0",
    "install_date": "Mon Dec  2 11:44:16 UTC 2019",
    "system_uname": "Linux sage-docker 3.13.0-170-generic #220-Ubuntu SMP Thu May 9 12:40:49 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux",
    "sage_version": "SageMath version 9.0.beta8, Release Date: 2019-12-01",
    "test_result": "",
    "files": [

    ]
}

suitesparse apparently has a custom build system (parts of it contain sub-libraries built with CMake (GraphBLAS, Mongoose) and hence should support DESTDIR but according to the SPKG.txt we don't install those components anyways.

We should have better documentation, for adding new SPKGs, about ensuring that DESTDIR install works. I opened #28831 for that.

Change History (24)

comment:1 Changed 3 years ago by Erik Bray

Authors: Erik Bray
Branch: u/embray/ticket-28832
Cc: Dima Pasechnik added
Commit: 19212a8db7f8deec0a864145b802b3aa85099d0c
Status: newneeds_review

This fixes it:

{
    "package_name": "suitesparse",
    "package_version": "5.6.0",
    "install_date": "Mon Dec  2 14:40:14 UTC 2019",
    "system_uname": "Linux sage-docker 3.13.0-170-generic #220-Ubuntu SMP Thu May 9 12:40:49 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux"
,
    "sage_version": "SageMath version 9.0.beta8, Release Date: 2019-12-01",
    "test_result": "",
    "files": [
        "include/RBio.h",
        "include/SuiteSparseQR.hpp",
        "include/SuiteSparseQR_C.h",
        "include/SuiteSparseQR_definitions.h",
        "include/SuiteSparse_config.h",
        "include/amd.h",
        "include/btf.h",
        "include/camd.h",
        "include/ccolamd.h",
        "include/cholmod.h",
        "include/cholmod_blas.h",
        "include/cholmod_camd.h",
        "include/cholmod_check.h",
        "include/cholmod_cholesky.h",
        "include/cholmod_complexity.h",
        "include/cholmod_config.h",
        "include/cholmod_core.h",
        "include/cholmod_function.h",
        "include/cholmod_gpu.h",
        "include/cholmod_gpu_kernels.h",
        "include/cholmod_io64.h",
        "include/cholmod_matrixops.h",
        "include/cholmod_modify.h",
        "include/cholmod_partition.h",
        "include/cholmod_supernodal.h",
        "include/cholmod_template.h",
        "include/colamd.h",
        "include/cs.h",
        "include/klu.h",
        "include/ldl.h",
        "include/spqr.hpp",
        "include/umfpack.h",
        "include/umfpack_col_to_triplet.h",
        "include/umfpack_defaults.h",
        "include/umfpack_free_numeric.h",
        "include/umfpack_free_symbolic.h",
        "include/umfpack_get_determinant.h",
        "include/umfpack_get_lunz.h",
        "include/umfpack_get_numeric.h",
        "include/umfpack_get_symbolic.h",
        "include/umfpack_global.h",
        "include/umfpack_load_numeric.h",
        "include/umfpack_load_symbolic.h",
        "include/umfpack_numeric.h",
        "include/umfpack_qsymbolic.h",
        "include/umfpack_report_control.h",
        "include/umfpack_report_info.h",
        "include/umfpack_report_matrix.h",
        "include/umfpack_report_numeric.h",
        "include/umfpack_report_perm.h",
        "include/umfpack_report_status.h",
        "include/umfpack_report_symbolic.h",
        "include/umfpack_report_triplet.h",
        "include/umfpack_report_vector.h",
        "include/umfpack_save_numeric.h",
        "include/umfpack_save_symbolic.h",
        "include/umfpack_scale.h",
        "include/umfpack_solve.h",
        "include/umfpack_symbolic.h",
        "include/umfpack_tictoc.h",
        "include/umfpack_timer.h",
        "include/umfpack_transpose.h",
        "include/umfpack_triplet_to_col.h",
        "include/umfpack_wsolve.h",
        "lib/libamd.so",
        "lib/libamd.so.2",
        "lib/libamd.so.2.4.6",
        "lib/libbtf.so",
        "lib/libbtf.so.1",
        "lib/libbtf.so.1.2.6",
        "lib/libcamd.so",
        "lib/libcamd.so.2",
        "lib/libcamd.so.2.4.6",
        "lib/libccolamd.so",
        "lib/libccolamd.so.2",
        "lib/libccolamd.so.2.9.6",
        "lib/libcholmod.so",
        "lib/libcholmod.so.3",
        "lib/libcholmod.so.3.0.13",
        "lib/libcolamd.so",
        "lib/libcolamd.so.2",
        "lib/libcolamd.so.2.9.6",
        "lib/libcxsparse.so",
        "lib/libcxsparse.so.3",
        "lib/libcxsparse.so.3.2.0",
        "lib/libklu.so",
        "lib/libklu.so.1",                                                                                                   
        "lib/libklu.so.1.3.8",
        "lib/libldl.so",
        "lib/libldl.so.2",
        "lib/libldl.so.2.2.6",
        "lib/librbio.so",
        "lib/librbio.so.2",
        "lib/librbio.so.2.2.6",
        "lib/libspqr.so",
        "lib/libspqr.so.2",
        "lib/libspqr.so.2.0.9",
        "lib/libsuitesparseconfig.so",
        "lib/libsuitesparseconfig.so.5",
        "lib/libsuitesparseconfig.so.5.5.0",
        "lib/libumfpack.so",
        "lib/libumfpack.so.5",
        "lib/libumfpack.so.5.7.9",
        "share/doc/suitesparse-5.6.0/AMD_README.txt",
        "share/doc/suitesparse-5.6.0/AMD_UserGuide.pdf",
        "share/doc/suitesparse-5.6.0/BTF_README.txt",
        "share/doc/suitesparse-5.6.0/CAMD_README.txt",
        "share/doc/suitesparse-5.6.0/CAMD_UserGuide.pdf",
        "share/doc/suitesparse-5.6.0/CCOLAMD_README.txt",
        "share/doc/suitesparse-5.6.0/CHOLMOD_README.txt",
        "share/doc/suitesparse-5.6.0/CHOLMOD_UserGuide.pdf",
        "share/doc/suitesparse-5.6.0/COLAMD_README.txt",
        "share/doc/suitesparse-5.6.0/CXSPARSE_README.txt",
        "share/doc/suitesparse-5.6.0/KLU_README.txt",
        "share/doc/suitesparse-5.6.0/KLU_UserGuide.pdf",
        "share/doc/suitesparse-5.6.0/LDL_README.txt",
        "share/doc/suitesparse-5.6.0/RBIO_README.txt",
        "share/doc/suitesparse-5.6.0/SPQR_README.txt",
        "share/doc/suitesparse-5.6.0/SUITESPARSECONFIG_README.txt",
        "share/doc/suitesparse-5.6.0/SuiteSparse_README.txt",
        "share/doc/suitesparse-5.6.0/UMFPACK_QuickStart.pdf",
        "share/doc/suitesparse-5.6.0/UMFPACK_README.txt",
        "share/doc/suitesparse-5.6.0/UMFPACK_UserGuide.pdf",
        "share/doc/suitesparse-5.6.0/ldl_userguide.pdf",
        "share/doc/suitesparse-5.6.0/spqr_user_guide.pdf"
    ]
}

I also confirmed that none of the installed files contain the DESTDIR temporary path, which is generally required for a proper staged install.


New commits:

19212a8Trac #28832: Support staged installation of suitesparse using DESTDIR.
Last edited 3 years ago by Erik Bray (previous) (diff)

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

Reviewers: François Bissey
Status: needs_reviewpositive_review

Fair enough, I didn't know it was this important. Otherwise, I would have made the effort. Looks good to me.

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

Replying to fbissey:

Fair enough, I didn't know it was this important. Otherwise, I would have made the effort. Looks good to me.

Yeah don't worry, not your fault. I need to do more work to make this clearer. It's not even currently that important for "standard" packages, but is still useful for proper uninstallation/cleanup when updating package versions.

comment:4 Changed 3 years ago by Volker Braun

Status: positive_reviewneeds_work

I'm getting

ranlib libamd.a
gcc -L/home/release/Sage/local/lib -Wl,-rpath,/home/release/Sage/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/release/Sage/local/var/tmp/sage/build/suitesparse-5.6.0/inst/home/release/Sage/local/lib/libamd.so.2.4.6 -lm -lrt -lsuitesparseconfig
/usr/bin/ld: cannot find -lsuitesparseconfig
collect2: error: ld returned 1 exit status
make[6]: *** [Makefile:90: /home/release/Sage/local/var/tmp/sage/build/suitesparse-5.6.0/inst/home/release/Sage/local/lib/libamd.so.2.4.6] Error 1
make[6]: Leaving directory '/home/release/Sage/local/var/tmp/sage/build/suitesparse-5.6.0/src/AMD/Lib'

on my Ryzen 3800X. Note:

[release@zen Sage]$ find local | grep suitesparseconfig
local/var/tmp/sage/build/suitesparse-5.6.0/src/SuiteSparse_config/libsuitesparseconfig.a
local/var/tmp/sage/build/suitesparse-5.6.0/inst/home/release/Sage/local/lib/libsuitesparseconfig.so.5.5.0
local/var/tmp/sage/build/suitesparse-5.6.0/inst/home/release/Sage/local/lib/libsuitesparseconfig.so
local/var/tmp/sage/build/suitesparse-5.6.0/inst/home/release/Sage/local/lib/libsuitesparseconfig.so.5

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

Hum, it should have worked. There must be something no behaving in AMD's makefile. Investigating.

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

I think I know what's happening. The new patch removes

+-    LDFLAGS += -L$(INSTALL_LIB)

And later we have

SO_OPTS = $(LDFLAGS)

So we only get sage's LDFLAGS which doesn't include the path to the temporary installation. I think that part of the patch should be removed.

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

Branch: u/embray/ticket-28832u/fbissey/ticket-28832
Commit: 19212a8db7f8deec0a864145b802b3aa85099d0c5b54ee73c5fbd24e0b4bd19abd610bba202eba85
Status: needs_workneeds_review

New commits:

5b54ee7Re-add some LDFLAGS that are needed in suitesparse

comment:8 Changed 3 years ago by John Palmieri

I'm getting doctesting errors on OS X 10.14.6, for example:

sage -t --long src/sage/numerical/optimize.py
**********************************************************************
File "src/sage/numerical/optimize.py", line 597, in sage.numerical.optimize.linear_program
Failed example:
    sol=linear_program(c,G,h)
Exception raised:
    Traceback (most recent call last):
      File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta8/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 681, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta8/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 1123, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.numerical.optimize.linear_program[3]>", line 1, in <module>
        sol=linear_program(c,G,h)
      File "sage/misc/lazy_import.pyx", line 353, in sage.misc.lazy_import.LazyImport.__call__ (build/cythonized/sage/misc/lazy_import.c:3686)
        return self.get_object()(*args, **kwds)
      File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta8/local/lib/python3.7/site-packages/sage/numerical/optimize.py", line 639, in linear_program
        sol=solvers.lp(c_,G_,h_,solver=solver)
      File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta8/local/lib/python3.7/site-packages/cvxopt/coneprog.py", line 2783, in lp
        from cvxopt import base, blas, misc
      File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta8/local/lib/python3.7/site-packages/cvxopt/misc.py", line 21, in <module>
        from cvxopt import base, blas, lapack, cholmod, misc_solvers
    ImportError: dlopen(/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta8/local/lib/python3.7/site-packages/cvxopt/cholmod.cpython-37m-darwin.so, 2): Library not loaded: /Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta8/local/var/tmp/sage/build/suitesparse-5.6.0/inst/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta8/local/lib/libcholmod.3.0.13.dylib
      Referenced from: /Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta8/local/lib/python3.7/site-packages/cvxopt/cholmod.cpython-37m-darwin.so
      Reason: image not found

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

Is it just with this ticket, or since we introduced suitesparse? Also does cvxopt work and pass its testsuite on the same machine?

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

Lack of install_name on OS X could be an issue but should also be present without this ticket.

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

Actually, now I am thinking that install_name could be automatically set to the location provided by the -o argument in linking. Which, with this ticket will be prefixed by DESTDIR and that would be an issue. And now that I tried to ask you some output of otool -L I realise that the prefix is present in what the loader asks. So this is the problem. I will fix it ASAP.

comment:12 Changed 3 years ago by git

Commit: 5b54ee73c5fbd24e0b4bd19abd610bba202eba858d795908b343506192a5f9cdcaab76a2a7624180

Branch pushed to git repo; I updated commit sha1. New commits:

8d79590fix install_name on OS X

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

Try it now.

comment:14 Changed 3 years ago by John Palmieri

I'm using a different machine (OS X 10.15.1) but same result. cvxopt fails its test suite, too: same problem. With plain Sage 9.0.beta8, all tests pass and cvxopt passes its test suite.

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

Even with my updated branch? What does otool -L local/lib/libcholmod.3.0.13.dylib report?

comment:16 Changed 3 years ago by John Palmieri

Yes, this with the updated branch; the latest entry in git log is

commit 8d795908b343506192a5f9cdcaab76a2a7624180 (HEAD -> t/28832/ticket-28832, trac/u/fbissey/ticket-28832)
Author: François Bissey <frp.bissey@gmail.com>
Date:   Fri Dec 6 14:10:15 2019 +1300

    fix install_name on OS X

I unpacked a tar file, ran git trac config, git trac checkout 28832, and then make ptestlong.

The paths are broken — they point to the temporary build directory:

% otool -L local/lib/libcholmod.3.0.13.dylib
local/lib/libcholmod.3.0.13.dylib:
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/var/tmp/sage/build/suitesparse-5.6.0/inst/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/lib/libcholmod.3.dylib (compatibility version 3.0.0, current version 3.0.13)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1281.0.0)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/var/tmp/sage/build/suitesparse-5.6.0/inst/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/lib/libamd.2.dylib (compatibility version 2.0.0, current version 2.4.6)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/var/tmp/sage/build/suitesparse-5.6.0/inst/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/lib/libcolamd.2.dylib (compatibility version 2.0.0, current version 2.9.6)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/var/tmp/sage/build/suitesparse-5.6.0/inst/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/lib/libsuitesparseconfig.5.dylib (compatibility version 5.0.0, current version 5.5.0)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/var/tmp/sage/build/suitesparse-5.6.0/inst/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/lib/libccolamd.2.dylib (compatibility version 2.0.0, current version 2.9.6)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/var/tmp/sage/build/suitesparse-5.6.0/inst/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/lib/libcamd.2.dylib (compatibility version 2.0.0, current version 2.4.6)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-9.0.beta8/local/lib/libopenblas_haswellp-r0.3.6.dylib (compatibility version 0.0.0, current version 0.0.0)

In comparison, with plain 9.0.beta8:

% otool -L local/lib/libcholmod.3.0.13.dylib
local/lib/libcholmod.3.0.13.dylib:
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/PYTHON3/sage-9.0.beta8/local/lib/libcholmod.3.0.13.dylib (compatibility version 3.0.0, current version 3.0.13)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1281.0.0)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/PYTHON3/sage-9.0.beta8/local/lib/libamd.2.4.6.dylib (compatibility version 2.0.0, current version 2.4.6)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/PYTHON3/sage-9.0.beta8/local/lib/libcolamd.2.9.6.dylib (compatibility version 2.0.0, current version 2.9.6)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/PYTHON3/sage-9.0.beta8/local/lib/libsuitesparseconfig.5.5.0.dylib (compatibility version 5.0.0, current version 5.5.0)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/PYTHON3/sage-9.0.beta8/local/lib/libccolamd.2.9.6.dylib (compatibility version 2.0.0, current version 2.9.6)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/PYTHON3/sage-9.0.beta8/local/lib/libcamd.2.4.6.dylib (compatibility version 2.0.0, current version 2.4.6)
	/Users/palmieri/Desktop/Sage_stuff/sage_builds/PYTHON3/sage-9.0.beta8/local/lib/libopenblas_haswellp-r0.3.6.dylib (compatibility version 0.0.0, current version 0.0.0)

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

I am officially stupid. Correcting...

comment:18 Changed 3 years ago by git

Commit: 8d795908b343506192a5f9cdcaab76a2a7624180efeab2f799c7d4d6b2d89e110b9ddaffc77b2712

Branch pushed to git repo; I updated commit sha1. New commits:

efeab2fcorrect the final path in install_name

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

Should be better now.

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

Replying to fbissey:

I think I know what's happening. The new patch removes

+-    LDFLAGS += -L$(INSTALL_LIB)

And later we have

SO_OPTS = $(LDFLAGS)

So we only get sage's LDFLAGS which doesn't include the path to the temporary installation. I think that part of the patch should be removed.

You're right, of course. I made the same update to the patch in #28829, but I really should have fixed the version here. I'll have to rebase #28829 on this one if you think it's ready.

comment:21 Changed 3 years ago by Erik Bray

Thank you for adding the OSX stuff--I should've caught that. Positive review from me pending review from John.

comment:22 Changed 3 years ago by John Palmieri

Status: needs_reviewpositive_review

This version works for me.

comment:23 Changed 3 years ago by John Palmieri

Authors: Erik BrayErik Bray, François Bissey
Reviewers: François BisseyFrançois Bissey, Erik Bray

comment:24 Changed 3 years ago by Volker Braun

Branch: u/fbissey/ticket-28832efeab2f799c7d4d6b2d89e110b9ddaffc77b2712
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.