Context Navigation

← Previous Ticket
Next Ticket →

#21952 closed defect (wontfix)

Update cddlib's autotooling

Reported by:	embray	Owned by:
Priority:	major	Milestone:	sage-duplicate/invalid/wontfix
Component:	packages: standard	Keywords:	cddlib autotools
Cc:		Merged in:
Authors:	Erik Bray	Reviewers:
Report Upstream:	Reported upstream. Developers acknowledge bug.	Work issues:
Branch:		Commit:
Dependencies:		Stopgaps:

Status badges

Description

As discussed in this thread, cddlib's autotools files are quite outdated and in due for modernization. In fact, somehow, its configure.in is not even compatible with the version of automake that the Makefile.ins were generated with. This makes it difficult/impossible to make update cddlib's Makefiles (see #15872).

I reworked the latest version of cddlib (0.94h, though this work would also apply to at least the previous version, 0.94g) to use automake 1.14.1 (currently the highest version in sage, though #21196 would update that to 1.15).

I also cleaned up the contents of the tarball (removed .DS_Store files and other crud), and made it so that make distcheck works.

I've attached an updated upstream tarball, as well as a set of patches to the original upstream that should reproduce it.

Attachments (9)

cddlib-0.94.tar.gz (1.2 MB) - added by embray 4 years ago.
0001-Removed-generated-files-and-other-junk-that-should-n.patch (1014.5 KB) - added by embray 4 years ago.
0002-Newer-versions-expect-this-to-be-called-configure.ac.patch (2.1 KB) - added by embray 4 years ago.
0003-Added-the-modernization-to-configure.ac-suggested-at.patch (761 bytes) - added by embray 4 years ago.
0004-Added-some-recommendations-output-by-libtoolize-and-.patch (2.0 KB) - added by embray 4 years ago.
0005-Ran.patch (285.0 KB) - added by embray 4 years ago.
0006-Ran.patch (861.7 KB) - added by embray 4 years ago.
0007-Add-to-all-paths-in-EXTRA_DIST-so-that-VPATH-builds-.patch (946 bytes) - added by embray 4 years ago.
0008-Regenerated-Makefile.in-following-the-previous-chang.patch (912 bytes) - added by embray 4 years ago.

Change History (34)

Changed 4 years ago by embray

Attachment cddlib-0.94.tar.gz added

Changed 4 years ago by embray

Attachment 0005-Ran.patch added

Changed 4 years ago by embray

Attachment 0006-Ran.patch added

Changed 4 years ago by embray

Attachment 0007-Add-to-all-paths-in-EXTRA_DIST-so-that-VPATH-builds-.patch added

Changed 4 years ago by embray

Attachment 0008-Regenerated-Makefile.in-following-the-previous-chang.patch added

comment:1 Changed 4 years ago by mkoeppe

Do you have a git repository from which you created this?

comment:2 follow-up: ↓ 3 Changed 4 years ago by fbissey

I wouldn't have thought that ACLOCAL_AMFLAGS = -I m4 was necessary once you have AC_CONFIG_MACRO_DIR([m4]) but I could be wrong.

There are other things I am thinking off that can be done (VPATH) but we may not want to do everything at once.

comment:3 in reply to: ↑ 2 Changed 4 years ago by embray

Replying to fbissey:

I wouldn't have thought that ACLOCAL_AMFLAGS = -I m4 was necessary once you have AC_CONFIG_MACRO_DIR([m4]) but I could be wrong.

I'm fairly sure myself that it isn't strictly necessary, but I put it because autoconf suggested that I do anyways.

comment:4 Changed 4 years ago by embray

Or maybe it was libtoolize that told me that. I can't remember for sure. From https://www.gnu.org/software/libtool/manual/html_node/Invoking-libtoolize.html

"In the future other Autotools will automatically check the contents of AC_CONFIG_MACRO_DIRS, but at the moment it is more portable to add the macro directory to ACLOCAL_AMFLAGS in Makefile.am, which is where the tools currently look."

That future is now, but I guess it still suggests it just in case.

comment:5 Changed 4 years ago by mkoeppe

comment:6 Changed 4 years ago by embray

Type changed from PLEASE CHANGE to defect

comment:7 Changed 4 years ago by embray

Status changed from new to needs_review

I think this is actually ready to go. I just don't know what the process is for adding a new tarball to the sage package mirrors. For this particular case I don't see a better workaround than a new source package since the original is a bit broken.

comment:8 Changed 4 years ago by mkoeppe

There's no branch, what is to be reviewed?

Also, could you just share the git from which these patches are generated? (comment 1)

Also, "cddlib-0.94.tar.gz" does not seem to have an unambiguous version number.

comment:9 Changed 4 years ago by mkoeppe

Status changed from needs_review to needs_work

comment:10 Changed 4 years ago by embray

Status changed from needs_work to needs_info

What I don't understand is what the process is (though it seems like there is one?) of providing an alternative upstream tarball for one of the packages in Sage, such that is distributed to all the mirrors, etc.

I've attached such a tarball, as well as the patches that reproduce it from the original upstream tarball.

The problem here was that the source tarball itself was broken in various ways.

comment:11 Changed 4 years ago by mkoeppe

Most of the process is described here: http://doc.sagemath.org/html/en/developer/packaging.html#when-to-patch-when-to-repackage-when-to-autoconfiscate (and in the following sections).

The release manager adds the "upstream tar ball" to the server when the ticket is merged. Usually people just provide an URL in the description, rather than attaching it to trac.

Instead of attaching patches here, use a public git repository. Make an spkg-src that can regenerate your "upstream tar ball" from the repository. See bliss/spkg-src for an example.

comment:12 Changed 4 years ago by embray

Forgot about this until now, but thanks for the info. I'll follow the process you described then.

comment:13 Changed 3 years ago by saraedum

embray: are you planning to work on this again anytime soon? I just stumbled upon this because Debian has some patches to fix Sage's integration of cddlib for version 0.94h.

If you think that the patches proposed here are still the way this should be done, should we propose them upstream?

comment:14 Changed 3 years ago by fbissey

Just a note, from my experience in sage-on-gentoo, the debian patches are insufficient - as in I have doctest failures even with the debian patches.

Otherwise I am all for pushing updated autotooling upstream.

comment:15 Changed 3 years ago by embray

This really needs #21196. IIRC I ran into some stumbling blocks with that, but I forget--it was a long time ago.

comment:16 Changed 3 years ago by saraedum

Dependencies set to #21196

So, that's a dependency if I understood you correctly.

comment:17 Changed 3 years ago by saraedum

Dependencies #21196 deleted

I'm dropping that dependency again, as I am in favor of dropping autotools from our SPKGs.

comment:18 Changed 3 years ago by saraedum

Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.

I wrote a mail to upstream, offering my help with this. Let's see what they say.

comment:19 Changed 3 years ago by embray

I think the series of patches I originally attached to this ticket should do it. I forget why I just added a bunch of patches here as opposed to some other way of doing it...

comment:20 Changed 3 years ago by saraedum

Some of the patches don't apply anymore so I rebase them. I am trying to convince the author of the beauty of github (or similar) and he wants to have a look. I proposed a new home for upstream here: https://github.com/cddlib/cddlib.

And a modified version of your patches is here: https://github.com/cddlib/cddlib/pull/1

Let's continue the discussion there.