Upgrade to giac 1.5 fails to build with older libstdc++

[embray]

#26315 broke building giac on some systems that have a old-ish libstdc++. It is completely mysterious to me why this broke when it worked before, and without a repository it's very difficult to determine what change between the versions is responsible, as there is little difference between the two versions in the relevant code.

Nevertheless, it can be fixed by explicitly using std::isnan and std::isinf so that there is no risk of them conflicting with the libc math.h equivalents thereof.

Upstream: Not yet reported upstream; Will do shortly.

CC: @dimpase @frederichan-IMJPRG @sagetrac-parisse @slel

Component: packages: standard

Author: Erik Bray

Branch: 2651e20

Reviewer: Dima Pasechnik

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

[embray]

comment:1

Also I have no idea what the FIR_LINUX macro here means. There is no documentation mentioning it and it is not defined anywhere.

[embray]

Commit: 832b97c

[embray]

Branch: u/embray/build/giac/isnan-isinf-bug

[embray]

Author: Erik Bray

[jdemeyer]

comment:2

Bad branch?

[jdemeyer]

comment:3

This might also have been caused by #26787.

[sagetrac-git]

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

2651e20

Trac #27263: Added patch which fixes this issue in GIAC

[sagetrac-git]

Changed commit from 832b97c to 2651e20

[embray]

Upstream: Not yet reported upstream; Will do shortly.

[embray]

comment:5

Replying to @jdemeyer:

This might also have been caused by #26787.

According to my research this occurs with or without -std=c++11 but I haven't explicitly tested that.

I will also try to bring it up upstream in case there is any idea.

---

New commits:

2651e20

Trac #27263: Added patch which fixes this issue in GIAC

[embray]

comment:7

I still don't fully understand what #26787 does though so it's possible.

[embray]

comment:8

Okay, confirmed that it is #26787 after all. If I just put unset CXX in the spkg-install for giac it builds. Something like that might be better than patching...

[embray]

comment:9

Right now in sage-env-config I have

export CONFIGURED_CXX="g++ -std=gnu++11 -std=gnu++11"

I don't know why the -std=gnu++11 is repeated. Conceivably it might also matter that it's gnu++11 and not c++11. Perhaps this older gcc is not fully c++11-compatible?

In any case, what would be the best approach here, do you think? Would it be reasonable to just unset CXX in the spkg-install?

[jdemeyer]

comment:10

Why not just apply the patch? I'd rather not mess with environment variables.

[embray]

comment:11

I guess, I don't really know what impact the patch has on other systems. But it does seem clear to me: Just explicitly use the isnan in the std namespace. I assume there's no reason that should go wrong...

[dimpase]

comment:12

LGTM

[dimpase]

Reviewer: Dima Pasechnik

[jdemeyer]

comment:13

I think you should bump the giac patchlevel such that this upgrade is actually tested.

[kiwifb]

comment:14

Replying to @embray:

Right now in sage-env-config I have

export CONFIGURED_CXX="g++ -std=gnu++11 -std=gnu++11"

The only way I think you can end up with that is for CXX to be g++ -std=gnu++11 at configuration time. The macro will then happily add another one because it is fairly stupid.

I don't know why the -std=gnu++11 is repeated. Conceivably it might also matter that it's gnu++11 and not c++11.

Don't think so but I am a C++ bigwig.

Perhaps this older gcc is not fully c++11-compatible?

Most certainly. Before gcc-5 C++11 support was experimental and the implementation was not fixed in stone.

In any case, what would be the best approach here, do you think? Would it be reasonable to just unset CXX in the spkg-install?

I'd rather have that little patch here.

[dimpase]

comment:15

Replying to @jdemeyer:

I think you should bump the giac patchlevel such that this upgrade is actually tested.

In theory yes, but this only matters for systems that otherwise simply cannot built giac, so you don't need to do this bumping for all practical purposes.

[vbraun]

Changed branch from u/embray/build/giac/isnan-isinf-bug to 2651e20

[slel]

Changed commit from 2651e20 to none