Opened 3 years ago

Closed 8 weeks ago

#29413 closed enhancement (fixed)

Meta-ticket: Upgrade cddlib, fix cddlib header search for packages latte_int, gfan, topcom

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.8
Component: packages: optional Keywords: upgrade, cddlib
Cc: Michael Orlitzky, Dima Pasechnik, Julian Rüth, Samuel Lelièvre, Antonio Rojas, François Bissey, gh-kliem Merged in:
Authors: Dima Pasechnik Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 0c27c4f (Commits, GitHub, GitLab) Commit: 0c27c4f85247e7af526a5c4bb0fbb378c5ff1c2c
Dependencies: #31482 Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

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 Matthias Köppe

Milestone: sage-9.1sage-9.2

comment:2 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

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

Cc: Samuel Lelièvre added
Keywords: upgrade cddlib added

New: cddlib 0.94k was released on 2020-09-15.

Version 0, edited 2 years ago by Samuel Lelièvre (next)

comment:4 Changed 2 years ago by Dima Pasechnik

a newer cdd 0.94m fixing #30319 has been released

Last edited 2 years ago by Dima Pasechnik (previous) (diff)

comment:5 Changed 22 months ago by Matthias Köppe

Cc: Antonio Rojas François Bissey gh-kliem 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 François Bissey

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 Changed 22 months ago by François Bissey

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 in reply to:  7 Changed 22 months ago by Matthias Köppe

Replying to fbissey:

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.

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 user-provided include path.

comment:9 Changed 22 months ago by François Bissey

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 François Bissey

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 Changed 22 months ago by François Bissey

And topcom has a new version: 0.17.8 (still massively vendoring).

comment:12 Changed 22 months ago by François Bissey

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 gh-kliem

I think I got it figured out for latte now: https://github.com/latte-int/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 Matthias Köppe

Description: modified (diff)

comment:15 Changed 21 months ago by Matthias Köppe

Description: modified (diff)

comment:16 Changed 21 months ago by Matthias Köppe

Dependencies: #31482

comment:17 Changed 21 months ago by Matthias Köppe

Description: modified (diff)
Summary: Upgrade cddlib, fix cddlib header search for packages latte_int, gfan, topcomMeta-ticket: Upgrade cddlib, fix cddlib header search for packages latte_int, gfan, topcom

comment:18 Changed 21 months ago by Matthias Köppe

Also need to check polymake.

comment:19 Changed 21 months ago by Matthias Köppe

Description: modified (diff)

comment:20 Changed 21 months ago by Matthias Köppe

Description: modified (diff)

comment:21 Changed 21 months ago by Matthias Köppe

Description: modified (diff)

comment:22 Changed 21 months ago by gh-kliem

Description: modified (diff)

comment:23 in reply to:  11 Changed 21 months ago by Matthias Köppe

Description: modified (diff)

Replying to fbissey:

And topcom has a new version: 0.17.8 (still massively vendoring).

Opened #31531 for topcom

comment:24 Changed 21 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.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 Michael Orlitzky

Description: modified (diff)

comment:26 Changed 16 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

comment:27 Changed 12 months ago by Matthias Köppe

Milestone: sage-9.5sage-9.6

comment:28 Changed 8 months ago by Matthias Köppe

Milestone: sage-9.6sage-9.7

comment:29 Changed 4 months ago by Matthias Köppe

Milestone: sage-9.7sage-9.8

comment:30 Changed 3 months ago by John Palmieri

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:31 Changed 2 months ago by Dima Pasechnik

let's reenable cddlib for Homebrew.

comment:32 Changed 2 months ago by Dima Pasechnik

Authors: Dima Pasechnik
Branch: u/dimpase/configure/cdd_on_homebrew
Commit: add35c4283823f03756465793040d2362fd14ba9
Status: newneeds_review

New commits:

add35c4allow Homebrew's cddlib

comment:33 Changed 2 months ago by John Palmieri

Good idea. Is there any reason to keep this meta-ticket open and handle homebrew/cddlib elsewhere? Otherwise, I'm happy with this change.

comment:34 Changed 2 months ago by Matthias Köppe

I think also the comment in build/pkgs/cddlib/spkg-configure.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 git

Commit: add35c4283823f03756465793040d2362fd14ba99ee86aed6363aeead54a4076641e774d5f4e1232

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

9ee86aereflect the progress on #29413 in comments

comment:36 Changed 2 months ago by Dima Pasechnik

I've opened #34634 to deal with the renaming cddlib/->cdd/ of headers' subdir on Debian.

Last edited 2 months ago by Dima Pasechnik (previous) (diff)

comment:37 Changed 2 months ago by Matthias Köppe

Description: modified (diff)

comment:38 Changed 2 months ago by Matthias Köppe

Another update on debian please for the comment

comment:39 Changed 2 months ago by git

Commit: 9ee86aed6363aeead54a4076641e774d5f4e12320c27c4f85247e7af526a5c4bb0fbb378c5ff1c2c

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

0c27c4fall good on Debian

comment:40 in reply to:  38 Changed 2 months ago by Dima Pasechnik

Replying to Matthias Köppe:

Another update on debian please for the comment

done in ​0c27c4f

comment:41 Changed 2 months ago by Matthias Köppe

Reviewers: Matthias Koeppe
Status: needs_reviewpositive_review

Thanks. Should #31531 be a dependency?

comment:42 Changed 2 months ago by Dima Pasechnik

I don't see how #31531 is a dependency to this Homebrew-specific branch.

comment:43 Changed 2 months ago by Matthias Köppe

ok then

comment:44 Changed 8 weeks ago by Volker Braun

Branch: u/dimpase/configure/cdd_on_homebrew0c27c4f85247e7af526a5c4bb0fbb378c5ff1c2c
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.