Opened 3 years ago
Closed 8 weeks ago
#29413 closed enhancement (fixed)
Metaticket: Upgrade cddlib, fix cddlib header search for packages latte_int, gfan, topcom
Reported by:  Matthias Köppe  Owned by:  

Priority:  major  Milestone:  sage9.8 
Component:  packages: optional  Keywords:  upgrade, cddlib 
Cc:  Michael Orlitzky, Dima Pasechnik, Julian Rüth, Samuel Lelièvre, Antonio Rojas, François Bissey, ghkliem  Merged in:  
Authors:  Dima Pasechnik  Reviewers:  Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  0c27c4f (Commits, GitHub, GitLab)  Commit:  0c27c4f85247e7af526a5c4bb0fbb378c5ff1c2c 
Dependencies:  #31482  Stopgaps: 
Description (last modified by )
Follow up from #28905.
We upgrade to 0.94m: https://repology.org/project/cddlib/versions
Change History (44)
comment:1 Changed 3 years ago by
Milestone:  sage9.1 → sage9.2 

comment:2 Changed 2 years ago by
Milestone:  sage9.2 → sage9.3 

comment:3 Changed 2 years ago by
Cc:  Samuel Lelièvre added 

Keywords:  upgrade cddlib added 
comment:5 Changed 22 months ago by
Cc:  Antonio Rojas François Bissey ghkliem added 

Description:  modified (diff) 
Are there already patches around for the cdd header file locations for gfan
and topcom
?
comment:6 Changed 22 months ago by
For cddlib
, this is a bit upsetting. We compile in Gentoo and in sage with DNOCDDPREFIX
which trigger block like this
#ifdef NOCDDPREFIX #include "setoper.h" #include "cdd.h" #include "cdd_f.h" #else #include "cdd/setoper.h" #include "cdd/cdd.h" #include "cdd/cdd_f.h" #endif
in src/lp_cdd.cpp
, src/gfanlib_zcone.cpp
and src/app_librarytest.cpp
. So, it feels a bit stupid now that the prefix is cddlib
. But at least the patch should be easy and then remove the DNOCDDPREFIX
from CXXFLAGS.
comment:7 followup: 8 Changed 22 months ago by
nix has a patch to go with the removal of DNOCDDPREFIX
. However, there is a simpler fix. gfan only install binaries and no headers. Adding Ipath_to/include/cddlib
to CXXFLAGS
will allow compilation and it is compatible with older cddlib too. I'll look at topcom next.
comment:8 Changed 22 months ago by
Replying to fbissey:
However, there is a simpler fix. gfan only install binaries and no headers. Adding
Ipath_to/include/cddlib
toCXXFLAGS
will allow compilation and it is compatible with older cddlib too.
This is not a good fix. All of these packages should be modified (with proper upstreamable patches) so that they look for upstream's official new header name cddlib/cdd.h
in the normal userprovided include path.
comment:9 Changed 22 months ago by
That's easy enough to do. I would hope the fix has already been forwarded to the gfan people. Upstream is overdue for a maintenance release :)
comment:10 Changed 22 months ago by
For topcom I am going to advocate adding to CPPFLAGS for now. Because upstream is a serial vendorer and its build system relies exclusively on vendored versions. Once we can get that upstream to get out of vendoring we may be able to suggest patches to them. But right now the point is moot.
comment:11 followup: 23 Changed 22 months ago by
And topcom has a new version: 0.17.8 (still massively vendoring).
comment:12 Changed 22 months ago by
Now that I see how much you already patched topcom, it is trivial to add a little love for cddlib at https://github.com/sagemath/sage/blob/develop/build/pkgs/topcom/patches/configure.ac#L63
comment:13 Changed 22 months ago by
I think I got it figured out for latte now: https://github.com/latteint/latte/pull/23
There is a couple of scenarios that could happen:
 System cdd in:
 inclusion directory
cdd/
cddlib'
 Sages cdd in:
 inclusion directory (of sage)
cddlib/
(once cdd is upgraded)
comment:14 Changed 21 months ago by
Description:  modified (diff) 

comment:15 Changed 21 months ago by
Description:  modified (diff) 

comment:16 Changed 21 months ago by
Dependencies:  → #31482 

comment:17 Changed 21 months ago by
Description:  modified (diff) 

Summary:  Upgrade cddlib, fix cddlib header search for packages latte_int, gfan, topcom → Metaticket: Upgrade cddlib, fix cddlib header search for packages latte_int, gfan, topcom 
comment:19 Changed 21 months ago by
Description:  modified (diff) 

comment:20 Changed 21 months ago by
Description:  modified (diff) 

comment:21 Changed 21 months ago by
Description:  modified (diff) 

comment:22 Changed 21 months ago by
Description:  modified (diff) 

comment:23 Changed 21 months ago by
Description:  modified (diff) 

comment:24 Changed 21 months ago by
Milestone:  sage9.3 → sage9.4 

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.
comment:25 Changed 17 months ago by
Description:  modified (diff) 

comment:26 Changed 16 months ago by
Milestone:  sage9.4 → sage9.5 

comment:27 Changed 12 months ago by
Milestone:  sage9.5 → sage9.6 

comment:28 Changed 8 months ago by
Milestone:  sage9.6 → sage9.7 

comment:29 Changed 4 months ago by
Milestone:  sage9.7 → sage9.8 

comment:30 Changed 3 months ago by
Is the last issue ("make singular pick sage's cdd") resolved? The Singular change is marked as "merged", and I've been using homebrew
's cddlib
and singular
on at least one OS X machine for a while with no apparent issues.
comment:32 Changed 2 months ago by
Authors:  → Dima Pasechnik 

Branch:  → u/dimpase/configure/cdd_on_homebrew 
Commit:  → add35c4283823f03756465793040d2362fd14ba9 
Status:  new → needs_review 
New commits:
add35c4  allow Homebrew's cddlib

comment:33 Changed 2 months ago by
Good idea. Is there any reason to keep this metaticket open and handle homebrew/cddlib elsewhere? Otherwise, I'm happy with this change.
comment:34 Changed 2 months ago by
I think also the comment in build/pkgs/cddlib/spkgconfigure.m4
should be updated so that it does not talk about this ticket as something for the future
comment:35 Changed 2 months ago by
Commit:  add35c4283823f03756465793040d2362fd14ba9 → 9ee86aed6363aeead54a4076641e774d5f4e1232 

Branch pushed to git repo; I updated commit sha1. New commits:
9ee86ae  reflect the progress on #29413 in comments

comment:36 Changed 2 months ago by
I've opened #34634 to deal with the renaming cddlib/>cdd/
of headers' subdir on Debian.
comment:37 Changed 2 months ago by
Description:  modified (diff) 

comment:39 Changed 2 months ago by
Commit:  9ee86aed6363aeead54a4076641e774d5f4e1232 → 0c27c4f85247e7af526a5c4bb0fbb378c5ff1c2c 

Branch pushed to git repo; I updated commit sha1. New commits:
0c27c4f  all good on Debian

comment:40 Changed 2 months ago by
comment:41 Changed 2 months ago by
Reviewers:  → Matthias Koeppe 

Status:  needs_review → positive_review 
Thanks. Should #31531 be a dependency?
comment:42 Changed 2 months ago by
I don't see how #31531 is a dependency to this Homebrewspecific branch.
comment:44 Changed 8 weeks ago by
Branch:  u/dimpase/configure/cdd_on_homebrew → 0c27c4f85247e7af526a5c4bb0fbb378c5ff1c2c 

Resolution:  → fixed 
Status:  positive_review → closed 
New: cddlib 0.94k was released on 20200915.