Opened 4 years ago
Closed 4 years ago
#25118 closed defect (fixed)
gfan fails when compiled with XCode 9.3
Reported by:  vbraun  Owned by:  

Priority:  critical  Milestone:  sage8.3 
Component:  packages: standard  Keywords:  
Cc:  dimpase  Merged in:  
Authors:  Dima Pasechnik  Reviewers:  François Bissey 
Report Upstream:  N/A  Work issues:  
Branch:  ab26648 (Commits, GitHub, GitLab)  Commit:  ab26648b54a45f87e5cb8a18411298e8350a130b 
Dependencies:  Stopgaps: 
Description
On certain architectures only, gfan compiled with XCode 9.3 fails
sage t long src/doc/de/tutorial/tour_advanced.rst # 2 doctests failed sage t long src/doc/en/tutorial/tour_advanced.rst # 2 doctests failed sage t long src/doc/fr/tutorial/tour_advanced.rst # 2 doctests failed sage t long src/doc/ja/tutorial/tour_advanced.rst # 2 doctests failed sage t long src/doc/pt/tutorial/tour_advanced.rst # 2 doctests failed sage t long src/doc/ru/tutorial/tour_advanced.rst # 2 doctests failed sage t long src/sage/rings/polynomial/groebner_fan.py # 70 doctests failed sage t long src/sage/rings/polynomial/multi_polynomial_ideal.py # 1 doctest failed
There are also reported Cython testsuite failures, so it might be a compiler issue.
Change History (20)
comment:1 Changed 4 years ago by
 Cc dimpase added
comment:2 Changed 4 years ago by
comment:3 followup: ↓ 4 Changed 4 years ago by
gfan6.2 source assumes that cdd headers are in cdd/, so one can e.g.
cd $SAGE_LOCAL/include mkdir cdd cp cdd*.h cdd/ cp setoper.h cdd/
before hitting make. Also, the makefile needs to be modified not to use fnoguessbranchprobability
on clang.
comment:4 in reply to: ↑ 3 Changed 4 years ago by
Replying to dimpase:
gfan6.2 source assumes that cdd headers are in cdd/, so one can e.g.
cd $SAGE_LOCAL/include mkdir cdd cp cdd*.h cdd/ cp setoper.h cdd/before hitting make. Also, the makefile needs to be modified not to use
fnoguessbranchprobability
on clang.
For cdd's header you can pass DNOCDDPREFIX
that's one of the things I have learnt from looking at the new spkg at #23353. We probably should depend on it.
comment:5 Changed 4 years ago by
Can one try building/testing gfan with O0 on affected Xcode/CPUs, see if this provides a fix?
(i.e. with CXXFLAGS (and CFLAGS?) having O0 instead of the default?)
comment:6 Changed 4 years ago by
Travis CI can now do Xcode 9.3 for you,so it's matter of writing a suitable yaml file to test gfan there...
comment:7 Changed 4 years ago by
The crashes in gfan's 0.6.2 testsuite come from line 208 of app_main.cpp
delete ep;//< In next release, make the class virtual
It's a pointer to an object of type EnumerationFilePrinter handling output.
it's perhaps safe to just delete it, as it is at the end of main()
member function
of GCats class. (I'll figure it out exactly, anyway, it's a small thing to fix).
If I delete this line then 44 out of 45 tests pass (same as on Ubunty).
This was tried on OSX 10.13.4 on very old Core2 Duo CPU, with clang from XCode 9.3,
which is Apple LLVM version 9.1.0 (clang902.0.39.1)
.
Please, please, try this on i5/i7.
comment:8 Changed 4 years ago by
I won’t be able to try until the weekend.
comment:9 Changed 4 years ago by
Nice work Dima. So I took my broken 8.2.rc1. Digged the source for gfan0.5, found the line in those sources, created a patch and put it in the build tree. Check ed that the multi_polynomial_ideal.py
test is broken  yup. Rebuild gfan with my patch, gfan do not produces libraries, so there is no need to rebuild sage. We really want to upgrade and hope stuff like these is fixed by the way
[gfan0.5.p2] ./intsinpolytope.h:1:9: warning: 'INTSINPOLYTOPE_H_INCLUDED' is used as a header guard here, followed by #define of a different macro [Wheaderguard] [gfan0.5.p2] #ifndef INTSINPOLYTOPE_H_INCLUDED [gfan0.5.p2] ^~~~~~~~~~~~~~~~~~~~~~~~~ [gfan0.5.p2] ./intsinpolytope.h:2:9: note: 'INTSINPOLYTOPE_H_INLCUDED' is defined here; did you mean 'INTSINPOLYTOPE_H_INCLUDED'? [gfan0.5.p2] #define INTSINPOLYTOPE_H_INLCUDED [gfan0.5.p2] ^~~~~~~~~~~~~~~~~~~~~~~~~ [gfan0.5.p2] INTSINPOLYTOPE_H_INCLUDED [gfan0.5.p2] 1 warning generated.
Tried the multi_polynomial_ideal.py
test again. Success! I checked the other tests (well only one of the .rst, after all they are all copies of the same tests) and they work too.
I am doing full doctesting now in case something broke in the process.
And the patch

app_main.cpp
diff git a/app_main.cpp b/app_main.cpp index f6fcb66..e24b0b2 100644
a b public: 180 180 } 181 181 182 182 ep>close(); 183 delete ep;184 183 185 184 printf("\n"); 186 185
comment:10 Changed 4 years ago by
All doctests passed. We can go ahead with this.
comment:11 Changed 4 years ago by
Yep, I also tried the same as in comment 9 patch on 8.2.rc1 with latest Xcode on my museum Core2 Duo, and only some timeouts from tests, no real problems.
comment:12 Changed 4 years ago by
comment:13 Changed 4 years ago by
 Branch set to u/dimpase/gfan_xcode63_fix
 Commit set to 21464333b254d31bfe95801d575b203d9a9731cf
 Priority changed from major to blocker
 Status changed from new to needs_review
 Stopgaps #25113 deleted
comment:14 Changed 4 years ago by
I am going to be a pain in your ass. Please make it a revision bump of gfan.
comment:15 Changed 4 years ago by
 Commit changed from 21464333b254d31bfe95801d575b203d9a9731cf to ab26648b54a45f87e5cb8a18411298e8350a130b
Branch pushed to git repo; I updated commit sha1. New commits:
ab26648  gfan version bump

comment:16 Changed 4 years ago by
Done, sorry for forgetting this.
comment:17 Changed 4 years ago by
I wonder if something needs to be done with configure, as this changes configure.ac.
comment:18 Changed 4 years ago by
 Reviewers set to François Bissey
 Status changed from needs_review to positive_review
Depends on the flow of the release manager, but Volker didn't do a bump for #25113 so hopefully he has a procedure for that now.
comment:19 Changed 4 years ago by
 Milestone changed from sage8.2 to sage8.3
 Priority changed from blocker to critical
comment:20 Changed 4 years ago by
 Branch changed from u/dimpase/gfan_xcode63_fix to ab26648b54a45f87e5cb8a18411298e8350a130b
 Resolution set to fixed
 Status changed from positive_review to closed
See testing results for the "standalone" gfan6.2 on https://github.com/rwst/gfan06/issues/2
Basically, one test fails on OSX Xeon with Xcode 9.3 (same as on Ubuntu with gcc 5.4), while a slew of tests fail on OSX i5/7 with Xcode 9.3.