#22646 closed enhancement (fixed)
./configure CC=/path/to/gcc ...
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.1 |
Component: | build | Keywords: | |
Cc: | jdemeyer, fbissey | Merged in: | |
Authors: | François Bissey | Reviewers: | Matthias Koeppe, Jeroen Demeyer, Vincent Delecroix, John Palmieri, Dima Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | 99eb31f (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
Standard autotools packages allow variables such as CC to be set at configure time using
./configure CC=/path/to/gcc
In this ticket, we add support for this. (The syntax already works, but the settings right now only end up in the unused file build/make/Makefile-auto
.)
New configure tarball: https://trac.sagemath.org/raw-attachment/ticket/22646/configure-235.tar.gz
Attachments (1)
Change History (136)
comment:1 Changed 5 years ago by
- Cc fbissey added
comment:2 Changed 5 years ago by
comment:3 Changed 5 years ago by
Yes, all of CC, CXX, FC, etc. As a guideline, one can look into the list of variables that appear in an automake-generated Makefile.
In my opinion, if the user provides a specific CC, CXX in this way and configure finds out that they are not suitable, that should be an error.
A gcc package should only be installed if no CC, CXX were provided in this way by the user.
Only in that case, sage-env
should do its special magic to set these variables to have the sage gcc used.
comment:4 Changed 5 years ago by
OK, past experience shows that if you set something special at build time, it is forgotten at runtime (see #21701) so we will need to leverage sage-env-config
, considering the magic in sage-env
that means moving quite a bit of stuff.
comment:5 Changed 5 years ago by
Yes, sage-env-config
is what needs to be used.
comment:6 Changed 5 years ago by
- Branch set to u/fbissey/compilers
- Commit set to 483106af4a85ea41461791619dab3f1cef9daa11
This is a first draft for this. I am dropping the setting for CPP
. If there are packages, not using autotools, that need it to be set, I'll reconsider.
New commits:
483106a | First draft of moving compiler settings to sage-env-config
|
comment:7 Changed 5 years ago by
- Status changed from new to needs_review
OK I am looking to make #12426 on this soon and rework slightly the toolchain building in the case were we only build gfortran. In the absence of further comments, I am putting this for review. I'll attach a configure tarball.
comment:8 Changed 5 years ago by
- Description modified (diff)
comment:9 Changed 5 years ago by
- Description modified (diff)
comment:10 Changed 5 years ago by
- Commit changed from 483106af4a85ea41461791619dab3f1cef9daa11 to 945e8243c7fccc612354b616904ac1e2ef51c8ae
comment:11 Changed 5 years ago by
- Description modified (diff)
comment:12 Changed 5 years ago by
- Commit changed from 945e8243c7fccc612354b616904ac1e2ef51c8ae to 32a0b00e746fd57a0830efc7176a6b8502c824fa
Branch pushed to git repo; I updated commit sha1. New commits:
32a0b00 | one AC_SUBST per variable only :(
|
comment:13 Changed 5 years ago by
- Commit changed from 32a0b00e746fd57a0830efc7176a6b8502c824fa to 11dc29ac731fcf912a739f9ba8ec7fd5c66b46e8
Branch pushed to git repo; I updated commit sha1. New commits:
11dc29a | configure/automake doesn't like variables only defined in conditionals
|
comment:14 Changed 5 years ago by
- Commit changed from 11dc29ac731fcf912a739f9ba8ec7fd5c66b46e8 to ef9f1b37c462bb61c036262a23b3588292d8f392
Branch pushed to git repo; I updated commit sha1. New commits:
ef9f1b3 | missing "fi" in sage-env-config.in
|
comment:15 follow-ups: ↓ 16 ↓ 18 Changed 5 years ago by
regarding CPP, there are some packages (gf2x
) that need CPP and CXXCPP to be set on FreeBSD.
comment:16 in reply to: ↑ 15 Changed 5 years ago by
Replying to dimpase:
regarding CPP, there are some packages (
gf2x
) that need CPP and CXXCPP to be set on FreeBSD.
Is it because CC
/CXX
is different from what you need for C{,XX}PP
? Or is there something missing in configure
? By default AC_PROG_C{,XX}PP
check C{C,XX} -E
first.
comment:17 Changed 5 years ago by
Just looked over at gf2x
' git repo. configure.ac
doesn't check for CPP
or CXXPP
we may want to get that done for the next release. In the meantime, I'd rather have individual spkg-install
set it as needed, because I don't want to interfere with what configure script may be looking for.
The thing that I particularly don't like is
if [ -x "$SAGE_LOCAL/bin/cpp" ]; then CPP=cpp fi
in sage-env
. Without it package using autoconf should naturally use gcc -E
, but because of this, when you install sage's gcc you get cpp
.
comment:18 in reply to: ↑ 15 Changed 5 years ago by
Replying to dimpase:
regarding CPP, there are some packages (
gf2x
) that need CPP and CXXCPP to be set on FreeBSD.
Reported to gf2x
upstream at https://gforge.inria.fr/tracker/index.php?func=detail&aid=21377&group_id=1874&atid=6979
comment:19 follow-up: ↓ 20 Changed 5 years ago by
I would like to test this with gcc-6 because my defafult gcc 7.1 is causing troubles (see this sage thread). Is that all what I should do on a freshly cloned Sage?
$ ./bootstrap $ ./configure CC=/usr/bin/gcc-6 CXX=/usr/bin/g++-6 $ make
comment:20 in reply to: ↑ 19 Changed 5 years ago by
Replying to vdelecroix:
I would like to test this with gcc-6 because my defafult gcc 7.1 is causing troubles (see this sage thread). Is that all what I should do on a freshly cloned Sage?
$ ./bootstrap $ ./configure CC=/usr/bin/gcc-6 CXX=/usr/bin/g++-6 $ make
Pretty much.
$ ./bootstrap $ CC=/usr/bin/gcc-6 CXX=/usr/bin/g++-6 make
should work too. Actually it would work without this ticket. But without this ticket once sage is built your settings for CC/CXX/FC is forgotten. You may want to read #21701 on the potential consequences of that amnesia.
comment:21 Changed 5 years ago by
I am trying the ./configure
way right now. I will report in few hours.
comment:22 Changed 5 years ago by
... even sooner, compilation started but then Sage tries to download gcc-5.4
! Is that expected? Should I also set SAGE_INSTALL_GCC=no
somewhere?
comment:23 Changed 5 years ago by
I was writing that when your comment hit: Something I didn't think about. sage may try to build its own gcc because the gfortran version is not the same as gcc.
Either provide the matching gfortran or uses #12426 which is build on top of this ticket and decouples gfortran (because clang).
comment:24 follow-up: ↓ 25 Changed 5 years ago by
It seems that archlinux do not provide the matching gfortran eventhough there is the Fortran runtime libraries for gcc-6... will try #12426
comment:25 in reply to: ↑ 24 ; follow-up: ↓ 26 Changed 5 years ago by
Replying to vdelecroix:
It seems that archlinux do not provide the matching gfortran eventhough there is the Fortran runtime libraries for gcc-6... will try #12426
sometimes gcc
can be called to compile fortran, e.g. something like gcc6 -x f95
might be able to compile fortran.
comment:26 in reply to: ↑ 25 Changed 5 years ago by
Replying to dimpase:
Replying to vdelecroix:
It seems that archlinux do not provide the matching gfortran eventhough there is the Fortran runtime libraries for gcc-6... will try #12426
sometimes
gcc
can be called to compile fortran, e.g. something likegcc6 -x f95
might be able to compile fortran.
You would need f951
to be installed for that to work. gcc, g++ and gfortran are front end compiler drivers. cc1, cxx1 and f951 are the real compilers. When you do something like gcc -x f95
you are basically instructing the main compiler driver to drive for f951 rather than cc1. f951 still needs to be present (usually in /usr/libexec/gcc/$host/$version/).
comment:27 Changed 5 years ago by
Success (for both make build
and sage -i polymake
)! Tell me if you want me to test more stuff.
comment:28 Changed 5 years ago by
Well, I'd like you to run the test suite (make ptestlong). Of course you ended up using the full #12426.
comment:29 Changed 5 years ago by
On top of #12426, ptestlong gives
---------------------------------------------------------------------- All tests passed! ----------------------------------------------------------------------
comment:30 Changed 5 years ago by
- Commit changed from ef9f1b37c462bb61c036262a23b3588292d8f392 to a50d44f8da9bf9d1408e8cb945f07d227c884fb9
comment:31 Changed 5 years ago by
- Description modified (diff)
comment:32 Changed 5 years ago by
I am trying
./configure CC=clang CXX=clang++
on Fedora 23 (clang 3.7) with SAGE_INSTALL_GCC=no
and I still see it doing some g++
tests:
... checking for complex.h... yes checking whether clang++ supports C++11 features by default... no checking whether clang++ supports C++11 features with -std=gnu++11... yes checking for clang option to accept ISO C99... none needed checking for Fortran flag needed to accept free-form source... -ffree-form checking if gcc accepts -dumpversion option... yes checking gcc version... 4.2.1 checking if g++ accepts -dumpversion option... yes checking g++ version... 4.2.1 checking for sqrt in -lm... yes ...
And I don't even get how it gets 4.2.1
there, as gcc/g++/gfortran installed is 5.3.1.
Is it "gcc emulation mode"?!
comment:33 Changed 5 years ago by
clang fakes gcc. Not any version of gcc, just 4.2.1. Same thing on OS X. So this is totally expected. I wouldn't call it an emulation mode. clang just happen to support all the pragmas identifying itself as gcc 4.2.1.
comment:34 Changed 5 years ago by
So this ends up with a silly error in libfplll:
... checking dependency style of clang... (cached) gcc3 checking whether make sets $(MAKE)... (cached) yes checking whether clang++ supports C++11 features by default... no checking whether clang++ supports C++11 features with -std=c++11... no checking whether clang++ supports C++11 features with -std=c++0x... no checking whether clang++ supports C++11 features with +std=c++11... no checking whether clang++ supports C++11 features with -h std=c++11... no configure: error: *** A compiler with support for C++11 language features is required. Error configuring fplll ...
Is this known, are there any patches needed?
comment:35 follow-up: ↓ 42 Changed 5 years ago by
I have no problem with clang 3.9. It looks like 3.7 may not have enough features. You may have noticed that sage's configure said that clang 3.7 could support c+11 with -std=gnu++11. It didn't check with -std=c++11. However fplll doesn't want gnu++11
AX_CXX_COMPILE_STDCXX([11],[noext],[mandatory])
the noext
in fplll's configure.ac prevents you from using gnu++11. It would probably work though.
comment:36 Changed 5 years ago by
As well, I have some linking gevalt with flint:
... clang -r ../build/ulong_extras/mulmod_precomp.lo ../build/ulong_extras/factor_trial_partial.lo ../build/ulong_extras/sizeinbase.lo . ./build/ulong_extras/randbits.lo ../build/ulong_extras/is_square.lo ../build/ulong_extras/inlines.lo ../build/ulong_extras/clog.lo .. /build/ulong_extras/mulmod2_preinv.lo ../build/ulong_extras/factor_SQUFOF.lo ../build/ulong_extras/cbrt_newton_iteration.lo ../build/ ulong_extras/nth_prime_bounds.lo ../build/ulong_extras/is_probabprime_lucas.lo ../build/ulong_extras/primes_clear.lo ../build/ulong_e xtras/jacobi.lo ../build/ulong_extras/factor_partial.lo ../build/ulong_extras/mod2_precomp.lo ../build/ulong_extras/flog.lo ../build/ ulong_extras/is_probabprime_fibonacci.lo ../build/ulong_extras/factorial_mod2_preinv.lo ../build/ulong_extras/invmod.lo ../build/ulon g_extras/factor_pp1.lo ../build/ulong_extras/prime_inverses_arr_readonly.lo ../build/ulong_extras/is_probabprime_BPSW.lo ../build/ulo ng_extras/powmod_preinv.lo ../build/ulong_extras/gcdinv.lo ../build/ulong_extras/randprime.lo ../build/ulong_extras/rootrem.lo ../bui ld/ulong_extras/powmod_precomp.lo ../build/ulong_extras/randint.lo ../build/ulong_extras/is_prime_pseudosquare.lo ../build/ulong_extras/primes_init.lo ../build/ulong_extras/primes_sieve_range.lo ../build/ulong_extras/mod2_preinv.lo ../build/ulong_extras/is_perfect_power235.lo ../build/ulong_extras/moebius_mu.lo ../build/ulong_extras/sqrtrem.lo ../build/ulong_extras/factor_insert.lo ../build/ulong_extras/revbin.lo ../build/ulong_extras/mod_precomp.lo ../build/ulong_extras/cbrt_estimate.lo ../build/ulong_extras/primitive_root_prime.lo ../build/ulong_extras/factor.lo ../build/ulong_extras/factor_trial.lo ../build/ulong_extras/is_strong_probabprime2_preinv.lo ../build/ulong_extras/lll_mod_preinv.lo ../build/ulong_extras/is_prime_pocklington.lo ../build/ulong_extras/pow.lo ../build/ulong_extras/factor_power235.lo ../build/ulong_extras/sqrt.lo ../build/ulong_extras/remove2_precomp.lo ../build/ulong_extras/sqrtmodn.lo ../build/ulong_extras/root_estimate.lo ../build/ulong_extras/cbrtrem.lo ../build/ulong_extras/discrete_log_bsgs.lo ../build/ulong_extras/prime_pi.lo ../build/ulong_extras/sqrtmod_primepow.lo ../build/ulong_extras/randlimb.lo ../build/ulong_extras/sqrtmod.lo ../build/ulong_extras/root.lo ../build/ulong_extras/divrem2_precomp.lo ../build/ulong_extras/factorial_fast_mod2_preinv.lo ../build/ulong_extras/cbrt.lo ../build/ulong_extras/remove.lo ../build/ulong_extras/is_probabprime.lo ../build/ulong_extras/is_squarefree.lo ../build/ulong_extras/is_probabprime_fermat.lo ../build/ulong_extras/randtest.lo ../build/ulong_extras/primes_arr_readonly.lo ../build/ulong_extras/primes_jump_after.lo ../build/ulong_extras/is_prime.lo ../build/ulong_extras/nextprime.lo ../build/ulong_extras/compute_primes.lo ../build/ulong_extras/gcd.lo ../build/ulong_extras/nth_prime.lo ../build/ulong_extras/factor_lehman.lo ../build/ulong_extras/ll_mod_preinv.lo ../build/ulong_extras/prime_pi_bounds.lo ../build/ulong_extras/primes_extend_small.lo ../build/ulong_extras/is_strong_probabprime_precomp.lo ../build/ulong_extras/is_oddprime_binary.lo ../build/ulong_extras/cbrt_chebyshev_approximation.lo ../build/ulong_extras/cleanup_primes.lo ../build/ulong_extras/euler_phi.lo ../build/ulong_extras/factor_trial_range.lo ../build/ulong_extras/factor_one_line.lo ../build/ulong_extras/cbrt_binary_search.lo ../build/ulong_extras/xgcd.lo ../build/ulong_extras/is_oddprime_small.lo ../build/ulong_extras/powmod2_preinv.lo ../build/ulong_extras/mulmod_preinv.lo -o ../build/ulong_extras/../ulong_extras.lo -nostdlib clang: warning: argument unused during compilation: '-r' /usr/bin/ld: warning: cannot find entry symbol _start; defaulting to 00000000004001f0 ../build/ulong_extras/is_square.lo: In function `n_is_square': /home/scratch/dimpase/sage/sage-clang/local/var/tmp/sage/build/flint-2.5.2.p1/src/ulong_extras/is_square.c:54: undefined reference to `sqrt' ...
Is this all due to -stdlib=libstdc++
?
comment:37 follow-up: ↓ 39 Changed 5 years ago by
Ha... the fix for compiling flint on debian 9 with pie breaks that compiler... That's because flint insist on not using autotools (duck for cover).
I don't know how your clang 3.7 is built, it may also be using g++ libstdc++ in which case the version of g++ is important for c++11 support. Which version of gcc/g++ is this?
comment:38 follow-up: ↓ 40 Changed 5 years ago by
with giac it's definitely some lib(std)c++-related trouble:
./index.h:572:11: error: no template named 'hash_map' in namespace 'std'; did you mean '__gnu_cxx::hash_map'? typedef std::hash_map< index_t,index_m,hash_function_object > hash_index ; ^~~~~~~~~~~~~ __gnu_cxx::hash_map
Should we abandon efforts to run on clang++ with libstdc++, and only allow libc++ with clang++ ?
comment:39 in reply to: ↑ 37 Changed 5 years ago by
Replying to fbissey:
Ha... the fix for compiling flint on debian 9 with pie breaks that compiler... That's because flint insist on not using autotools (duck for cover).
hear, hear, @wbhart :-)
I don't know how your clang 3.7 is built, it may also be using g++ libstdc++ in which case the version of g++ is important for c++11 support. Which version of gcc/g++ is this?
It's g++ 5.3.1.
comment:40 in reply to: ↑ 38 Changed 5 years ago by
Replying to dimpase:
with giac it's definitely some lib(std)c++-related trouble:
./index.h:572:11: error: no template named 'hash_map' in namespace 'std'; did you mean '__gnu_cxx::hash_map'? typedef std::hash_map< index_t,index_m,hash_function_object > hash_index ; ^~~~~~~~~~~~~ __gnu_cxx::hash_mapShould we abandon efforts to run on clang++ with libstdc++, and only allow libc++ with clang++ ?
Possibly. More checks to devise. That clang 3.7 is obviously not good enough.
comment:41 Changed 5 years ago by
I'm trying this to test #12426 but I get a download error. I already created an Error was mine, or my browser's, perhaps.
upstream/
directory and placed the tarball on this ticket on it, so I'm not sure why it's even trying to download it. That said, I started completely from scratch with a new git clone
.
comment:42 in reply to: ↑ 35 Changed 5 years ago by
Replying to fbissey:
I have no problem with clang 3.9. It looks like 3.7 may not have enough features. You may have noticed that sage's configure said that clang 3.7 could support c+11 with -std=gnu++11. It didn't check with -std=c++11. However fplll doesn't want gnu++11
Shouldn't Sage ./configure check for C++11 support with-std=c++11
?
Currently it checks for C++11 support -std=gnu++11
; e.g. on FreeBSD I see
checking whether clang++-devel supports C++11 features with -std=gnu++11... yes
comment:43 Changed 5 years ago by
If you are using #12426 to test this you need the configure tarball from #12426.
The c++11 test is a bit more subtle than that. If you don't say anything it will check for both in turn, but starts with gnu++11 and stops if that works. If you want to force the test for std=c++11 you have to pass noext
to the macro. gnu++11 should be a super set of c++11 which is why it stops there. I am not sure why fplll insists that you shouldn't use gnu++11.
comment:44 follow-ups: ↓ 45 ↓ 46 Changed 5 years ago by
concerning GF2X on FreeBSD (cf comment 16), does the following patch fix the issue?
--- a/configure.ac +++ b/configure.ac @@ -93,10 +93,10 @@ AC_ARG_ENABLE([fft-interface], AM_CONDITIONAL([ENABLE_FFT_INTERFACE],[test "x$enable_fft_interface" = xyes]) AC_PROG_CC -AC_PROG_CPP AC_PROG_CXX AC_COMPILE_WARNINGS AC_PROG_CC_C99 +AC_PROG_CPP GF2X_PROG_CC_FOR_BUILD GF2X_PROG_EXEEXT_FOR_BUILD
comment:45 in reply to: ↑ 44 Changed 5 years ago by
Replying to zimmerma:
concerning GF2X on FreeBSD (cf comment 16), does the following patch fix the issue?
--- a/configure.ac +++ b/configure.ac @@ -93,10 +93,10 @@ AC_ARG_ENABLE([fft-interface], AM_CONDITIONAL([ENABLE_FFT_INTERFACE],[test "x$enable_fft_interface" = xyes]) AC_PROG_CC -AC_PROG_CPP AC_PROG_CXX AC_COMPILE_WARNINGS AC_PROG_CC_C99 +AC_PROG_CPP GF2X_PROG_CC_FOR_BUILD GF2X_PROG_EXEEXT_FOR_BUILD
The patch is difficult to apply considering there is no AC_PROG_CPP
in gf2x-1.1, I think you missed thome's first commit adding it and you are only quoting the one moving it. I don't think the move matters too much by the way.
comment:46 in reply to: ↑ 44 ; follow-up: ↓ 47 Changed 5 years ago by
Replying to zimmerma:
concerning GF2X on FreeBSD (cf comment 16), does the following patch fix the issue?
--- a/configure.ac +++ b/configure.ac @@ -93,10 +93,10 @@ AC_ARG_ENABLE([fft-interface], AM_CONDITIONAL([ENABLE_FFT_INTERFACE],[test "x$enable_fft_interface" = xyes]) AC_PROG_CC -AC_PROG_CPP AC_PROG_CXX AC_COMPILE_WARNINGS AC_PROG_CC_C99 +AC_PROG_CPP GF2X_PROG_CC_FOR_BUILD GF2X_PROG_EXEEXT_FOR_BUILD
At present even with CPP and CXXCPP properly set, I still need
GF2X_CONFIGURE="--disable-hardware-specific-code $GF2X_CONFIGURE"
- otherwise I get non-sensical errors at tune stage, saying something along the lines of "this code can only be run on a 64-bit system" (see #22679, the last commit)
I was not able to figure out what goes wrong. Perhaps it's indeed this change that is needed to get the toolchain right, any idea?
comment:47 in reply to: ↑ 46 Changed 5 years ago by
Replying to dimpase:
I still need
GF2X_CONFIGURE="--disable-hardware-specific-code $GF2X_CONFIGURE"
- otherwise I get non-sensical errors at tune stage, saying something along the lines of "this code can only be run on a 64-bit system" (see #22679, the last commit)I was not able to figure out what goes wrong. Perhaps it's indeed this change that is needed to get the toolchain right, any idea?
Setting CPP wouldn't help I think. Really we would need the failure (compilation line+ error message) and config.log. One thing you could try is to pull the current git master and see if that works.
comment:48 Changed 5 years ago by
comment:49 Changed 5 years ago by
- Commit changed from a50d44f8da9bf9d1408e8cb945f07d227c884fb9 to f5657399ab53fcfe274dee50d0cf5d04bb83ae94
comment:50 Changed 5 years ago by
- Description modified (diff)
comment:51 follow-up: ↓ 53 Changed 5 years ago by
- Status changed from needs_review to needs_work
Why did you drop the checks like if [ -z "$CC" ]
when moving the code from sage-env
to sage-env-config
? It should still be possible to override $CC
at runtime.
comment:52 Changed 5 years ago by
Why AC_PROG_OBJC([clang])
instead of AC_PROG_OBJC()
? Doesn't autoconf
search for clang
by default? If not, you should at least add cc
to the search list (see the old sage-env
).
comment:53 in reply to: ↑ 51 ; follow-up: ↓ 54 Changed 5 years ago by
Replying to jdemeyer:
Why did you drop the checks like
if [ -z "$CC" ]
when moving the code fromsage-env
tosage-env-config
? It should still be possible to override$CC
at runtime.
Giving people the means to shoot themselves in the foot? I guess overriding CC
at runtime is relatively safe, CXX
and FC
not so much.
As for AC_PROG_OBJC
, I cannot find the default order anymore. It is only used by R on OS X, the idea was to cut the chase to what we really want there. But with such a short list all it does over just setting it is checking the presence. I guess I do overreach there.
comment:54 in reply to: ↑ 53 Changed 5 years ago by
Replying to fbissey:
I guess overriding
CC
at runtime is relatively safe,CXX
andFC
not so much.
I think that's not for you to decide. The autoconf philosophy is to have sensible defaults but always allow customizing everything. There will always be (obscure) cases where you would want to override some environment variable and it's not fun if you need to edit sage-env-config
for that.
comment:55 Changed 5 years ago by
It all depends where you put the balance between convenience and safety, and all that kind of stuff. At least I am sure you know the price for convenience.
comment:56 Changed 5 years ago by
Well, it has always been possible to customize $CC
on the fly and I don't think that this has led to many problems. So I see no reason to start disallowing it in this ticket.
comment:57 follow-up: ↓ 58 Changed 5 years ago by
To keep consistent with autotools packages, setting an environment variable such as CC should NOT override what has been configured using "configure".
(Rather, to override, a user would use make CC=....
.)
comment:58 in reply to: ↑ 57 ; follow-up: ↓ 59 Changed 5 years ago by
Replying to mkoeppe:
To keep consistent with autotools packages, setting an environment variable such as CC should NOT override what has been configured using "configure". (Rather, to override, a user would use
make CC=....
.)
I see your point. However, not all Sage package use make
, so this cannot work for Sage.
Alternatively, we need to ensure that make CC=...
really does work in all cases.
comment:59 in reply to: ↑ 58 ; follow-up: ↓ 60 Changed 5 years ago by
Replying to jdemeyer:
Replying to mkoeppe:
To keep consistent with autotools packages, setting an environment variable such as CC should NOT override what has been configured using "configure". (Rather, to override, a user would use
make CC=....
.)I see your point. However, not all Sage package use
make
, so this cannot work for Sage.
This unrelated to what build systems Sage packages use. They just use the environment prepared by Sage's top-level build scripts and Makefiles.
comment:60 in reply to: ↑ 59 Changed 5 years ago by
Replying to mkoeppe:
This unrelated to what build systems Sage packages use.
No, it's not. GNU make passes variables to sub-makes, so a top-level make CC=foo
will pass CC=foo
when running make
inside a specific package.
comment:61 follow-up: ↓ 62 Changed 5 years ago by
"because we always did..." is a terrible reason. But there is a point to both parties. If you want to be able to recompile a package with CC=foo ./sage -f bar
then it is very convenient. But then would make CC=foo bar
work instead? Of course, it will not work when we set a compiler in spkg-install
(and we do that quite a bit on OS X).
What I don't like about overriding the compilers at runtime (to use sage proper rather its packaging system) is that you can get in the situation of #21701 where for a while I had no clue what was happening. When you invoke sagelib, and you do some compilation with cython from sagelib, having the right compiler is rather critical. I don't like the idea of that compiler be able to be changed. It is an invitation to broken behavior.
comment:62 in reply to: ↑ 61 Changed 5 years ago by
Replying to fbissey:
"because we always did..." is a terrible reason.
Let me rephrase it as "because it always was possible". You are proposing to remove a feature: being able to change compilers at any time. If you remove a feature, there should be a good reason for that.
comment:63 follow-up: ↓ 64 Changed 5 years ago by
I presume we can add CC=
etc. as options to sage -i
and sage -f
. This would allow per package overriding. Does this go far enough to make everyone happy?
comment:64 in reply to: ↑ 63 Changed 5 years ago by
Replying to dimpase:
I presume we can add
CC=
etc. as options tosage -i
andsage -f
. This would allow per package overriding. Does this go far enough to make everyone happy?
Let's not bother with that. We could do a survey on whether or not people are using this "feature", that is purely accidental, on sage-devel. We have enough people that we would get a few yeses, even if it is just for the principle of the thing.
Driving changes through people's throat is usually not a good idea. The best ways are
- to offer something in exchange of greater value one way or another. The current offer is obviously not appropriate.
- let people figure themselves that things needs to be changed, usually for their own good.
In that context I am happy to provide the "shoot in the foot gun", if it ever become painful enough, someone else will drive the change. I don't expect it will happen anytime soon because the reality is that it is a very low use "feature" for people that don't mind taking it in the foot on a regular basis in the first place.
comment:65 follow-up: ↓ 66 Changed 5 years ago by
@ Jeroen question on implementation of said feature:
Currently (not this branch) an installed gcc installed by sage will be selected even if you provide an alternative compiler. Do you want to continue with that behavior or would you prefer things to be consistent and have a provided compiler always be the one used?
comment:66 in reply to: ↑ 65 ; follow-up: ↓ 67 Changed 5 years ago by
Replying to fbissey:
Currently (not this branch) an installed gcc installed by sage will be selected even if you provide an alternative compiler. Do you want to continue with that behavior or would you prefer things to be consistent and have a provided compiler always be the one used?
That's a good question. I think it's not a very relevant question, since the people who like to play with different compilers will usually not use the Sage-compiled GCC.
Still, for consistency, it makes more sense if an environment variable would override the compiler in all cases. So, the compiler should be determined by the following in decreasing order of priority:
- Environment variables
- Sage-installed GCC
./configure
setting
comment:67 in reply to: ↑ 66 ; follow-up: ↓ 68 Changed 5 years ago by
Replying to jdemeyer:
Replying to fbissey:
Currently (not this branch) an installed gcc installed by sage will be selected even if you provide an alternative compiler. Do you want to continue with that behavior or would you prefer things to be consistent and have a provided compiler always be the one used?
That's a good question. I think it's not a very relevant question, since the people who like to play with different compilers will usually not use the Sage-compiled GCC.
Still, for consistency, it makes more sense if an environment variable would override the compiler in all cases. So, the compiler should be determined by the following in decreasing order of priority:
- Environment variables
- Sage-installed GCC
./configure
setting
Exactly my opinion of what it should be for consistency, we are in agreement - but since that is changing the behavior and all, better check ;)
Now to the best way of achieving all that....
comment:68 in reply to: ↑ 67 Changed 5 years ago by
Replying to fbissey:
Replying to jdemeyer:
Replying to fbissey:
Currently (not this branch) an installed gcc installed by sage will be selected even if you provide an alternative compiler. Do you want to continue with that behavior or would you prefer things to be consistent and have a provided compiler always be the one used?
That's a good question. I think it's not a very relevant question, since the people who like to play with different compilers will usually not use the Sage-compiled GCC.
For people who use OSX it's not a play, it's matter of having working Sage, or not...
comment:69 Changed 5 years ago by
- Commit changed from f5657399ab53fcfe274dee50d0cf5d04bb83ae94 to f4d6748d5ae0826d0f30af78e68e627121d616d4
Branch pushed to git repo; I updated commit sha1. New commits:
f4d6748 | Change the setting of compilers in the order: environment, sage installed compilers, configured compilers.
|
comment:70 Changed 5 years ago by
- Commit changed from f4d6748d5ae0826d0f30af78e68e627121d616d4 to 4c41390b5340b250aed08628f1ee24b598a72319
comment:71 Changed 5 years ago by
- Status changed from needs_work to needs_review
New tarball will be up in the morning in my time zone. Otherwise back to review.
comment:72 Changed 5 years ago by
- Status changed from needs_review to needs_work
I think that this logic
# On OS X 10.7 (Lion) or later when building GCC, use clang as # default C compiler as some older versions of XCode 4 ship broken # versions of GCC which fail to bootstrap GCC-4.6.3. # See http://trac.sagemath.org/sage_trac/ticket/12820 if [ "$SAGE_BUILD_TOOLCHAIN" = yes ] && [ "$UNAME" = "Darwin" ] && [ "$MACOSX_VERSION" -ge 11 ; then CC=`which clang 2>/dev/null` CXX=`which clang++ 2>/dev/null`
should also be moved to configure
.
Unfortunately, autoconf 2.69 (which is still the latest release despite being 5 years old) does not support clang
.
So I suggest something along the lines of
if test -z "$CC"; then if test -n "$MACOSX_VERSION" && test "$MACOSX_VERSION" -ge 11 ; then AC_CHECK_TOOL(CC, clang) fi fi AC_PROG_CC()
comment:73 follow-ups: ↓ 74 ↓ 76 Changed 5 years ago by
I think that's just scope creep. But I am willing to make some concessions. In that particular case I think we could just remove that particular piece of code. It seems to go back to when apple was still shipping a "real" apple patched gcc rather than having it being a clang wrapper. I don't think we should continue to support such snowflakes. In fact I don't think we realistically support anything older than xcode 6 but most likely 7.
comment:74 in reply to: ↑ 73 ; follow-up: ↓ 75 Changed 5 years ago by
Replying to fbissey:
In fact I don't think we realistically support anything older than xcode 6 but most likely 7.
So you are saying that Sage 8.0 wouldn't build anyway on OS X 10.7 for example?
comment:75 in reply to: ↑ 74 Changed 5 years ago by
Replying to jdemeyer:
Replying to fbissey:
In fact I don't think we realistically support anything older than xcode 6 but most likely 7.
So you are saying that Sage 8.0 wouldn't build anyway on OS X 10.7 for example?
Depends which version of xcode you can put on there. But if you cannot install xcode 7, I'd say with difficulties. I could be wrong but I expect it wouldn't build out of the box. May be with a bit of hand holding. Do we know what versions we have on the build bots? A lot of the third party software binaries these days don't go further back than 10.9 in backward compatibility support. I think there are serious limitations in going further back.
comment:76 in reply to: ↑ 73 Changed 5 years ago by
Replying to fbissey:
It seems to go back to when apple was still shipping a "real" apple patched gcc rather than having it being a clang wrapper.
So, with XCode, the gcc
and clang
programs are exactly the same? That would indeed mean that there is no reason for the code in 72. Should we just remove it?
comment:77 Changed 5 years ago by
xcode 8:
Mirage:~ fbissey$ /usr/bin/gcc -v Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1 Apple LLVM version 8.1.0 (clang-802.0.42) Target: x86_64-apple-darwin16.6.0 Thread model: posix InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
xcode 7 and 6 were similar. I don't remember much before that. But yes you are now seeing it my way: removal seems a better idea.
comment:78 follow-ups: ↓ 79 ↓ 82 Changed 5 years ago by
IMHO we should drop OSX 10.6. On the other hand, Apple continues to sell OSX 10.7---it's probably the oldest system they currently support.
comment:79 in reply to: ↑ 78 ; follow-up: ↓ 80 Changed 5 years ago by
Replying to dimpase:
IMHO we should drop OSX 10.6. On the other hand, Apple continues to sell OSX 10.7---it's probably the oldest system they currently support.
Really??? Is it just an upgrade path or is it for "vintage" hardware?
comment:80 in reply to: ↑ 79 Changed 5 years ago by
Replying to fbissey:
Replying to dimpase:
IMHO we should drop OSX 10.6. On the other hand, Apple continues to sell OSX 10.7---it's probably the oldest system they currently support.
Really??? Is it just an upgrade path or is it for "vintage" hardware?
the latter, I think---one can buy an official DVD: https://www.apple.com/uk/shop/product/D6106ZM/A/os-x-lion
comment:81 Changed 5 years ago by
Could be an upgrade path as well. You need at least 10.7 to have the app store through which you can install OS X upgrades. I think you cannot do that from 10.6, you need a media or at the very least a .dmg
. I had an old imac like that for which I would have needed the media (it's dead now, the gpu died).
Or I could have my versions mixed up.
comment:82 in reply to: ↑ 78 Changed 5 years ago by
IMHO we should drop OSX 10.6. On the other hand, Apple continues to sell OSX 10.7---it's probably the oldest system they currently support.
Given that people with hardware for 10.6 can probably upgrade to 10.7, this might seem reasonable.
comment:83 Changed 5 years ago by
I just checked that one can still get official OSX 10.6 just as well... (but no 10.5). 10.6 has a feature missing in 10.7, it can run PPC applications. Other than that, I don't see why people would want to stick with it.
comment:84 Changed 5 years ago by
- Commit changed from 4c41390b5340b250aed08628f1ee24b598a72319 to 38273c6c529d844cceea62e57bbbb993e2000515
Branch pushed to git repo; I updated commit sha1. New commits:
38273c6 | Remove bits specific to outdated xcode toolchain
|
comment:85 Changed 5 years ago by
- Commit changed from 38273c6c529d844cceea62e57bbbb993e2000515 to ab61da6c3615e09e1b3ad8aedbe64a8e019b1060
Branch pushed to git repo; I updated commit sha1. New commits:
ab61da6 | update configure tarball
|
comment:86 Changed 5 years ago by
- Status changed from needs_work to needs_review
OK so removed that old cruft for now. May be we'll discover one of the bots still has a very old version of OS X.
comment:87 Changed 5 years ago by
- Commit changed from ab61da6c3615e09e1b3ad8aedbe64a8e019b1060 to 58208125c2616a6b2d027b8573db65c3844c9c82
Branch pushed to git repo; I updated commit sha1. New commits:
5820812 | Merge branch 'develop' into compiler and bump configure tarball
|
comment:88 Changed 5 years ago by
- Description modified (diff)
comment:89 follow-ups: ↓ 90 ↓ 91 Changed 5 years ago by
Two comments:
1) Strong objection to mixing an autoconf feature with sage-specific behavior (environment CC overrides configured CC), which makes it different from any other autoconf-based package. It is an important feature that the configured variables stick in the build and that this works even in the presence of some polluted environment.
If it is really necessary to make overriding possible using environment settings, you can invent new variables such as SAGE_OVERRIDE_CC
etc.
2) I would suggest to only set variables in sage-env-config
and keep all logic (if-then) in sage-env
. For example:
SAGE_CONFIGURED_CC="@CC@"
comment:90 in reply to: ↑ 89 ; follow-ups: ↓ 92 ↓ 93 Changed 5 years ago by
Replying to mkoeppe:
Two comments:
1) Strong objection to mixing an autoconf feature with sage-specific behavior (environment CC overrides configured CC), which makes it different from any other autoconf-based package. It is an important feature that the configured variables stick in the build and that this works even in the presence of some polluted environment.
If it is really necessary to make overriding possible using environment settings, you can invent new variables such as
SAGE_OVERRIDE_CC
etc.
No. I just tried an autotooled package (gf2x), you can configure with one compiler and over-ride it at build time with CC
and co. However I discourage anyone to over-ride it at runtime if you need to compile cython
files from inside sage. That's my usual dichotomy problem between sage the packaging stack and sage, the python runtime on which you do your math.
2) I would suggest to only set variables in
sage-env-config
and keep all logic (if-then) insage-env
. For example:SAGE_CONFIGURED_CC="@CC@"
As the requester on this ticket I wish you had expressed yourself earlier on that particular design. It is a fine idea to confine sage-env-config
to be a variable store without any logic in it. I just wish I had that requirement earlier. I had the first draft of the current design up 2 months ago and really opened to criticism 7 weeks ago.
comment:91 in reply to: ↑ 89 Changed 5 years ago by
Replying to mkoeppe:
Strong objection to mixing an autoconf feature with sage-specific behavior (environment CC overrides configured CC), which makes it different from any other autoconf-based package.
I don't consider Sage to be a "package" (it's a distribution of packages), so I don't see it as an issue that it behaves differently from other packages. It's comparing an apple with oranges. It has always been the case in Sage that a CC
environment variable overrides the default CC
and nobody has ever considered this a problem.
If it is really necessary to make overriding possible using environment settings, you can invent new variables such as
SAGE_OVERRIDE_CC
etc.
In that case, we should simply support make CC=...
from the top-level. But that will require some additional work.
comment:92 in reply to: ↑ 90 Changed 5 years ago by
Replying to fbissey:
Replying to mkoeppe:
1) Strong objection to mixing an autoconf feature with sage-specific behavior (environment CC overrides configured CC), which makes it different from any other autoconf-based package. It is an important feature that the configured variables stick in the build and that this works even in the presence of some polluted environment.
No. I just tried an autotooled package (gf2x), you can configure with one compiler and over-ride it at build time with
CC
and co.
You don't say what exactly you tried. My point is that you can override by using makefile variables like this:
make CC=xyzzy
but you cannot override using environment variables:
CC=xyzzy make
Any implementation for Sage should copy this behavior.
comment:93 in reply to: ↑ 90 Changed 5 years ago by
Replying to fbissey:
As the requester on this ticket I wish you had expressed yourself earlier on that particular design. It is a fine idea to confine
sage-env-config
to be a variable store without any logic in it. I just wish I had that requirement earlier. I had the first draft of the current design up 2 months ago and really opened to criticism 7 weeks ago.
Right, and I wish I had more time for Sage development lately. In any case, this one is just a suggestion. If #21707 is still something that we want to do, we can change the structure of these scripts at that time.
comment:94 Changed 5 years ago by
@ mkoeppe I guess we are all there in regards to time [in fact I started writting this answer more than 24 hours ago now...]. I was just venting.
I have no vested interest in #21707. If anything I think this is misguided at this point. I think some other work like the decoupling of "sagelib" from the packaging system should be done first and would be beneficial to some of what you seek in #21707 in the long run (personal opinion).
I will turn sage-env-config
in a variable store in this ticket. It is not going to be included but could be material early in the 8.1 cycle.
On that note I will leave the current behivior for over-riding CC
. I don't want to be part of that debate more than is necessary. Feel free to open a ticket to get the behavior you want, but I think you need to get traction from sage-devel first, since there is opposition.
comment:95 Changed 5 years ago by
- Commit changed from 58208125c2616a6b2d027b8573db65c3844c9c82 to 79d0963576906f01449ba9b4d3ad4e307b8523b6
Branch pushed to git repo; I updated commit sha1. New commits:
79d0963 | Remove compiler selecting logic from sage-env-config and put it back in sage-env.
|
comment:96 Changed 5 years ago by
- Milestone changed from sage-8.0 to sage-8.1
comment:97 Changed 5 years ago by
- Commit changed from 79d0963576906f01449ba9b4d3ad4e307b8523b6 to 1015a1fc8fa01cfcbb8faaecd8348682ed72491c
Branch pushed to git repo; I updated commit sha1. New commits:
1015a1f | Merge branch 'master' into compiler and configure tarball bump
|
comment:98 Changed 5 years ago by
- Description modified (diff)
comment:99 Changed 5 years ago by
From looking at the latest patch, $
seems to be missing in front of CONFIGURED_CXX
etc. in several places in src/bin/sage-env
. (Haven't tested.)
comment:100 Changed 5 years ago by
- Status changed from needs_review to needs_work
comment:101 Changed 5 years ago by
- Commit changed from 1015a1fc8fa01cfcbb8faaecd8348682ed72491c to c4fce28f6568eadb16c161ed9a5d02ba17a8229e
Branch pushed to git repo; I updated commit sha1. New commits:
c4fce28 | fix missing "$" in sage-env
|
comment:103 Changed 5 years ago by
- Commit changed from c4fce28f6568eadb16c161ed9a5d02ba17a8229e to bbee7b8529268fe8f8721d098a9f677030d456c2
Branch pushed to git repo; I updated commit sha1. New commits:
bbee7b8 | Merge branch 'develop' into compiler and bump configure tarball
|
comment:104 Changed 5 years ago by
- Description modified (diff)
comment:105 follow-up: ↓ 106 Changed 5 years ago by
It would be nice to get this merged soon, so it is early in the 8.1 release cycle. What are the obstacles to this?
comment:106 in reply to: ↑ 105 ; follow-up: ↓ 108 Changed 5 years ago by
Replying to jhpalmieri:
It would be nice to get this merged soon, so it is early in the 8.1 release cycle. What are the obstacles to this?
Possibly bikering over what behavior it should enforce. Currently CC=.. ./sage -f ...
or CC=... make ....
will over-ride the configure time compiler. Some people think it shouldn't, only make ... CC=...
should.
comment:107 Changed 5 years ago by
But be my guest in reviewing this.
comment:108 in reply to: ↑ 106 Changed 5 years ago by
Replying to fbissey:
Possibly bikering over what behavior it should enforce. Currently
CC=.. ./sage -f ...
orCC=... make ....
will over-ride the configure time compiler. Some people think it shouldn't, onlymake ... CC=...
should.
Would a simple renaming of the the configure parameters from CC=
to --c-compiler=
, etc, alleviate at least some of concerns (unless there is some hidden magic in naming the parameter CC
---I hope not)? Personally, every time I do ./configure CC=...
I get deja vu---maybe it's just me though.
comment:109 follow-up: ↓ 110 Changed 5 years ago by
I am not sure what that would achieve. AC_PROG_CC
makes sure that CC, which is used in standard makefile rules is set to something sensible - by going through a list or checking that the current setting of CC is sane. You may have deja vu because you haven't been on a system with a variety of compilers available out of the box and where you often wants something different from the default.
If you are going to use something non-standard like --c-compiler=
I am almost tempted to tell you not to use it to set CC
either. But then you lose some of the work AC_PROG_CC
does for you [test that the compiler work, if it is gcc (real or fake)].
comment:110 in reply to: ↑ 109 ; follow-up: ↓ 111 Changed 5 years ago by
Replying to fbissey:
I am not sure what that would achieve.
AC_PROG_CC
makes sure that CC, which is used in standard makefile rules is set to something sensible - by going through a list or checking that the current setting of CC is sane. You may have deja vu because you haven't been on a system with a variety of compilers available out of the box and where you often wants something different from the default.
I certainly have been there and wanted that, my point is that having CC=...
as a parameter to a script (rather than an environment variable) is akin to having two girlfriends with the same given name---sometimes convenient, but sometimes making you dazed and confused... ;-)
If you are going to use something non-standard like
--c-compiler=
I am almost tempted to tell you not to use it to setCC
either. But then you lose some of the workAC_PROG_CC
does for you [test that the compiler work, if it is gcc (real or fake)].
I don't see what you mean here. E.g. in configure.ac
one can probably do something like
CCORIG=gcc AC_SUBST(CC,$CCORIG) AC_ARG_WITH(cc,AS_HELP_STRING(--with-cc=...,specify the full path to the C compiler ($CC)),CC=$withval) AC_PROG_CC() ... AC_ARG_WITH(cxx,...
to create ./configure
arguments such as --with-cc=
, and set CC
, etc to the right values.
comment:111 in reply to: ↑ 110 ; follow-ups: ↓ 112 ↓ 113 Changed 5 years ago by
Replying to dimpase:
Replying to fbissey:
I am not sure what that would achieve.
AC_PROG_CC
makes sure that CC, which is used in standard makefile rules is set to something sensible - by going through a list or checking that the current setting of CC is sane. You may have deja vu because you haven't been on a system with a variety of compilers available out of the box and where you often wants something different from the default.I certainly have been there and wanted that, my point is that having
CC=...
as a parameter to a script (rather than an environment variable) is akin to having two girlfriends with the same given name---sometimes convenient, but sometimes making you dazed and confused... ;-)
You choose to see it as a parameter because it is put at the end of the line. But really it is just setting that environment variable while running that command. You could put it in front with the same result, the main point of putting it in a "parameter" position is that it is captured in config.log
. But whether it is passed as, a parameter, a single shot environment variable or exported from the environment, it all give the same result.
There is no duality, it is always an environment variable. And setting it on that command line, either in front, or in parameter position, is all about not polluting your main environment with the setting in question.
But my feeling is that this is beside the point of this ticket.
comment:112 in reply to: ↑ 111 Changed 5 years ago by
Replying to fbissey:
It did indeed escape me that CC=
is not a script parameter, sorry. No objections then.
comment:113 in reply to: ↑ 111 ; follow-up: ↓ 114 Changed 5 years ago by
Replying to fbissey:
You could put it in front with the same result, the main point of putting it in a "parameter" position is that it is captured in
config.log
.
... and config.status
, of course, so that the setting persists across config.status --recheck
etc.
For positive review, I would like to see:
- the difference in behavior to standard autotools (comment 92) documented somewhere
- a follow-up ticket for making the behavior equal to standard autotools should be created
- the ticket summary should be updated (support "for all
SAGE_...
variables" is not implemented in the branch) - follow-up ticket?
comment:114 in reply to: ↑ 113 Changed 5 years ago by
Replying to mkoeppe:
Replying to fbissey:
You could put it in front with the same result, the main point of putting it in a "parameter" position is that it is captured in
config.log
.... and
config.status
, of course, so that the setting persists acrossconfig.status --recheck
etc.For positive review, I would like to see:
- the difference in behavior to standard autotools (comment 92) documented somewhere
Yes, certainly some documentation is warranted. Note that since "sage" is not an autotool "product", I have in fact no problem with CC=... ./sage
changing the compiler. It is consistent with how I am changing the environment for one off runs on the command line for many applications. Example OMP_NUM_THREADS=4 ./my_openmp_app
. May be I am wrong to put it in prefix position and putting it postfix would work, but I wasn't taught any better.
- a follow-up ticket for making the behavior equal to standard autotools should be created
I can create a follow up ticket and copy you onto it but I probably won't do any work on it. I will leave that to someone else.
- the ticket summary should be updated (support "for all
SAGE_...
variables" is not implemented in the branch) - follow-up ticket?
I completely forgot about those I must say. This is more complicated as we will, at the very least, need to generate an exhaustive list of those variables, preferably automatically. Definitely another ticket.
comment:115 Changed 5 years ago by
Actually, while I may add a bit of documentation on configuration, the current documentation is quite explicit about how CC
and others is used
- :envvar:`CC` - while some programs allow you to use this to specify your C compiler, **not every Sage package recognizes this**. If GCC is installed within Sage, :envvar:`CC` is ignored and Sage's ``gcc`` is used instead.
When you have to use a variable in postfix position it is not treated as an environment variable. I may add anote that this differs from the behavior you expect from autotools.
comment:116 Changed 5 years ago by
- Description modified (diff)
comment:117 Changed 5 years ago by
- Commit changed from bbee7b8529268fe8f8721d098a9f677030d456c2 to e485a75b7c89bb283b82db7a30583dbb3227270e
Branch pushed to git repo; I updated commit sha1. New commits:
e485a75 | Merge branch 'develop' into compiler and bump configure tarball
|
comment:118 Changed 5 years ago by
- Description modified (diff)
comment:119 Changed 5 years ago by
Progress on the documentation?
comment:120 Changed 5 years ago by
I have a draft somewhere. I am just on too spread out.
comment:121 Changed 5 years ago by
- Commit changed from e485a75b7c89bb283b82db7a30583dbb3227270e to a8c8904577f481f82c63e9354afdf7aecc743f87
comment:122 Changed 5 years ago by
- Description modified (diff)
Ok rebased and added some documentation. I don't think it is particularly great feedback appreciated.
Changed 5 years ago by
comment:123 Changed 5 years ago by
- Description modified (diff)
comment:124 Changed 5 years ago by
A few minor suggestions for the documentation: in the first sentence, it is ambiguous about which variables need to be specified just for OS X. Maybe something like this instead:
Some environment variables deserve a special mention:
CC
,CXX
, andFC
; and on OS X,OBJC
andOBJCXX
.
Also, change "over-riden" to "over-ridden" and change "sage" to "Sage" (twice).
Otherwise, I don't have anything to add to it; I think it's fine.
comment:125 Changed 5 years ago by
- Commit changed from a8c8904577f481f82c63e9354afdf7aecc743f87 to 99eb31f851a4fcdf1e17bd1671b5218ae9acc0be
Branch pushed to git repo; I updated commit sha1. New commits:
99eb31f | Minor edits from John Palmieri's feedback
|
comment:126 Changed 5 years ago by
OK, revision included. It is ready for someone to push the positive review button.
comment:127 Changed 5 years ago by
- Reviewers set to Matthias Koeppe, Jeroen Demeyer, Vincent Delecroix, John Palmieri, Dima Pasechnik
- Status changed from needs_review to positive_review
comment:128 follow-ups: ↓ 129 ↓ 130 Changed 5 years ago by
Great! Now on to #12426?
comment:129 in reply to: ↑ 128 Changed 5 years ago by
Replying to jhpalmieri:
Great! Now on to #12426?
yes, sure. I'll test it for building gfortran on FreeBSD.
comment:130 in reply to: ↑ 128 ; follow-up: ↓ 131 Changed 5 years ago by
Replying to jhpalmieri:
Great! Now on to #12426?
Thanks everyone!
For #12426. It would be nice to have it in 8.1 but some people may feel the shift warrants it to go first thing in 8.2.beta0. We could take the debate to sage-devel. Things needed in my opinion for #12426:
- new release of eclib - that won't solve all the problems but working with clang lead to improvement of the code.
- update and fix ccache
- a good discussion about the giac test
comment:131 in reply to: ↑ 130 Changed 5 years ago by
Replying to fbissey:
Replying to jhpalmieri:
Great! Now on to #12426?
Thanks everyone!
For #12426. It would be nice to have it in 8.1 but some people may feel the shift warrants it to go first thing in 8.2.beta0. We could take the debate to sage-devel. Things needed in my opinion for #12426:
- new release of eclib - that won't solve all the problems but working with clang lead to improvement of the code.
eclib already took advantage of clang, it helped finding a couple of ugly memory leaks... I think it can be done after this ticket - working without a properly supported toolchain is tricky.
- update and fix ccache
Why is this so urgent, and again, such things are harder to fix without a properly supported toolchain.
- a good discussion about the giac test
What is this about? giac certainly would benefit from more uniform following c++11 or/and gnu++11...
- How about clang on Linux? Are we aiming at it now here? Or, again, this could wait for another ticket? (I'd rather do the latter; it's after all not that urgent).
comment:132 Changed 5 years ago by
ccache
is not urgent. While it is not a blocker we should aim at supporting a maximum of optional packages out of the box. So fixing the known problems make sense.
More urgent is doing all that is possible for standard packages that have a testsuite to pass it with clang. This won't happen at this stage for eclib
and giac
. eclib
because it is hard, giac
because there is currently a test that orders results differently depending on the compiler used :( - we would need a patching option in spkg-check to fix this kind of case and it would need a little extra infrastructure.
clang on linux is just a nice side effect :) and the last ticket for it to work once #12426 is merged is in.
Note that #12426 makes it possible to use icc
and it works, at say ~95%, out of the box for standard sage. sqlite needs a fix that belongs upstream and pari optimisation needs to be tuned down on top of my head.
comment:133 Changed 5 years ago by
IMHO, this all should be dealt with on follow-up tickets, and #12426 ought to get in without them.
comment:134 Changed 5 years ago by
- Branch changed from u/fbissey/compilers to 99eb31f851a4fcdf1e17bd1671b5218ae9acc0be
- Resolution set to fixed
- Status changed from positive_review to closed
comment:135 Changed 5 years ago by
- Commit 99eb31f851a4fcdf1e17bd1671b5218ae9acc0be deleted
Follow-up: #23789
I guess that should be extended to
CXX
,FC
, possiblyF77
andCPP
(although I actually don't recommend to set that one and let the package decides unless it require hand holding. That's from my experience setting it insage-env
that sometimes backfire because$CC -E
may work slightly differently and is what many package assume instead of plaincpp
.Would that need to over-ride
sage-env
if sage's gcc has been installed? Useful for mad scientist experiments but probably not for common users.