Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

suitesparse: Update to 7.0.1, fix DESTDIR bug #34206

Open
mkoeppe opened this issue Jul 21, 2022 · 84 comments
Open

suitesparse: Update to 7.0.1, fix DESTDIR bug #34206

mkoeppe opened this issue Jul 21, 2022 · 84 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 21, 2022

(from #34203 comment:34)

[suitesparse-5.10.1] gcc -std=c99 -L/Users/mkoeppe/s/sage/sage-rebasing/local/lib -Wl,-rpath,/Users/mkoeppe/s/sage/sage-rebasing/local/lib  -L/Users/mkoeppe/s/sage/sage-rebasing/local/var/tmp/sage/build/suitesparse-5.10.1/inst/Users/mkoeppe/s/sage/sage-rebasing/local/lib -L/Users/mkoeppe/s/sage/sage-rebasing/local/var/tmp/sage/build/suitesparse-5.10.1/inst/Users/mkoeppe/s/sage/sage-rebasing/local/lib -L/Users/mkoeppe/s/sage/sage-rebasing/local/var/tmp/sage/build/suitesparse-5.10.1/inst/Users/mkoeppe/s/sage/sage-rebasing/local/lib -L/Users/mkoeppe/s/sage/sage-rebasing/local/var/tmp/sage/build/suitesparse-5.10.1/inst/Users/mkoeppe/s/sage/sage-rebasing/local/var/tmp/sage/build/suitesparse-5.10.1/src/lib -dynamiclib -compatibility_version 1 -current_version 1.0.2 -Wl,-install_name -Wl,/Users/mkoeppe/s/sage/sage-rebasing/local/var/tmp/sage/build/suitesparse-5.10.1/src/lib/libsliplu.1.dylib -shared -undefined dynamic_lookup slip_matrix_div.o slip_create_mpq_array.o SLIP_free.o SLIP_LU_factorize.o SLIP_realloc.o slip_matrix_mul.o slip_get_largest_pivot.o slip_ref_triangular_solve.o SLIP_backslash.o slip_create_mpz_array.o slip_get_nonzero_pivot.o SLIP_LU_solve.o slip_back_sub.o slip_cumsum.o slip_get_pivot.o SLIP_malloc.o slip_sparse_collapse.o SLIP_calloc.o slip_dfs.o slip_get_smallest_pivot.o SLIP_matrix_allocate.o slip_sparse_realloc.o slip_cast_array.o slip_expand_double_array.o SLIP_gmp.o SLIP_matrix_copy.o SLIP_matrix_check.o slip_cast_matrix.o slip_expand_mpfr_array.o SLIP_initialize.o SLIP_matrix_free.o slip_check_solution.o slip_expand_mpq_array.o SLIP_initialize_expert.o SLIP_matrix_nnz.o SLIP_create_default_options.o SLIP_finalize.o SLIP_LU_analysis_free.o slip_permute_x.o slip_permute_b.o slip_create_mpfr_array.o slip_forward_sub.o SLIP_LU_analyze.o slip_reach.o -o /Users/mkoeppe/s/sage/sage-rebasing/local/var/tmp/sage/build/suitesparse-5.10.1/inst/Users/mkoeppe/s/sage/sage-rebasing/local/var/tmp/sage/build/suitesparse-5.10.1/src/lib/libsliplu.1.0.2.dylib -lm -rpath /Users/mkoeppe/s/sage/sage-rebasing/local/var/tmp/sage/build/suitesparse-5.10.1/inst/Users/mkoeppe/s/sage/sage-rebasing/local/var/tmp/sage/build/suitesparse-5.10.1/src/lib -lsuitesparseconfig -lamd -lcolamd -lm -lgmp -lmpfr

DESTDIR is used twice in the -o.

DESTDIR handling is coming from build/pkgs/suitesparse/patches/03-buildflags.patch

We upgrade to 5.12.0 and pick up a fresh set of patches from Debian or another distro

Depends on #34746
Depends on #34835

CC: @kiwifb

Component: packages: standard

Author: François Bissey

Branch/Commit: u/mkoeppe/suitesparse6.0.1 @ 424cb49

Issue created by migration from https://trac.sagemath.org/ticket/34206

@mkoeppe mkoeppe added this to the sage-9.7 milestone Jul 21, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 21, 2022

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 21, 2022

New commits:

bad180abuild/pkgs/suitesparse: Update to 5.12.0
abb0174build/pkgs/suitesparse: Add cmake dependency, activate components that need cmake

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 21, 2022

Commit: abb0174

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title suitesparse: Fix DESTDIR bug suitesparse: Update to 5.12.0, fix DESTDIR bug Jul 21, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 21, 2022

comment:3

DrTimothyAldenDavis/SuiteSparse#86 - "build system: Support staged installs via DESTDIR"

@kiwifb
Copy link
Member

kiwifb commented Jul 21, 2022

comment:4

My original spkg was always a quick and dirty job. It shouldn't be a surprise that some bugs appear.

@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@mkoeppe mkoeppe changed the title suitesparse: Update to 5.12.0, fix DESTDIR bug suitesparse: Update to 6.0.1, fix DESTDIR bug Dec 7, 2022
@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

comment:7

I guess I should have a stab at this.

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

comment:8

Way too many things are built by default. We can split the package or at least not building the big stuff that we do not need like graphblas and mongoose which are very specialised stuff.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 7, 2022

comment:9

I think our optional igraph package uses graphblas

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

comment:10

That's new and graphblas wasn't built before because I had removed all the subpackages using cmake at the time the initial suitesparse was included in sage. But if it can be used, why not.

In other news, DESTDIR works properly in 6.0.1 after some basic testing.

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

comment:11

Cannot see a dependency on GraphBLAS in igraph but if you can point to it, I'll certainly include it.

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

comment:12

I am testing a first draft locally. The changes are big enough that none of the current patches actually apply or are meaningful. I will need your input for any tweaks you require for BLAS/LAPACK on OS X. It should be easy enough to figure out if something is needed.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 7, 2022

comment:13

Replying to François Bissey:

Cannot see a dependency on GraphBLAS in igraph but if you can point to it, I'll certainly include it.

Sorry for the noise - looks like I misremembered

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

comment:14

Do we want to build and install static libraries?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 7, 2022

comment:15

No, only shared

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

Changed commit from abb0174 to none

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

Author: François Bissey

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

comment:16

Ready for inspection.

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

Changed branch from u/fbissey/suitsparse6.0.1 to trac/u/fbissey/suitsparse6.0.1

@kiwifb
Copy link
Member

kiwifb commented Dec 7, 2022

Changed branch from trac/u/fbissey/suitsparse6.0.1 to u/fbissey/suitesparse6.0.1

@kiwifb
Copy link
Member

kiwifb commented Dec 12, 2022

comment:51

Yes, it starts making much more sense.

@kiwifb
Copy link
Member

kiwifb commented Dec 12, 2022

comment:52

But there should be other similar failures potentially - at least you would think, since this is also tested in autoconf packages as soon blas/lapack is mixed with C code.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 12, 2022

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

d3544febuild/pkgs/zstd: New (gcc/gfortran dependency)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 12, 2022

Changed commit from 41d66d5 to d3544fe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 12, 2022

comment:54

Let's try this

@kiwifb
Copy link
Member

kiwifb commented Dec 12, 2022

comment:55

If it works, I'll have to re-enable "supernodal" in cholmod.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 12, 2022

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 12, 2022

Changed commit from d3544fe to 63c0139

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 12, 2022

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

63c0139build/pkgs/zstd: Only require zstd when gcc/gfortran are to be built

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 12, 2022

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 12, 2022

comment:59

in the failing build, gfortran still does not list zstd as a supported compression algorithm:

/sage/local/bin/gfortran -v
Using built-in specs.
COLLECT_GCC=/sage/local/bin/gfortran
COLLECT_LTO_WRAPPER=/sage/local/libexec/gcc/x86_64-pc-linux-gnu/12.2.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../src/configure --prefix=/sage/local --with-local-prefix=/sage/local --with-system-zlib --without-isl --disable-multilib --disable-nls --disable-libitm --disable-bootstrap --enable-languages=fortran --disable-darwin-at-rpath   CXX=g++
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.2.0 (GCC) 

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 12, 2022

Changed commit from 63c0139 to 424cb49

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 12, 2022

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

424cb49build/pkgs/zstd/spkg-install.in: Also install header

@kiwifb
Copy link
Member

kiwifb commented Dec 12, 2022

comment:61

In gentoo we just seem to do make -C lib install. Although there is also a make -C lib libzstd.pc during the build phase.

@kiwifb
Copy link
Member

kiwifb commented Dec 12, 2022

comment:62

Installing zstd may not be enough, you may have to tell gcc's configure to use it --with-zstd. But really, we want to match the support from the host gcc. So, it may be more complicated to achieve.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 12, 2022

comment:63

gfortran is now configured correctly

# local/bin/gfortran -v
Using built-in specs.
COLLECT_GCC=local/bin/gfortran
COLLECT_LTO_WRAPPER=/sage/local/libexec/gcc/x86_64-pc-linux-gnu/12.2.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../src/configure --prefix=/sage/local --with-local-prefix=/sage/local --with-system-zlib --without-isl --disable-multilib --disable-nls --disable-libitm --disable-bootstrap --enable-languages=fortran --disable-darwin-at-rpath   CXX=g++
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.2.0 (GCC) 

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 12, 2022

comment:64

Replying to François Bissey:

we want to match the support from the host gcc. So, it may be more complicated to achieve.

Quite possible that this is needed, yes

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 12, 2022

comment:65

suitesparse still fails to build.

comment:49 has changed - getting a different error now

# cat SuiteSparse_config/build/CMakeFiles/CMakeError.log
make[6]: Entering directory '/sage/local/var/tmp/sage/build/suitesparse-git/src/SuiteSparse_config/build/CMakeFiles/FortranCInterface'
make[6]: Leaving directory '/sage/local/var/tmp/sage/build/suitesparse-git/src/SuiteSparse_config/build/CMakeFiles/FortranCInterface'
make[6]: Entering directory '/sage/local/var/tmp/sage/build/suitesparse-git/src/SuiteSparse_config/build/CMakeFiles/FortranCInterface'
[ 92%] Building Fortran object CMakeFiles/FortranCInterface.dir/main.F.o
[ 94%] Building Fortran object CMakeFiles/FortranCInterface.dir/call_sub.f.o
[ 97%] Building Fortran object CMakeFiles/FortranCInterface.dir/call_mod.f90.o
[100%] Linking Fortran executable FortranCInterface
lto1: internal compiler error: bytecode stream: expected tag LTO_function instead of LTO_eh_table
0x9b4cc1 lto_tag_check
	../../src/gcc/lto-streamer.h:1028
0x9b4cc1 input_function
	../../src/gcc/lto-streamer-in.cc:1361
0x9b4cc1 lto_read_body_or_constructor
	../../src/gcc/lto-streamer-in.cc:1621
0x7020ce cgraph_node::get_untransformed_body()
	../../src/gcc/cgraph.cc:4011
0x70c48d cgraph_node::expand()
	../../src/gcc/cgraphunit.cc:1806
0x70c48d cgraph_node::expand()
	../../src/gcc/cgraphunit.cc:1788
0x70da4e expand_all_functions
	../../src/gcc/cgraphunit.cc:1999
0x70da4e symbol_table::compile()
	../../src/gcc/cgraphunit.cc:2349
0x680901 lto_main()
	../../src/gcc/lto/lto.cc:657
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
make[7]: *** [/tmp/ccL3cCy7.mk:2: /tmp/cc63Hwwl.ltrans0.ltrans.o] Error 1

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 12, 2022

comment:66

This will need more work on another day

@kiwifb
Copy link
Member

kiwifb commented Dec 14, 2022

comment:68

I have given it some thoughts. bookworm and sid are the only you report failing at this stage. It looks to me that they are both using gcc-12.2.0+patches - gentoo currently similarly ship gcc-12.2.1_pre20221210, some kind of pre-release. It seems it has incompatibilities with earlier gcc/gfortran releases, at least in debian. And our gcc/gfortran is an earlier and official release. I don't see a way out of this that doesn't forbid this version of the compiler unless there are both gcc and gfortran installed.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 14, 2022

comment:69

I agree, the Debian patches are likely the cause for the incompatibility. I think the LTO data comes with a version annotation, and Debian should probably have bumped the version along with their patches.

(We also carry a patch, for macOS M1/M2 support, but I don't think it is the problem here.)

@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Dec 31, 2022
@mkoeppe mkoeppe changed the title suitesparse: Update to 6.0.1, fix DESTDIR bug suitesparse: Update to 7.0.1, fix DESTDIR bug Feb 12, 2023
@mkoeppe mkoeppe modified the milestones: sage-10.0, sage-10.1 Apr 30, 2023
@mkoeppe mkoeppe removed this from the sage-10.1 milestone Aug 7, 2023
vbraun pushed a commit to vbraun/sage that referenced this issue Aug 27, 2024
    
<!-- ^^^^^
Upgrade the shipped suitesparse components to 7.6.0
-->
This is to fix sagemath#37192 and supersedes sagemath#34206
The new cmake superbuild is leveraged to only build the suitesparse
components of interest
* amd
* camd
* colamd
* ccolamd
* cholmod
* umfpack

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies


<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#37204
Reported by: François Bissey
Reviewer(s): Dima Pasechnik, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Aug 28, 2024
    
<!-- ^^^^^
Upgrade the shipped suitesparse components to 7.6.0
-->
This is to fix sagemath#37192 and supersedes sagemath#34206
The new cmake superbuild is leveraged to only build the suitesparse
components of interest
* amd
* camd
* colamd
* ccolamd
* cholmod
* umfpack

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies


<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#37204
Reported by: François Bissey
Reviewer(s): Dima Pasechnik, Matthias Köppe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants