Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade experimental libFES package to version 0.2 #15209

Open
sagetrac-Bouillaguet mannequin opened this issue Sep 18, 2013 · 36 comments
Open

Upgrade experimental libFES package to version 0.2 #15209

sagetrac-Bouillaguet mannequin opened this issue Sep 18, 2013 · 36 comments

Comments

@sagetrac-Bouillaguet
Copy link
Mannequin

sagetrac-Bouillaguet mannequin commented Sep 18, 2013

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

CC: @malb

Component: packages: experimental

Author: Charles Bouillaguet, Travis Scholl

Branch/Commit: u/tscholl2/libFES @ e8c18d0

Issue created by migration from https://trac.sagemath.org/ticket/15209

@sagetrac-Bouillaguet sagetrac-Bouillaguet mannequin added this to the sage-6.1 milestone Sep 18, 2013
@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Sep 19, 2013

Branch: u/Bouillaguet/libFES

@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Sep 19, 2013

Commit: b1f5e3e

@malb
Copy link
Member

malb commented Jan 22, 2014

comment:4

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

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@fchapoton
Copy link
Contributor

comment:8

change of priority just to save the patchbots..

@tscholl2

This comment has been minimized.

@tscholl2
Copy link
Contributor

Changed author from Charles Bouillaguet to Charles Bouillaguet, Travis Scholl

@tscholl2
Copy link
Contributor

Changed commit from b1f5e3e to 6701651

@tscholl2
Copy link
Contributor

comment:9

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

@tscholl2
Copy link
Contributor

Changed branch from u/Bouillaguet/libFES to u/tscholl2/libFES

@malb
Copy link
Member

malb commented May 25, 2015

comment:10

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

@jdemeyer
Copy link

Work Issues: type file

@jdemeyer
Copy link

comment:11

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.

@jdemeyer jdemeyer removed this from the sage-6.4 milestone May 25, 2015
@jdemeyer
Copy link

jdemeyer commented Jun 1, 2015

comment:15

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).

@jdemeyer
Copy link

jdemeyer commented Jun 1, 2015

Changed work issues from type file to none

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2015

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

2b2ff60renamed fes to libFES

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2015

Changed commit from d4740f5 to 2b2ff60

@tscholl2
Copy link
Contributor

tscholl2 commented Jun 2, 2015

comment:17

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.

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2015

comment:18

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.

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2015

comment:19

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?

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2015

comment:20

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2015

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

5396eb3removed 'unset RM'

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2015

Changed commit from 2b2ff60 to 5396eb3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2015

Changed commit from 5396eb3 to e8c18d0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2015

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

5c5b919renamed libFES to libfes
e8c18d0renamed rest of libFES to libfes

@tscholl2
Copy link
Contributor

tscholl2 commented Jun 2, 2015

comment:23

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?

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2015

comment:24

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.

@tscholl2
Copy link
Contributor

tscholl2 commented Jun 3, 2015

comment:25

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.

@jdemeyer
Copy link

jdemeyer commented Jun 3, 2015

comment:26

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.

@jdemeyer
Copy link

jdemeyer commented Jun 3, 2015

Attachment: libfes.patch.gz

@jdemeyer
Copy link

jdemeyer commented Jun 3, 2015

comment:27

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).

@tscholl2
Copy link
Contributor

tscholl2 commented Jun 6, 2015

comment:28

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 https://github.com/sagemath/sage-prod/files/10658445/libfes.patch.gz
patch -p1 < libfes.patch
autoreconf -i -v
./configure
make
make distcheck

Thanks for looking into autotools for this!

@jdemeyer
Copy link

jdemeyer commented Jun 6, 2015

comment:29

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.

@tscholl2
Copy link
Contributor

tscholl2 commented Jun 6, 2015

comment:30

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants