Opened 10 months ago

Closed 10 days ago

#31161 closed enhancement (fixed)

build/pkgs/qhull: Update, add spkg-configure.m4

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.5
Component: packages: standard Keywords: upgrade, qhull, spkg-configure
Cc: gh-zlscherr, jhpalmieri, dimpase, arojas, mjo, mkoeppe Merged in:
Authors: Samuel Lelièvre, Dima Pasechnik Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: e073d08 (Commits, GitHub, GitLab) Commit: e073d08f6005e28b0b9ec3799b1d0b1a05691dfc
Dependencies: Stopgaps:

Status badges

Description (last modified by dimpase)

Library conflicts prompted #31148 (MR51: Allow Matplotlib to use system qhull >= 7.2.0).

We should also allow our optional package qhull to find system qhull; and upgrade it on the way.

With #31580 qhull is promoted to standard.

Change History (30)

comment:1 Changed 7 months ago by slelievre

  • Authors set to Samuel Lelièvre
  • Branch set to public/31161
  • Commit set to a8b6dedcb1c67d01a2850e207f72a126ce4eeec3
  • Keywords upgrade qhull added
  • Status changed from new to needs_review

New commits:

63cf71e31161: Add qhull distro info
a8b6ded31161: Upgrade: qhull 2020-src-8.0.2

comment:2 Changed 7 months ago by slelievre

  • Status changed from needs_review to needs_work

This is still missing an spkg-configure.m4 file. Help welcome.

comment:3 Changed 7 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

comment:4 Changed 3 months ago by git

  • Commit changed from a8b6dedcb1c67d01a2850e207f72a126ce4eeec3 to fa99c206a8bf2d4f3c45d1f818086538238eaf4a

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

a43698e31161: Add qhull distro info
cbf8f4b31161: Upgrade: qhull 2020-src-8.0.2
fa99c20added basic spkg-configure.m4

comment:5 Changed 3 months ago by dimpase

  • Authors changed from Samuel Lelièvre to Samuel Lelièvre, Dima Pasechnik
  • Status changed from needs_work to needs_review

comment:6 Changed 3 months ago by dimpase

  • Reviewers set to https://github.com/sagemath/sagetrac-mirror/actions/runs/1045348934

testing on GH actions (with matplotlib 3.4 update on #31580)

Last edited 3 months ago by dimpase (previous) (diff)

comment:7 Changed 3 months ago by dimpase

  • Description modified (diff)

comment:8 Changed 3 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

comment:9 Changed 6 weeks ago by git

  • Commit changed from fa99c206a8bf2d4f3c45d1f818086538238eaf4a to 47df75627ba56b259e838a63fe941886e19e451b

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

8cdfdac31161: Add qhull distro info
8c5cd8131161: Upgrade: qhull 2020-src-8.0.2
47df756added basic spkg-configure.m4

comment:10 Changed 6 weeks ago by dimpase

rebased over latest beta

comment:12 Changed 6 weeks ago by git

  • Commit changed from 47df75627ba56b259e838a63fe941886e19e451b to fcf8b2764a0379d2e9707d1e47ab311ece53a303

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

fcf8b27correct distros info

comment:13 Changed 5 weeks ago by git

  • Commit changed from fcf8b2764a0379d2e9707d1e47ab311ece53a303 to 09399677aba12df5cadd9e10c80ec70b7cf37481

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

a3d08b431161: Add qhull distro info
8e99f5e31161: Upgrade: qhull 2020-src-8.0.2
64f613aadded basic spkg-configure.m4
0939967correct distros info

comment:14 Changed 5 weeks ago by dimpase

rebased over 9.5.beta1

tests on https://github.com/sagemath/sagetrac-mirror/actions/runs/1233039865 as part of matplotlib update to 3.4

comment:15 Changed 5 weeks ago by dimpase

scratch the latest GH action link, it should be https://github.com/sagemath/sagetrac-mirror/actions/runs/1233224818

comment:16 Changed 5 weeks ago by dimpase

  • Cc mjo mkoeppe added
  • Reviewers changed from https://github.com/sagemath/sagetrac-mirror/actions/runs/1045348934 to https://github.com/sagemath/sagetrac-mirror/actions/runs/1233224818

comment:17 Changed 5 weeks ago by dimpase

  • Component changed from packages: optional to packages: standard
  • Keywords build-configure added

comment:18 Changed 5 weeks ago by dimpase

  • Keywords spkg-configure added; build-configure removed

comment:19 follow-up: Changed 5 weeks ago by jhpalmieri

Why did we patch the makefile to apparently only install some of the pieces, and why is it now okay to remove that patch?

comment:20 in reply to: ↑ 19 Changed 5 weeks ago by dimpase

Replying to jhpalmieri:

Why did we patch the makefile to apparently only install some of the pieces, and why is it now okay to remove that patch?

That's the question for Samuel, I only added spkg-configure.m4 here.

comment:21 Changed 5 weeks ago by slelievre

Sorry. Please restore the patch.

Dima, can you indent the spkg-configure.m4 file using spaces only? And maybe set your text editor to not use tabs in .m4 files? See #31528.

comment:22 Changed 5 weeks ago by jhpalmieri

It built without the patch for me, but that's only one machine. So maybe the patch isn't needed with the new version? (Mine was an honest question, not a request to restore the patch.)

comment:23 Changed 5 weeks ago by dimpase

I don't know whether restoring this patch won't break the new matplotlib (which now depends on qhull). I won't be bothered, honestly. This patch looks unnecessary to me. Why it was there in the 1st place, goes back more than 5 years (I think it was imported from an old-style spkg, in 2016).

And it won't apply, it seems that the qhull's build system has been changed quite a bit.

comment:24 Changed 5 weeks ago by dimpase

one more minor thing: on Ubuntu the package name is libqhull-dev (and there is no libqhull or qhull package) - so this needs to be corrected.

Last edited 5 weeks ago by dimpase (previous) (diff)

comment:25 Changed 5 weeks ago by git

  • Commit changed from 09399677aba12df5cadd9e10c80ec70b7cf37481 to e073d08f6005e28b0b9ec3799b1d0b1a05691dfc

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

7e7e983correct package name on debian
e073d08replace tabs by spaces, adjust spacing

comment:26 Changed 5 weeks ago by dimpase

  • Reviewers changed from https://github.com/sagemath/sagetrac-mirror/actions/runs/1233224818 to https://github.com/sagemath/sagetrac-mirror/actions/runs/1235471874, ...

comment:27 Changed 4 weeks ago by dimpase

tests are done, please have a look

comment:28 Changed 4 weeks ago by jhpalmieri

  • Reviewers changed from https://github.com/sagemath/sagetrac-mirror/actions/runs/1235471874, ... to John Palmieri
  • Status changed from needs_review to positive_review

Looks okay to me. A further test will be how it works with the upgraded matplotlib, but that's for #31580.

comment:29 Changed 4 weeks ago by dimpase

Thanks. The GH Actions were run for #31580 (the branch here is a part of it).

comment:30 Changed 10 days ago by vbraun

  • Branch changed from public/31161 to e073d08f6005e28b0b9ec3799b1d0b1a05691dfc
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.