Opened 6 years ago

Last modified 4 years ago

#15209 needs_work enhancement

Upgrade experimental libFES package to version 0.2

Reported by: Bouillaguet Owned by:
Priority: minor Milestone: sage-6.8
Component: packages: experimental Keywords:
Cc: malb Merged in:
Authors: Charles Bouillaguet, Travis Scholl Reviewers:
Report Upstream: N/A Work issues:
Branch: u/tscholl2/libFES (Commits) Commit: e8c18d0d36cfe4f4f3a94e426c359917691cb503
Dependencies: Stopgaps:

Description (last modified by tscholl2)

A new version has been released !

What is new in this version? Nothing, except it is now twice faster for quadratic systems.

A patch must be applied to the sage tree

tarball: https://cloud.sagemath.com/projects/330d780d-8c93-4e93-9e4b-bfb04951901a/files/libFES-0.2.tar.bz2

Attachments (1)

libfes.patch (2.3 KB) - added by jdemeyer 4 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 Changed 6 years ago by Bouillaguet

  • Cc malb added; Martin Albrecht removed

comment:2 Changed 6 years ago by Bouillaguet

  • Branch set to u/Bouillaguet/libFES
  • Status changed from new to needs_review

comment:3 Changed 6 years ago by Bouillaguet

  • Commit set to b1f5e3ecc789db674fe17552812f2d0e2ab84be0

comment:4 Changed 5 years ago by malb

  • Status changed from needs_review to needs_work

Since we now moved to git proper the SPKG should be replaced by an entry in $SAGE_ROOT/build/pkgs.

comment:5 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:6 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:7 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:8 Changed 4 years ago by chapoton

  • Priority changed from major to minor

change of priority just to save the patchbots..

comment:9 Changed 4 years ago by tscholl2

  • Authors changed from Charles Bouillaguet to Charles Bouillaguet, Travis Scholl
  • Branch changed from u/Bouillaguet/libFES to u/tscholl2/libFES
  • Commit changed from b1f5e3ecc789db674fe17552812f2d0e2ab84be0 to 67016514210059a880fc7248ddd204f793121710
  • Description modified (diff)
  • Status changed from needs_work to needs_review

I noticed with the old version this library didn't pass many doctests

This new version seems to pass more, but not all of them. It's also now in a spkg folder in build/


New commits:

a9215b3Merge remote-tracking branch 'origin/u/Bouillaguet/libFES'
6701651updated libFES package to new version

comment:10 Changed 4 years ago by malb

If it doesn't pass all tests, why is the ticket "needs review"?

comment:11 Changed 4 years ago by jdemeyer

  • Milestone changed from sage-6.4 to sage-6.8
  • Status changed from needs_review to needs_work
  • Work issues set to type file

This needs a type file, see #18431.

If it doesn't pass all tests, why is the ticket "needs review"?

Given that the current version also doesn't pass all tests and given that it's an experimental package, I guess that's allowed.

comment:12 Changed 4 years ago by git

  • Commit changed from 67016514210059a880fc7248ddd204f793121710 to d4740f5f2d1b60ceccf9c45ab3e1ebd48d4a46db

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

d4740f5added a type file

comment:13 Changed 4 years ago by tscholl2

Sorry I was using 6.7 and didn't see the change about the type/dependency files in 6.8. I pushed new commit with those as well. Hopefully it now fits in with the current spkg standard.

comment:14 Changed 4 years ago by tscholl2

  • Status changed from needs_work to needs_review

comment:15 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work
  • Work issues type file deleted

Since you renamed the package from fes to libfes, you should also change all instances of optional - FES to optional - libFES (or whatever capitalization you prefer).

comment:16 Changed 4 years ago by git

  • Commit changed from d4740f5f2d1b60ceccf9c45ab3e1ebd48d4a46db to 2b2ff6080050c2293486b02210baea1907a599d9

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

2b2ff60renamed fes to libFES

comment:17 Changed 4 years ago by tscholl2

  • Status changed from needs_work to needs_review

Good point on changing the name everywhere.

I used ./sage -grep "FES" to find any other references. The only one I could find was in a polynomial function under rings/. That also referenced an old install_package function which when I ran Sage suggested using sage -i libFES so I changed that documentation as well.

Also I finally figured out that the functions that were being wrapped had several signature changes. Now they are how they should be and it passes all the tests.

comment:18 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Can you rename the package build/pkgs/libFES -> build/pkgs/libfes (all other packages in build/pkgs are lower-case). Also the package name in src/module_list.py and other places should be lower-case then.

Last edited 4 years ago by jdemeyer (previous) (diff)

comment:19 Changed 4 years ago by jdemeyer

I am also a bit worried that the upstream page doesn't mention this source tarball anywhere. Where does the tarball on this ticket come from?

comment:20 Changed 4 years ago by jdemeyer

This is not needed:

unset RM

And this shouldn't be needed if the upstream tarball was properly made using make dist (which makes it even more important to answer the question how that tarball was obtained):

autoreconf -i -v
Last edited 4 years ago by jdemeyer (previous) (diff)

comment:21 Changed 4 years ago by git

  • Commit changed from 2b2ff6080050c2293486b02210baea1907a599d9 to 5396eb34d09fa65aeef599a69c7aab9b8fe8cc8d

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

5396eb3removed 'unset RM'

comment:22 Changed 4 years ago by git

  • Commit changed from 5396eb34d09fa65aeef599a69c7aab9b8fe8cc8d to e8c18d0d36cfe4f4f3a94e426c359917691cb503

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

5c5b919renamed libFES to libfes
e8c18d0renamed rest of libFES to libfes

comment:23 follow-up: Changed 4 years ago by tscholl2

Thanks for the comments. I'm sorry I should have been more explicit about the source of the tarball.

The original source tarball is listed on the upstream website you linked to. Under "Advanced use" it has a link:

"You can get the code in a source tarball"

The link is to the bitbucket page for the source code, https://bitbucket.org/fes/fes/get/master.tar.bz2 The md5sum is 86a983de37eafa4c3110b1a640418023 master.tar.bz2

However, I had to modify this tarball because it expands to a folder named fes-fes-1229a4ec1e4e/. Somehow Sage's package installer wasn't correctly renaming the expanded format and installation would fail.

So I renamed fes-fes-1229a4ec1e4e/ to libFES-0.2/ and re-compressed the file with tar -cjf libFES-0.2.tar.bz2 libFES-0.2 The new md5sum is f895755c85ada462d77ab4d8e3fb4fd3 libFES-0.2.tar.bz2 This is the file linked to on my SMC project in the ticket description.

As for the lines in the spkg-install file, if I remove

autoreconf -i -v

I get an error when trying to install the package:

./spkg-install: line 19: ./configure: No such file or directory
Error configuring libFES

Is it bad to run autoreconf in install file? Should I run this command and then re-compress the folder?

comment:24 in reply to: ↑ 23 ; follow-up: Changed 4 years ago by jdemeyer

Replying to tscholl2:

The original source tarball is listed on the upstream website you linked to. Under "Advanced use" it has a link:

"You can get the code in a source tarball"

The link is to the bitbucket page for the source code, https://bitbucket.org/fes/fes/get/master.tar.bz2

Yes, but it's not really said that this is "version 0.2". It looks like it's just a snapshot of the bitbucket repository. Unless you know for sure that this is really version 0.2, use a date libfes-20140427 (the date of the files in the tarball) as version number. Do you feel like asking upstream about this?

Is it bad to run autoreconf in install file?

Yes, because autoreconf is not installed on many systems.

Should I run this command and then re-compress the folder?

No, the proper way to generate source tarball using autotools is to run make dist. So you should somehow get the correct source tarball, build it (in whatever way, using autoreconf if needed) on your own system, and run make dist. This command will generate a tarball which is meant to be used as source tarball.

comment:25 in reply to: ↑ 24 ; follow-up: Changed 4 years ago by tscholl2

Replying to jdemeyer:

Replying to tscholl2:

The original source tarball is listed on the upstream website you linked to. Under "Advanced use" it has a link:

"You can get the code in a source tarball"

The link is to the bitbucket page for the source code, https://bitbucket.org/fes/fes/get/master.tar.bz2

Yes, but it's not really said that this is "version 0.2". It looks like it's just a snapshot of the bitbucket repository. Unless you know for sure that this is really version 0.2, use a date libfes-20140427 (the date of the files in the tarball) as version number. Do you feel like asking upstream about this?

I compared the files and it matched the latest commit so I figured it was the latest snapshot. I could also check with upstream.

Is it bad to run autoreconf in install file?

Yes, because autoreconf is not installed on many systems.

Should I run this command and then re-compress the folder?

No, the proper way to generate source tarball using autotools is to run make dist. So you should somehow get the correct source tarball, build it (in whatever way, using autoreconf if needed) on your own system, and run make dist. This command will generate a tarball which is meant to be used as source tarball.

Running make dist fails and I don't quite understand the error message:

~/tmp/sources/fes$ make dist
make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory '/projects/330d780d-8c93-4e93-9e4b-bfb04951901a/tmp/sources/fes'
make[1]: *** No rule to make target 'configfsf.guess', needed by 'distdir'.  Stop.
make[1]: Leaving directory '/projects/330d780d-8c93-4e93-9e4b-bfb04951901a/tmp/sources/fes'
Makefile:600: recipe for target 'dist' failed
make: *** [dist] Error 2

I will email upstream (he was the original author of this ticket actually) and see if he knows how to fix this. Otherwise it will probably take me a few days to figure out this error. I thought since this was an optional and experimental package that requiring autoreconf was not so terrible.

comment:26 in reply to: ↑ 25 Changed 4 years ago by jdemeyer

Replying to tscholl2:

~/tmp/sources/fes$ make dist
make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory '/projects/330d780d-8c93-4e93-9e4b-bfb04951901a/tmp/sources/fes'
make[1]: *** No rule to make target 'configfsf.guess', needed by 'distdir'.  Stop.
make[1]: Leaving directory '/projects/330d780d-8c93-4e93-9e4b-bfb04951901a/tmp/sources/fes'
Makefile:600: recipe for target 'dist' failed
make: *** [dist] Error 2

The upstream autotools files are pretty broken, I will see what I can do.

Changed 4 years ago by jdemeyer

comment:27 Changed 4 years ago by jdemeyer

I attached to this ticket the needed fixes for the autotools project. With this fix, it passes make distcheck (which generates the distribution and checks that it actually works).

comment:28 Changed 4 years ago by tscholl2

Replying to jdemeyer:

I attached to this ticket the needed fixes for the autotools project. With this fix, it passes make distcheck (which generates the distribution and checks that it actually works).

I wasn't able to get make distcheck to pass when I applied your patch. Can you tell me if I am doing something wrong. I tried patching the tarball I attached and also with a totally new directory using the following code (this should be equivalent since the tarball attached should be the latest snapshot of upstream master):

git clone https://bitbucket.org/fes/fes.git
cd fes
wget http://trac.sagemath.org/raw-attachment/ticket/15209/libfes.patch
patch -p1 < libfes.patch
autoreconf -i -v
./configure
make
make distcheck

Thanks for looking into autotools for this!

comment:29 follow-up: Changed 4 years ago by jdemeyer

Exactly the same commands do work for me and it gives a tarball. What error do you get?

Ideally, upstream should apply the autotools patches and release a source tarball with make dist.

comment:30 in reply to: ↑ 29 Changed 4 years ago by tscholl2

Replying to jdemeyer:

Exactly the same commands do work for me and it gives a tarball. What error do you get?

Here is the error, it looks like it is expecting some directory that isn't there or isn't being named properly? Also it does generate a tarball, but it's only 45 bytes:

~/tmp/sources/fes$ make distcheck
make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory '/projects/330d780d-8c93-4e93-9e4b-bfb04951901a/tmp/sources/fes'
if test -d "libfes-0.2"; then find "libfes-0.2" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -rf "libfes-0.2" || { sleep 5 && rm -rf "libfes-0.2"; }; else :; fi
test -d "libfes-0.2" || mkdir "libfes-0.2"
 (cd src && make  top_distdir=../libfes-0.2 distdir=../libfes-0.2/src \
     am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory '/projects/330d780d-8c93-4e93-9e4b-bfb04951901a/tmp/sources/fes/src'
make[2]: Leaving directory '/projects/330d780d-8c93-4e93-9e4b-bfb04951901a/tmp/sources/fes/src'
 (cd test && make  top_distdir=../libfes-0.2 distdir=../libfes-0.2/test \
     am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory '/projects/330d780d-8c93-4e93-9e4b-bfb04951901a/tmp/sources/fes/test'
make[2]: Leaving directory '/projects/330d780d-8c93-4e93-9e4b-bfb04951901a/tmp/sources/fes/test'
test -n "" \
|| find "libfes-0.2" -type d ! -perm -755 \
        -exec chmod u+rwx,go+rx {} \; -o \
  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -444 -exec /bin/bash /projects/330d780d-8c93-4e93-9e4b-bfb04951901a/tmp/sources/fes/install-sh -c -m a+r {} {} \; \
|| chmod -R a+r "libfes-0.2"
tardir=libfes-0.2 && ${TAR-tar} chof - "$tardir" | GZIP=--best gzip -c >libfes-0.2.tar.gz
tar: value 738736247 out of uid_t range 0..2097151
tar: Exiting with failure status due to previous errors
make[1]: Leaving directory '/projects/330d780d-8c93-4e93-9e4b-bfb04951901a/tmp/sources/fes'
if test -d "libfes-0.2"; then find "libfes-0.2" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -rf "libfes-0.2" || { sleep 5 && rm -rf "libfes-0.2"; }; else :; fi
case 'libfes-0.2.tar.gz' in \
*.tar.gz*) \
  GZIP=--best gzip -dc libfes-0.2.tar.gz | ${TAR-tar} xf - ;;\
*.tar.bz2*) \
  bzip2 -dc libfes-0.2.tar.bz2 | ${TAR-tar} xf - ;;\
*.tar.lz*) \
  lzip -dc libfes-0.2.tar.lz | ${TAR-tar} xf - ;;\
*.tar.xz*) \
  xz -dc libfes-0.2.tar.xz | ${TAR-tar} xf - ;;\
*.tar.Z*) \
  uncompress -c libfes-0.2.tar.Z | ${TAR-tar} xf - ;;\
*.shar.gz*) \
  GZIP=--best gzip -dc libfes-0.2.shar.gz | unshar ;;\
*.zip*) \
  unzip libfes-0.2.zip ;;\
esac
chmod -R a-w libfes-0.2
chmod: cannot access ‘libfes-0.2’: No such file or directory
Makefile:606: recipe for target 'distcheck' failed
make: *** [distcheck] Error 1

If it helps, this is on running on a sage math cloud project. I'm not sure if there is some other tool that should be installed, but if there is I can ask William to install it for this.

Ideally, upstream should apply the autotools patches and release a source tarball with make dist.

I emailed upstream about this issue. I haven't heard back yet.

Note: See TracTickets for help on using tickets.