spkg-configure.m4 for symmetrica
Closed this issue · 41 comments
Debian, Fedora and Conda package it, so we can use it at least on some
boxes.
The present branch runs a test known to crash an unpatched Symmetrica version, so this allows to ignore an old Symmetrica version installed on old Fedora versions (e.g. on Fedora 26).
configure tarball:
http://users.ox.ac.uk/~coml0531/sage/configure-4416fd47f080f6f849eef2ba969b69a66d651df7.tar.gz
Depends on #27827
Component: build: configure
Author: Dima Pasechnik
Branch: 47f09f1
Reviewer: François Bissey, Isuru Fernando
Issue created by migration from https://trac.sagemath.org/ticket/28208
I wonder whether the Symmetrica Cython interface should be converted to using dynamic library. It's silly to link in a static one, creating a 30Mb
local/lib/python2.7/site-packages/sage/libs/symmetrica/symmetrica.so.
For comparison, the 2nd biggest .so among
local/lib/python2.7/site-packages/sage/libs/*/*.so is just 3Mb.
Given that the symmetrica spkg is using a custom makefile, it's just a matter of modifying it to build a shared library instead of a static one.
well, on OSX and Cygwin it might actually be a problem to get right. I guess it's a primary reason for having it static, it's easier. Buidling a shared library without autotoolization is a pain...
symmetrica package also needs a custom uninstall, as local/lib/libsymmetrica.a does not get cleaned for some reason. (Or perhaps it's a bug in the build system).
EDIT: I had a pre-sdh_install installation, on which uninstall failed (it does work on more recent installs). Perhaps we still want a custom uninstall, but at least it's something that will go away eventually by itself.
At present, it tests for presense of symmetrica.h header, which apparently what Debian has added (we can change it to ``symmetrica/defs.h`, which is one of the two original headers). I wonder what Conda would prefer.
conda package has symmetrica/defs.h and symmetrica/macro.h and no symmetrica.h
Replying to @isuruf:
conda package has
symmetrica/defs.handsymmetrica/macro.hand nosymmetrica.h
Thanks. Is there a place I can look up the sources used (without an actual conda install)? I am trying to understand what patches are applied there by conda.
Source used: https://github.com/conda-forge/symmetrica-feedstock/blob/master/recipe/meta.yaml#L10-L12
Patches used: https://github.com/conda-forge/symmetrica-feedstock/tree/master/recipe/patches
We use a CMake file to build which makes it easier to build windows packages and shared libs. (I've opened a PR to build shared libs at conda-forge/symmetrica-feedstock@3607965)
Built packages are at https://anaconda.org/conda-forge/symmetrica/files
Replying to @dimpase:
well, on OSX and Cygwin it might actually be a problem to get right. I guess it's a primary reason for having it static, it's easier. Buidling a shared library without autotoolization is a pain...
This is antic from way back in probably 2008 and 2009 when I was working with T. Abbott. I have a makefile for linux and a makefile for OS X. I only install a shared library. I should revisit it and do something to merge the makefiles.
I have the same headers than Isuru on conda.
And I'll certainly want to steal a cmake solution from conda if it is available :) . But cmake is still not an official sage package.
And I'll certainly want to steal a cmake solution from conda if it is available :)
It is. I just made symmetrica a shared library on conda for linux, osx and windows.
It's unfortunate that sage doesn't have cmake as a standard package.
Debian has autotoolised the package, if I am not mistaken:
https://salsa.debian.org/science-team/symmetrica/blob/master/debian/patches/upstream-autotoolization.patch
Anyhow, it's easy to make cmake standard; in practice this would hopefully mean that people will use system's cmake, as we have the relevant build/pkgs/cmake/spkg-configure.m4 already in place.
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
8253abb | spkg-configure.m4 for symmetrica |
Description changed:
---
+++
@@ -1,3 +1,4 @@
-Debian packages it, so we can use it on Debian.
+Debian and Conda package it, so we can use it at least on some
+boxes.
-The present branch does not try to check the version
+The present branch does not try to check the version - something that appears to be a tall order to me.Replying to @isuruf:
And I'll certainly want to steal a cmake solution from conda if it is available :)
It is. I just made symmetrica a shared library on conda for linux, osx and windows.
It's unfortunate that sage doesn't have cmake as a standard package.
I have one issue with your CMakeList.txt
ARCHIVE DESTINATION lib
LIBRARY DESTINATION lib
this should depend on the target. Sometimes it is lib64 or lib/ (debian). I usually use https://cmake.org/cmake/help/v3.15/module/GNUInstallDirs.html in my own cmake scripts. My version is now here
https://github.com/cschwan/sage-on-gentoo/blob/master/sci-libs/symmetrica/files/CMakeLists.txt
One thing interesting about the debian autotool patch is that it looks like they got the test working.
Reviewer: François Bissey
Looks rather trivial to me.
Thanks @kiwifb for the fixes. I added support for running tests with ctest as well.
Replying to @isuruf:
Thanks @kiwifb for the fixes. I added support for running tests with
ctestas well.
I think I messed up a little bit by setting SOVERSION to 2.0 instead of just 2 (I was trying to get back to what I had before, but it ended up not being possible because I was not using the right scheme). We should check what debian ends up with, but I think we should just use "2".
With the test working I will align myself onto you. No need to have too many versions around.
Unfortunately, Fedora comes with a buggy libsymmetrica-devel package.
So it needs to be recognised and ruled out.
More precisely it appears that the Sage's fixes to symmetrica have been applied to Fedora 30 (perhaps 29 too?), see Changelog in
https://fedora.pkgs.org/30/fedora-x86_64/symmetrica-devel-2.0-22.fc30.x86_64.rpm.html
but not to older ones -- I tested on Fedora 26, which is a year past its EOL.
The following C code dumps core on old symmetrica, and works on the good one.
/* compile as follows: cc tt.c -lsymmetrica -lm */
#include "symmetrica/def.h"
#include "symmetrica/macro.h"
int main()
{
OP b,n;
anfang();
n = callocobject(); b = callocobject();
sscan_integer("4",n);
kostka_tafel(n, b);
println(b);
ende();
}The output should be (from the good one, and Sage's doctest in src/sage/libs/symmetrica/kostka.pxi):
[1:0:0:0:0]
[1:1:0:0:0]
[1:1:1:0:0]
[1:2:1:1:0]
[1:3:2:3:1]
I'll make a test in the m4 file with it.
Description changed:
---
+++
@@ -1,4 +1,4 @@
-Debian and Conda package it, so we can use it at least on some
+Debian, Fedora and Conda package it, so we can use it at least on some
boxes.
-The present branch does not try to check the version - something that appears to be a tall order to me.
+The present branch runs a known to crash an unpatched Symmetrica version, so this allows to ignore an old Symmetrica version installed on old Fedora versions (e.g. on Fedora 26).OK, so this does the trick of ruling old version out.
Description changed:
---
+++
@@ -1,4 +1,4 @@
Debian, Fedora and Conda package it, so we can use it at least on some
boxes.
-The present branch runs a known to crash an unpatched Symmetrica version, so this allows to ignore an old Symmetrica version installed on old Fedora versions (e.g. on Fedora 26).
+The present branch runs a test known to crash an unpatched Symmetrica version, so this allows to ignore an old Symmetrica version installed on old Fedora versions (e.g. on Fedora 26).Checked debian and conda packages and they are picked up.
Changed reviewer from François Bissey to François Bissey, Isuru Fernando
Merge conflict
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
47f09f1 | bump configure version |
Description changed:
---
+++
@@ -2,3 +2,6 @@
boxes.
The present branch runs a test known to crash an unpatched Symmetrica version, so this allows to ignore an old Symmetrica version installed on old Fedora versions (e.g. on Fedora 26).
+
+configure tarball:
+http://users.ox.ac.uk/~coml0531/sage/configure-4416fd47f080f6f849eef2ba969b69a66d651df7.tar.gzChanged branch from u/dimpase/packages/symmconfig to 47f09f1