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 to Giac 1.5 #26315

Closed
slel opened this issue Sep 19, 2018 · 89 comments
Closed

Upgrade to Giac 1.5 #26315

slel opened this issue Sep 19, 2018 · 89 comments

Comments

@slel
Copy link
Member

slel commented Sep 19, 2018

This ticket is to upgrade to Giac 1.5.

A preliminary version "1.5.0 unstable" is available for testing:

This upgrade should solve #25822 and #25324 since the issues reported there are fixed upstream.

All that will be left to do in those tickets is add doctests.

I suppose we finally have stable 1.5.0, here
(repacked):
tarball: http://users.ox.ac.uk/~coml0531/sage/giac-1.5.0.37.tar.bz2

CC: @antonio-rojas @kiwifb @frederichan-IMJPRG @infinity0 @saraedum @tobihan @timokau

Component: packages: standard

Keywords: upgrade, giac

Author: Dima Pasechnik

Branch: 5986d80

Reviewer: François Bissey, Jeroen Demeyer

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

@frederichan-IMJPRG
Copy link

comment:1

NB: I have reported in giac's forum that without glpk the built of:

http://www-fourier.ujf-grenoble.fr/~parisse/debian/dists/stable/main/source/giac_1.5.0-1.tar.gz

is broken.
This is a minor change in src/graphe.cc, but may be we could wait 1.5.0-2?

Whatever, we never use the "unstable" file https://www-fourier.ujf-grenoble.fr/~parisse/giac/giac-1.5.0.tar.gz for packaging because it changes often.

cf: http://www-fourier.ujf-grenoble.fr/~parisse/debian/dists/stable/main/source/README

@slel

This comment has been minimized.

@slel
Copy link
Member Author

slel commented Sep 20, 2018

comment:3

Thanks frederichan for your comments and the link to the readme.

Of course it does not seem reasonable to upgrade to giac-1.5.0-1.

But I thought I'd at least open the ticket if only to point out that
upgrading to giac 1.5 should fix two other tickets.

@frederichan-IMJPRG
Copy link

comment:4

From 21 sep I can now see a 1.5.0-3 version:

http://www-fourier.ujf-grenoble.fr/~parisse/debian/dists/stable/main/source/giac_1.5.0-3.tar.gz

@kiwifb
Copy link
Member

kiwifb commented Sep 27, 2018

comment:5

Looking at 1.5.0-3 here. I have libsamplerate-0.1.9 installed on the machine and I discovered that it is one of the new automagically detected configure option (on if you have it). Building giac failed until I passed --disable-samplerate at configure time. Unstable indeed.

@frederichan-IMJPRG
Copy link

comment:6

Hi Francois,
I didn't knew about this one. Should I report something? (if yes what?)

But I know that glpk and nauty are new possible dependencies. As they standard sage packages, may be we should add them to giac's deps in sage?

Frederic

@kiwifb
Copy link
Member

kiwifb commented Sep 27, 2018

comment:7

Replying to @frederichan-IMJPRG:

Hi Francois,
I didn't knew about this one. Should I report something? (if yes what?)

Possibly. It may be also be because gcc-8 is more strict about some stuff.

libtool: compile:  x86_64-pc-linux-gnu-g++ -DHAVE_CONFIG_H -I. -I.. -DIN_GIAC -I. -I.. -I. -I.. -g -O2 -fno-strict-aliasing -DGIAC_GENERIC_CONSTANTS -c signalprocessing.cc  -fPIC -DPIC -o .libs/signalprocessing.o
signalprocessing.cc: In function ‘giac::gen giac::_resample(const giac::gen&, const giac::context*)’:
signalprocessing.cc:637:77: error: assignment of read-only location ‘*(data.SRC_DATA::data_in + ((sizetype)(((long unsigned int)((i * nc) + j)) * 4)))’
             data.data_in[i*nc+j]=_evalf(chdata[j][i],contextptr).DOUBLE_val();

But I know that glpk and nauty are new possible dependencies. As they standard sage packages, may be we should add them to giac's deps in sage?

glpk has been a dependency in the 1.4 series too. I hadn't noticed, seems to build fine with or without but it means sage installations of giac may be inconsistent which each others so I am in favor to just add it - regardless of this ticket.

The nauty dependency is new to me. Looking at configure.in I can see

AC_CHECK_LIB(nauty,main)
AC_CHECK_HEADERS(nauty/naututil.h)

so it is not fatal if it is not there. Which is just as well because that stuff looks for a library and as far as I know the nauty we have only install a bunch of executables, no libraries or headers.

So at this stage do not add nauty - may be we are thinking of the wrong package.

@kiwifb
Copy link
Member

kiwifb commented Sep 27, 2018

comment:8

nauty 2.6.7 has libraries. We are at 2.6.1 I think.

@antonio-rojas
Copy link
Contributor

comment:9

Plain upstream nauty doesn't compile shared libraries. It is a (huge) Debian specific patch, also used in Fedora.

@kiwifb
Copy link
Member

kiwifb commented Sep 28, 2018

comment:10

Replying to @antonio-rojas:

Plain upstream nauty doesn't compile shared libraries. It is a (huge) Debian specific patch, also used in Fedora.

The Gentoo maintainer has adopted that patch too for 2.6.7 that fooled me.

@dimpase
Copy link
Member

dimpase commented Oct 24, 2018

comment:11

There are still (easy) patches needed to build with clang 6.

Any idea about what would be the best way to get them in?

@slel
Copy link
Member Author

slel commented Oct 24, 2018

comment:12

One way would be to suggest these patches on the XCas "bugs" forum:
https://xcas.univ-grenoble-alpes.fr/forum/viewforum.php?f=3&sid=c49dc6cf44516f78a1ef24e838c65904

@frederichan-IMJPRG
Copy link

comment:13

NB: I can post on this forum for you if you don't have an account or if it is long to answer.

@dimpase
Copy link
Member

dimpase commented Oct 25, 2018

patch needed for giac to build with clang 6.0.1 (possibly incomplete)

@dimpase
Copy link
Member

dimpase commented Oct 25, 2018

comment:14

Attachment: giac-1.5.0-3.clang601.patch.gz

Replying to @frederichan-IMJPRG:

NB: I can post on this forum for you if you don't have an account or if it is long to answer.

I requested an account on the forum at least once, to no avail.
Anyhow, I just posted the patch here as an attachment.

@frederichan-IMJPRG
Copy link

comment:15

I have reported it there:
https://xcas.univ-grenoble-alpes.fr/forum/viewtopic.php?f=3&t=2176

@dimpase
Copy link
Member

dimpase commented Oct 25, 2018

comment:16

Another bug in giac - it does not work with newer versions of libcurl, which don't have
curl/curlbuild.h header, see curl's commit

So since curl version 7.55 or so one needs to use curl/system.h instead.

Could you also report this on giac list?

@kiwifb
Copy link
Member

kiwifb commented Oct 25, 2018

comment:17

Replying to @dimpase:

Another bug in giac - it does not work with newer versions of libcurl, which don't have
curl/curlbuild.h header, see curl's commit

So since curl version 7.55 or so one needs to use curl/system.h instead.

Could you also report this on giac list?

We've know this one for a while. It was present in the last 1.4 releases. In distro we just removed the include line following arch's lead (I believe it originated in arch - correct me if it has prior history).

@frederichan-IMJPRG
Copy link

comment:18

curlbuild.h is reported there:
https://xcas.univ-grenoble-alpes.fr/forum/viewtopic.php?f=3&t=2180

by the way, was the clang problem a warning or does the compilation stop?

@dimpase
Copy link
Member

dimpase commented Oct 26, 2018

comment:19

Replying to @frederichan-IMJPRG:

curlbuild.h is reported there:
https://xcas.univ-grenoble-alpes.fr/forum/viewtopic.php?f=3&t=2180

by the way, was the clang problem a warning or does the compilation stop?

I only reported errors. There are surely lots of warnings too, e.g. the one about register keyword being ignored and deprecated in C++11, and that it will be an error in C++17 standard.

As a matter of fact I am trying to get giac built on FreeBSD 12.0, and I am not yet there.

@dimpase dimpase added the t: bug label Oct 26, 2018
@dimpase dimpase added this to the sage-8.5 milestone Oct 26, 2018
@dimpase
Copy link
Member

dimpase commented Oct 30, 2018

comment:21

One further patch that was needed for clang 6/freebsd 12.0 is as follows:

--- a/src/misc.h
+++ b/src/misc.h
@@ -23,6 +23,7 @@
 #include "gen.h"
 #include "unary.h"
 #include "symbolic.h"
+#include <sys/time.h>
 
 #ifndef NO_NAMESPACE_GIAC
 namespace giac {

without it there are errors related to a forward declaration of giac::timespec, as follows:

In file included from misc.cc:8744:
In file included from /usr/local/include/curl/curl.h:87:
/usr/include/sys/time.h:267:25: error: variable has incomplete type 'struct timespec'
tstosbt(struct timespec _ts)
                        ^
/usr/include/sys/socket.h:676:8: note: forward declaration of 'giac::timespec'
struct timespec;
       ^

@dimpase
Copy link
Member

dimpase commented Oct 30, 2018

comment:22

OK, I can post to the giac forum now...

@dimpase
Copy link
Member

dimpase commented Nov 21, 2018

comment:23

the giac unstable version (1.5.0-11 (?)) of 18/10/18 builds on clang 6.0.1 out of the box (there is now an issue with doc/el, which seems to be broken, and the need to have hevea installed).

@kiwifb
Copy link
Member

kiwifb commented Nov 21, 2018

comment:24

I never saw that one but I see 1.5.0-17 from 2018-11-20. I will give it a go later.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 16, 2019

Changed commit from 5108e2b to 5986d80

@dimpase

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Feb 4, 2019

comment:65

Let's get on with this one.

@kiwifb
Copy link
Member

kiwifb commented Feb 4, 2019

Reviewer: François Bissey, Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Feb 5, 2019

Changed branch from public/giac-1.5.0 to 5986d80

@embray
Copy link
Contributor

embray commented Feb 12, 2019

comment:67

I just got this change on my Ubuntu 14.04 system and giac does not build:

[giac-1.5.0.37] global.cc: In function 'bool giac::my_isnan(double)':
[giac-1.5.0.37] global.cc:4145:19: error: call of overloaded 'isnan(double&)' is ambiguous
[giac-1.5.0.37]      return isnan(d);
[giac-1.5.0.37]                    ^
[giac-1.5.0.37] global.cc:4145:19: note: candidates are:
[giac-1.5.0.37] In file included from /usr/include/math.h:69:0,
[giac-1.5.0.37]                  from /usr/include/c++/4.8/cmath:44,
[giac-1.5.0.37]                  from first.h:498,
[giac-1.5.0.37]                  from giacPCH.h:7,
[giac-1.5.0.37]                  from global.cc:3:
[giac-1.5.0.37] /usr/include/x86_64-linux-gnu/bits/mathcalls.h:234:12: note: int isnan(double)
[giac-1.5.0.37]  __MATHDECL_1 (int,isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
[giac-1.5.0.37]             ^
[giac-1.5.0.37] In file included from first.h:498:0,
[giac-1.5.0.37]                  from giacPCH.h:7,
[giac-1.5.0.37]                  from global.cc:3:
[giac-1.5.0.37] /usr/include/c++/4.8/cmath:626:3: note: constexpr bool std::isnan(long double)
[giac-1.5.0.37]    isnan(long double __x)
[giac-1.5.0.37]    ^
[giac-1.5.0.37] /usr/include/c++/4.8/cmath:622:3: note: constexpr bool std::isnan(double)
[giac-1.5.0.37]    isnan(double __x)
[giac-1.5.0.37]    ^
[giac-1.5.0.37] /usr/include/c++/4.8/cmath:618:3: note: constexpr bool std::isnan(float)
[giac-1.5.0.37]    isnan(float __x)
[giac-1.5.0.37]    ^
[giac-1.5.0.37] global.cc: In function 'bool giac::my_isinf(double)':
[giac-1.5.0.37] global.cc:4158:19: error: call of overloaded 'isinf(double&)' is ambiguous
[giac-1.5.0.37]      return isinf(d);
[giac-1.5.0.37]                    ^
[giac-1.5.0.37] global.cc:4158:19: note: candidates are:
[giac-1.5.0.37] In file included from /usr/include/math.h:69:0,
[giac-1.5.0.37]                  from /usr/include/c++/4.8/cmath:44,
[giac-1.5.0.37]                  from first.h:498,
[giac-1.5.0.37]                  from giacPCH.h:7,
[giac-1.5.0.37]                  from global.cc:3:
[giac-1.5.0.37] /usr/include/x86_64-linux-gnu/bits/mathcalls.h:201:12: note: int isinf(double)
[giac-1.5.0.37]  __MATHDECL_1 (int,isinf,, (_Mdouble_ __value)) __attribute__ ((__const__));
[giac-1.5.0.37]             ^
[giac-1.5.0.37] In file included from first.h:498:0,
[giac-1.5.0.37]                  from giacPCH.h:7,
[giac-1.5.0.37]                  from global.cc:3:
[giac-1.5.0.37] /usr/include/c++/4.8/cmath:608:3: note: constexpr bool std::isinf(long double)
[giac-1.5.0.37]    isinf(long double __x)
[giac-1.5.0.37]    ^
[giac-1.5.0.37] /usr/include/c++/4.8/cmath:604:3: note: constexpr bool std::isinf(double)
[giac-1.5.0.37]    isinf(double __x)
[giac-1.5.0.37]    ^
[giac-1.5.0.37] /usr/include/c++/4.8/cmath:600:3: note: constexpr bool std::isinf(float)
[giac-1.5.0.37]    isinf(float __x)
[giac-1.5.0.37]    ^
[giac-1.5.0.37] make[4]: *** [global.lo] Error 1

@embray
Copy link
Contributor

embray commented Feb 12, 2019

Changed commit from 5986d80 to none

@dimpase
Copy link
Member

dimpase commented Feb 12, 2019

comment:68

Replying to @embray:

I just got this change on my Ubuntu 14.04 system and giac does not build:

...

[giac-1.5.0.37] /usr/include/c++/4.8/cmath:618:3: note: constexpr bool std::isnan(float)

...

}}}

hmm, blacklist gcc 4.8.* ? With EOL of 14.04 in April 2019, why bother fixing this?

@embray
Copy link
Contributor

embray commented Feb 12, 2019

comment:69

Apparently this may actually be a bug in some older libstdc++ https://stackoverflow.com/a/33770775/982257

@embray
Copy link
Contributor

embray commented Feb 12, 2019

comment:70

Replying to @dimpase:

Replying to @embray:

I just got this change on my Ubuntu 14.04 system and giac does not build:

...

[giac-1.5.0.37] /usr/include/c++/4.8/cmath:618:3: note: constexpr bool std::isnan(float)

...

}}}

hmm, blacklist gcc 4.8.* ? With EOL of 14.04 in April 2019, why bother fixing this?

It's definitely getting up there. I'll be annoyed if I have to distupgrade this machine, but at the same time it may be inevitable quite soon anyways. Regardless, I've made it this far, and if a simple ::isnan is enough to fix it...

@embray
Copy link
Contributor

embray commented Feb 12, 2019

comment:71

Interestingly, the code where this is coming up is already in a my_isnan function defined in src/global.cc like:

#if defined(FIR_LINUX) || defined(FIR_ANDROID)
    return std::isnan(d);
#else
    return isnan(d);
#endif

where if FIR_LINUX is defined it will do the right thing I think. But I have no idea what this macro FIR_LINUX means, or what FIR means, nor is it at all clear how it gets set.

@embray
Copy link
Contributor

embray commented Feb 12, 2019

comment:72

IMO we should never "upgrade" packages that don't even have publicly accessible source code repositories. It makes it impossible to debug what changed between versions if something goes wrong. Incredible.

@slel
Copy link
Member Author

slel commented Feb 12, 2019

comment:73

The source code repository of Giac is available as part of the Geogebra one:

@embray
Copy link
Contributor

embray commented Feb 21, 2019

comment:74

Replying to @slel:

The source code repository of Giac is available as part of the Geogebra one:

For some reason I thought they were just vendoring giac on a version-by-version basis, but it turns out I'm wrong. This seems to actually be the repository for giac. Nice find!!

Upon closer inspection no, it can't be. It seems like somehow bits of giac are being synced into this repository, but it's not the full source code for giac. So I'm really confused.

@embray
Copy link
Contributor

embray commented Feb 28, 2019

comment:75

I think this might have also broken something on Cygwin. After fixing all the other blocking issues on Cygwin, the tests for sage.interfaces.giac now hang.

@embray
Copy link
Contributor

embray commented Feb 28, 2019

comment:76

Replying to @embray:

I think this might have also broken something on Cygwin. After fixing all the other blocking issues on Cygwin, the tests for sage.interfaces.giac now hang.

Well, I tried it a few more times and it worked, so if there is a problem it's not easy to reproduce. At the very least, then, there's nothing to do for now until and unless I can reproduce the problem again. If so, I'll just open a new ticket about it.

@embray
Copy link
Contributor

embray commented Mar 4, 2019

comment:77

For #25822 and #25324 are there specific fixes from upstream that we know of that could easily be applied as patches? I'm wondering if we couldn't partially revert back to the old giac, plus patches for any specific known issues that this was meant to fix, until and unless I can figure out what's going on with #27385.

@dimpase
Copy link
Member

dimpase commented Mar 4, 2019

comment:78

I don't know about any specific patches - it took several iterations of communicating with Giac's author to fix Clang 6.0-specific bugs on giac's forum.

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

10 participants