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 )
Change History (30)
comment:1 Changed 7 months ago by
- Branch set to public/31161
- Commit set to a8b6dedcb1c67d01a2850e207f72a126ce4eeec3
- Keywords upgrade qhull added
- Status changed from new to needs_review
comment:2 Changed 7 months ago by
- 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
- Milestone changed from sage-9.3 to sage-9.4
comment:4 Changed 3 months ago by
- Commit changed from a8b6dedcb1c67d01a2850e207f72a126ce4eeec3 to fa99c206a8bf2d4f3c45d1f818086538238eaf4a
comment:5 Changed 3 months ago by
- Status changed from needs_work to needs_review
comment:6 Changed 3 months ago by
- Reviewers set to https://github.com/sagemath/sagetrac-mirror/actions/runs/1045348934
testing on GH actions (with matplotlib 3.4 update on #31580)
comment:7 Changed 3 months ago by
- Description modified (diff)
comment:8 Changed 3 months ago by
- Milestone changed from sage-9.4 to sage-9.5
comment:9 Changed 6 weeks ago by
- Commit changed from fa99c206a8bf2d4f3c45d1f818086538238eaf4a to 47df75627ba56b259e838a63fe941886e19e451b
comment:10 Changed 6 weeks ago by
rebased over latest beta
comment:11 Changed 6 weeks ago by
comment:12 Changed 6 weeks ago by
- Commit changed from 47df75627ba56b259e838a63fe941886e19e451b to fcf8b2764a0379d2e9707d1e47ab311ece53a303
Branch pushed to git repo; I updated commit sha1. New commits:
| fcf8b27 | correct distros info
|
comment:13 Changed 5 weeks ago by
- Commit changed from fcf8b2764a0379d2e9707d1e47ab311ece53a303 to 09399677aba12df5cadd9e10c80ec70b7cf37481
comment:14 Changed 5 weeks ago by
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
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
- 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
- Component changed from packages: optional to packages: standard
- Keywords build-configure added
comment:18 Changed 5 weeks ago by
- Keywords spkg-configure added; build-configure removed
comment:19 follow-up: ↓ 20 Changed 5 weeks ago by
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
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
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
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
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
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.
comment:25 Changed 5 weeks ago by
- Commit changed from 09399677aba12df5cadd9e10c80ec70b7cf37481 to e073d08f6005e28b0b9ec3799b1d0b1a05691dfc
comment:26 Changed 5 weeks ago by
- Reviewers changed from https://github.com/sagemath/sagetrac-mirror/actions/runs/1233224818 to https://github.com/sagemath/sagetrac-mirror/actions/runs/1235471874, ...
restarted GH Action here https://github.com/sagemath/sagetrac-mirror/actions/runs/1235471874
comment:27 Changed 4 weeks ago by
tests are done, please have a look
comment:28 Changed 4 weeks ago by
- 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
Thanks. The GH Actions were run for #31580 (the branch here is a part of it).
comment:30 Changed 10 days ago by
- Branch changed from public/31161 to e073d08f6005e28b0b9ec3799b1d0b1a05691dfc
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
31161: Add qhull distro info31161: Upgrade: qhull 2020-src-8.0.2