Generic mechanism for system package checks at configure time
embray opened this issue · 150 comments
Here's a look at the work I've been doing on top of (albeit not completely dependent on) #21524 in order to better support use of system packages for any of Sage's SPKGs.
This takes the workarounds that are already hard-coded in Sage's configure.ac for detecting certain packages--notably gcc/gfortran, yasm, git, and and curl--and converts those to a more generic mechanism.
In the new mechanism each SPKG may have a file in its SPKG directory named spkg-configure.m4. The ./bootstrap script gathers these all up and includes them into configure.
Each spkg-configure.m4 should call a macro called SAGE_SPKG_CONFIGURE which is passed the package name, and some configure-time checks it should perform for that package. See the documentation in spkg-configure.m4 for more details on that macro. Inside these checks, they should set either sage_spkg_install_<pkgname>=yes|no in order to track whether or not Sage should install that package, or if we can just the version from the system (or we don't require the package at all for the current platform).
We then move the hard-coded bits for yasm, gcc, etc. out of configure.ac and into their individual spkg-configure.m4 files. The end result is the same in terms of the checks that are performed.
As proof of concept we also add a configure-time check for the system's patch, demonstrating how this might be used to enable support for more system packages.
See #26286 for a follow-up.
Depends on #21524
Depends on #25011
Depends on #25208
Depends on #25188
Depends on #25198
CC: @mezzarobba @dimpase
Component: build
Author: Erik Bray
Branch: 79de3bd
Reviewer: John Palmieri, Dima Pasechnik
Issue created by migration from https://trac.sagemath.org/ticket/24919
It should be noted that, of course, each time a package's spkg-configure.m4 is added or modified we must re-run ./bootstrap, same as if we were just modifying configure.ac directly.
One other aspect of this that probably needs work before it's ready, is to add flags for explicitly selecting whether or not to use the system's version of some package.
I would definitely prefer to not install SPKGs by default we don't have to. But we could also add a global flag like "always use SPKGs regardless of configure checks" or something.
Replying to @embray:
I would definitely prefer to not install SPKGs by default we don't have to.
I agree. That's also the current behaviour for gcc, git, ...
Replying to @jdemeyer:
Replying to @embray:
I would definitely prefer to not install SPKGs by default we don't have to.
I agree. That's also the current behaviour for gcc, git, ...
Indeed. I don't know if anyone would have a problem with this or not. I think most people would consider it an enhancement if Sage installed fewer dependencies by default :) I guess it mostly comes down to how reliable the system versions of those dependencies are and how well they integrate with Sage.
I'm concerned about using GMP/MPIR from the system--not because it wouldn't work, but because of how Sage currently installs MPIR as a synonym for GMP. Many platforms don't have an MPIR package in the first place, so I guess they would just use GMP. I'm not sure how best to handle that one.
Replying to @embray:
I'm concerned about using GMP/MPIR from the system--not because it wouldn't work, but because of how Sage currently installs MPIR as a synonym for GMP. Many platforms don't have an MPIR package in the first place, so I guess they would just use GMP. I'm not sure how best to handle that one.
Sage can use both MPIR and GMP. Of course, you'll need to check whether the MPIR/GMP version is sufficiently recent.
The branch doesn't merge.
Replying to @jdemeyer:
Replying to @embray:
I'm concerned about using GMP/MPIR from the system--not because it wouldn't work, but because of how Sage currently installs MPIR as a synonym for GMP. Many platforms don't have an MPIR package in the first place, so I guess they would just use GMP. I'm not sure how best to handle that one.
Sage can use both MPIR and GMP. Of course, you'll need to check whether the MPIR/GMP version is sufficiently recent.
I know. I think my concern here was just that Sage builds MPIR with its --enable-gmpcompat flag. So if some system has both GMP and MPIR installed side-by-side, Sage can only use the GMP on that system. And if some system has only MPIR and not GMP somehow, then it won't work because we use <include gmp.h>, etc. I'm not sure if that's really a problem though so maybe it's premature to worry about.
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
9ff40ba | Add the ability to move package-specific configure-time checks ( including |
0d3e713 | As an initial demonstration of the new system, move configuration |
f656da2 | Move the curl system package check to its own spkg-configure.m4 |
6e1ebc1 | Move the git system package check to its own spkg-configure.m4 |
32a7067 | Move the gcc/gfortran system package checks to their own spkg-configure.m4 |
a0e4078 | Slightly rewrote this macro to use some more polymorphic shell macros. |
d66e115 | Use m4_sinclude instead--this prevents problems when switching between branches where one of these listed files no longer exists |
d5ce869 | Move these comments inside the macro call to ensure they are actually expanded as part of the macro |
f9ab45d | Changed a bit how SAGE_SPKG_CONFIGURE works: |
070da0b | Implement an spkg-configure.m4 for patch, demonstrating use of the new system |
Rebased, but I think some other cleanup of configure.ac is now needed as a prerequisite (e.g. #25011).
By the way, I have curl headers installed in /usr/local/include/curl/curl.h, and
their recognition is broken, even if I set CFLAGS to have -I/usr/local/include and try to pass them to ./configure.
Replying to @dimpase:
By the way, I have
curlheaders installed in/usr/local/include/curl/curl.h, and
their recognition is broken, even if I set CFLAGS to have-I/usr/local/includeand try to pass them to./configure.
Dima, does that work without this ticket, or did this break it somehow?
The way this ticket does the feature test for curl is not substantively different from how it was done previously--it's just been moved to a different file. But if it did work before then I'm sure there is a real bug. Note, the correct curl-config also needs to be on the PATH, and needs to support curl-config --checkfor 7.22.
It didn't work before either, I think, but in fact curl-config is on the PATH,
and even tells you about the compiler flags:
$ curl-config --cflags
-I/usr/local/include
and the version seems good too:
$ curl-config --checkfor 7.22
$ curl-config --checkfor 999
requested version 999 is newer than existing 7.59.0
$ curl-config --version
libcurl 7.59.0
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
a1e5ea4 | Slightly rewrote this macro to use some more polymorphic shell macros. |
135efb4 | Use m4_sinclude instead--this prevents problems when switching between branches where one of these listed files no longer exists |
afc5572 | Move these comments inside the macro call to ensure they are actually expanded as part of the macro |
a7125f4 | Changed a bit how SAGE_SPKG_CONFIGURE works: |
3f64c89 | Implement an spkg-configure.m4 for patch, demonstrating use of the new system |
7e13822 | Revert "Trac #25113: OSX is f*ed up sometimes" |
2146433 | Fixes miscompilation by clang from XCode 6.3, see #25118 |
ab26648 | gfan version bump |
13619ff | move configure checks for OSX compatibility into a separate macro |
87d0d31 | Merge branch 'u/embray/build-configure/darwin-macro' into u/embray/build/autoconf-macros |
Replying to @dimpase:
It didn't work before either, I think, but in fact
curl-configis on the PATH,
and even tells you about the compiler flags:$ curl-config --cflags -I/usr/local/includeand the version seems good too:
$ curl-config --checkfor 7.22 $ curl-config --checkfor 999 requested version 999 is newer than existing 7.59.0 $ curl-config --version libcurl 7.59.0
Thanks. If it wasn't working before this ticket either then I wouldn't be inclined to address it here, since this is just moving around the existing code. I would open a separate issue for that. Though it seems like it should work if you run it like ./configure CFLAGS="-I/usr/local/include".
I'll see if I can set up a way to test that myself. Also if there are problems with the C compiler itself many other configure-time checks won't succeed at first.
Seems to have a bug since the last rebase...
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5b833a2 | Move the gcc/gfortran system package checks to their own spkg-configure.m4 |
c0b2750 | Slightly rewrote this macro to use some more polymorphic shell macros. |
751a515 | Use m4_sinclude instead--this prevents problems when switching between branches where one of these listed files no longer exists |
9249861 | Move these comments inside the macro call to ensure they are actually expanded as part of the macro |
4a7953a | Changed a bit how SAGE_SPKG_CONFIGURE works: |
6433ef7 | Implement an spkg-configure.m4 for patch, demonstrating use of the new system |
9df2996 | Revert "Trac #25113: OSX is f*ed up sometimes" |
b6f2947 | Fixes miscompilation by clang from XCode 6.3, see #25118 |
45b4357 | gfan version bump |
862d8e4 | move configure checks for OSX compatibility into a separate macro |
that's better, at least make distclean succeeds after ./bootstrap...
Replying to @embray:
Replying to @dimpase:
It didn't work before either, I think, but in fact
curl-configis on the PATH,
and even tells you about the compiler flags:$ curl-config --cflags -I/usr/local/includeand the version seems good too:
$ curl-config --checkfor 7.22 $ curl-config --checkfor 999 requested version 999 is newer than existing 7.59.0 $ curl-config --version libcurl 7.59.0Thanks. If it wasn't working before this ticket either then I wouldn't be inclined to address it here, since this is just moving around the existing code. I would open a separate issue for that. Though it seems like it should work if you run it like
./configure CFLAGS="-I/usr/local/include".
no, it doesn't work this way, at least not with clang on FreeBSD. I've opened
#25214
(I guess it differs from one compiler to the other whether /usr/local/include is searched by default or not)
By the way, on FreeBSD patch is BSD's patch, with a different versioning scheme, whereas GNU patch is gpatch. I wonder whether there is a way to deal with this easily.
Replying to @dimpase:
By the way, on FreeBSD
patchis BSD's patch, with a different versioning scheme, whereas GNU patch is gpatch. I wonder whether there is a way to deal with this easily.
Right now it strictly requires GNU patch. In the past there was some reason for that in terms of some features that were needed, or possibly bugs, but I'd be surprised if that still really matters, especially since patching of spkgs was refactored...
Replying to @dimpase:
that's better, at least
make distcleansucceeds after./bootstrap...
Does it get any further than that? :)
Thanks for testing this out btw.
Replying to @embray:
Replying to @dimpase:
that's better, at least
make distcleansucceeds after./bootstrap...Does it get any further than that? :)
Thanks for testing this out btw.
I was working on #23353 and messed things up while manually merging your branch.
Namely, now
git pull trac public/gfan/spkg_check
does not bring me the branch from #23353 back, and I wonder whether there is a painless way to force this... Do I need to locally rebase this branch somehow?
never mind, it builds, and skips patch, so this looks good so far.
(I have a soft link patch in PATH, pointing to gpatch, so this is a dirty way to avoid bumping into BSD patch on FreeBSD)
Would be wonderful if it can handle #25158 (cmake which is an optional package).
as well as
- autotools
- boost
- bzip2
- freetype
- gc
- gmp
- gsl
- iconv
- pcre
- r
- readline
- scons
- xz
- zlib
(this is still avoiding more python-related and maths-related packages...)
Replying to @dimpase:
as well as
- autotools
- boost
- bzip2
- freetype
- gc
- gmp
- gsl
- iconv
- pcre
- r
- readline
- scons
- xz
- zlib
(this is still avoiding more python-related and maths-related packages...)
Maybe not autotools, since it's an optional package, and the main point of sage's autotools package was installing lots of different autoconf/automake versions for building patches to different package's configure scripts.
The rest though yeah--all of these kind of low level system libraries are the highest priority to add support for (add ncurses to that list--ncurses takes forever and we have to build it twice).
But first I want to get the general mechanism in place before adding support for a bunch more packages.
Work Issues: merge conflicts
I could have sworn I rebased this recently. Maybe it was just on my todo list.
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
b6ad44b | As an initial demonstration of the new system, move configuration |
5ce5577 | Move the curl system package check to its own spkg-configure.m4 |
5668e2d | Move the git system package check to its own spkg-configure.m4 |
42617d3 | Move the gcc/gfortran system package checks to their own spkg-configure.m4 |
1c93c29 | Slightly rewrote this macro to use some more polymorphic shell macros. |
bcbeddc | Use m4_sinclude instead--this prevents problems when switching between branches where one of these listed files no longer exists |
742eaed | Move these comments inside the macro call to ensure they are actually expanded as part of the macro |
86d87da | Changed a bit how SAGE_SPKG_CONFIGURE works: |
0ac110b | Implement an spkg-configure.m4 for patch, demonstrating use of the new system |
e1e68ee | move configure checks for OSX compatibility into a separate macro |
Changed work issues from merge conflicts to none
I believe this issue can reasonably be addressed for Sage 8.4.
Branch pushed to git repo; I updated commit sha1. New commits:
f14f85b | Incorporate fixes from #25188 |
This is still ready go to. Most of the dependencies are listed as dependencies just because they would conflict with this. Or rather, because this moves around code from configure.ac, it also needs to incorporate other fixes I've made that also make changes to configure.ac.
But if any of those tickets remain controversial or whatever I can remove them from this ticket and deal with them later.
The substance of this ticket ticket is in providing support for build/<pkg>/spkg-configure.m4 files, providing configure-time checks for individual packages.
Why is it in "needs work" status?
Good work! What happens if the system package is updated to an incompatible version or even removed?
Replying to @timokau:
Good work! What happens if the system package is updated to an incompatible version or even removed?
Certainly there's nothing one can do about that; that's a problem that exists for any software. The best you can do is that if you notice something breaks to try re-running ./configure. Depending on the exact nature of the checks for a given package, that still might not tell you anything, but the eventual result of debugging such a problem would be either a workaround for said incompatible version or changing the range of package versions that are considered compatible.
It is also still possible to force installation of the spkg. Though one thing I think this might need, either in this ticket or in a followup, is a way to automatically add --with- flags for selecting the spkg over the system package or vice-versa.
My thinking was something like --with-curl, where the option can take an optional argument such as --with-curl=<something>, where <something> could technically be anything depending on the package, and it would be up to the package's spkg-configure.ac to determine what to do based on the argument. Without the argument I think the default should be a special value like "spkg" meaning --with-curl=spkg or install cURL from the SPKG.
In other words, --with-curl would mean, "configure Sage to include Sage's curl".
That's just one idea...there are other ways one could do this that might be better. That's just my current line of thinking.
after pulling and running bootstrap I get
checking for GNU patch >= 2.7.0... /usr/bin/patch
checking for curl 7.22... no
checking for git... /usr/bin/git
./configure: line 7948: syntax error near unexpected token `elif'
./configure: line 7948: ` elif test x$SAGE_INSTALL_GCC = xno; then'
This must be one of autoconf/m4 SNAFUs: these errors come from fragments such as
if ...
# ...
# true
elif...
whereas true is not commented out in original m4 files...
in fact, the following fixes configure:
--- a/build/pkgs/gcc/spkg-configure.m4
+++ b/build/pkgs/gcc/spkg-configure.m4
@@ -6,7 +6,7 @@ dnl In the latter case, a warning is given.
AC_DEFUN([SAGE_SHOULD_INSTALL_GCC], [
if test x$SAGE_INSTALL_GCC = xexists; then
# Already installed in Sage, but it should remain selected
- # true
+ true
elif test x$SAGE_INSTALL_GCC = xno; then
AC_MSG_WARN([$1])
else
I also saw build/pkgs/curl/spkg-configure.m4 not working due to bc not installed.
So bc should be added to build pre-reqs.
I've tried creating more spkg-configure.m4. I'm attaching one for xz. Not sure it's OK to use cut, but here it goes.
Attachment: spkg-configure.m4.gz
xz spkg-configure.m4
I am not sure how to deal with libraries; for some there are autoconf macros, e.g.
AX_CHECK_ZLIB.
Still, extra work for zlib would be needed to extract the version. How much can one rely
on pkgconf/pkg-config ?
Replying to @dimpase:
I've tried creating more
spkg-configure.m4. I'm attaching one forxz. Not sure it's OK to usecut, but here it goes.
This m4 file will also need proper check for liblzma being installed with headers and all.
Replying to @dimpase:
after pulling and running bootstrap I get
checking for GNU patch >= 2.7.0... /usr/bin/patch checking for curl 7.22... no checking for git... /usr/bin/git ./configure: line 7948: syntax error near unexpected token `elif' ./configure: line 7948: ` elif test x$SAGE_INSTALL_GCC = xno; then'
Huh, I'll have to give that a try. I definitely didn't get anything like that when I last worked on this ticket, but maybe the bug was in a code path that configure didn't go down on my system (are you using Sage's GCC or no?)
Thank you for debugging and finding the problem.
Replying to @dimpase:
I also saw
build/pkgs/curl/spkg-configure.m4not working due tobcnot installed.
Sobcshould be added to build pre-reqs.
Whoa, really?? Are you sure you don't have that problem without this patch? The test for curl existed in the original configure.ac, and I just moved the existing test to build/pkgs/curl/spkg-configure.m4 pretty much unmodified. It's not clear to me where it's looking for bc either. I thought bc was one of those things that's pretty much always there, like grep (albeit more obscure).
Could you be more specific about ho that came up? Was there something in config.log?
Replying to @dimpase:
I've tried creating more
spkg-configure.m4. I'm attaching one forxz. Not sure it's OK to usecut, but here it goes.
I can't imagine it wouldn't be OK to use cut. Again, it's a POSIX standard program, and the flags you're using are standard as well. Are you working on OSX? I feel like if it works on OSX it should work on any other platform we're supporting. If you're worried about it we could also put in an AC_PROG_CHECK for cut.
If nothing else, I believe that if errors occur in dependency checks, then we should just fail on detecting the dependency and build it.
Out of curiosity
6 changequote(<,>)
7 xz_version=`$ac_path_XZ --version 2>&1 \
8 | cut -d' ' -f4 | $SED -n 1p`
9 changequote([,])
why the changequote(<,>) here? It's not immediately obvious that anything in that command would be interpreted as quoting. But I must be missing a subtlety--please enlighten me.
LGTM otherwise.
Replying to @embray:
Replying to @dimpase:
I also saw
build/pkgs/curl/spkg-configure.m4not working due tobcnot installed.
Sobcshould be added to build pre-reqs.Whoa, really?? Are you sure you don't have that problem without this patch?
I built Sage on that system, a Debian 8(?) xlc container on CromeOS laptop, before, just fine. I suppose that test for curl just failed, and as a result Sage's curl was built.
Somehow bc was not installed.
The test for curl existed in the original configure.ac, and I just moved the existing test to build/pkgs/curl/spkg-configure.m4 pretty much unmodified. It's not clear to me where it's looking for bc either. I thought bc was one of those things that's pretty much always there, like grep (albeit more obscure).
Could you be more specific about ho that came up? Was there something in
config.log?
Yes, I saw "bc not found" in config.log, in a curl-related chunk.
I'm away from the box till evening but you can try uninstalling bc yourself and observing the result.
I'll have to figure out what exactly was trying to use bc and not finding it. That could be a bug in on of the standard autoconf macros, because in general they won't try to use a program that autoconf hasn't found first.
In any case, as long as the resulting behavior is to report curl as not found and move on, I'm mostly OK with that. The goal here is not necessarily to make all packages detectable 100% of the time* (though if there are cases where they should have been detected and weren't we certainly want to look into that).
- I should clarify: That is a goal long-term, theoretically, but for the purposes of this ticket I'm just concerned with the general mechanism.
Replying to @embray:
I'll have to figure out what exactly was trying to use
bcand not finding it.
without bc, one has
$ curl-config --checkfor 7.22
/usr/bin/curl-config: 1: /usr/bin/curl-config: bc: not found
/usr/bin/curl-config: 1: /usr/bin/curl-config: bc: not found
/usr/bin/curl-config: 112: test: Illegal number:
requested version 7.22 is newer than existing 7.52.1
As spkg-configure.m4 invokes curl-config --checkfor, you get this error
if no bc is installed. So this is a bug in curl, if you like. bc is used to
compute the "raw" version.
You see there
cpatch=`echo $checkfor | cut -d. -f3 | cut -d- -f1`
checknum=`echo "$cmajor*256*256 + $cminor*256 + ${cpatch:-0}" | bc`
numuppercase=`echo 073401 | tr 'a-f' 'A-F'`
nownum=`echo "obase=10; ibase=16; $numuppercase" | bc`
Replying to @embray:
Out of curiosity
6 changequote(<,>) 7 xz_version=`$ac_path_XZ --version 2>&1 \ 8 | cut -d' ' -f4 | $SED -n 1p` 9 changequote([,])why the
changequote(<,>)here?
It's just copy/paste from build/pkgs/patch/spkg-configure.m4
It's not immediately obvious that anything in that command would be interpreted as > quoting. But I must be missing a subtlety--please enlighten me.
Puzzling to me as well.
Replying to @dimpase:
As
spkg-configure.m4invokescurl-config --checkfor, you get this error
if nobcis installed.
Let us add the corresponding check in configure.ac and bail out if it's not there:
AC_CHECK_PROG(found_bc, bc, yes, no)
if test x$found_bc != xyes
then
AC_MSG_NOTICE([Sorry, the 'bc' command must be in the path to build AC_PACKAGE_NAME])
AC_MSG_NOTICE([On some systems it can be found in /usr/ccs/bin])
AC_MSG_NOTICE([See also https://www.gnu.org/software/bc/])
AC_MSG_ERROR([Exiting, as the calculator 'bc' can not be found.])
fi
How about doing something like this for cut, as well, just as an extra sanity check?
Replying to @dimpase:
Replying to @embray:
I'll have to figure out what exactly was trying to use
bcand not finding it.without
bc, one has$ curl-config --checkfor 7.22 /usr/bin/curl-config: 1: /usr/bin/curl-config: bc: not found /usr/bin/curl-config: 1: /usr/bin/curl-config: bc: not found /usr/bin/curl-config: 112: test: Illegal number: requested version 7.22 is newer than existing 7.52.1As
spkg-configure.m4invokescurl-config --checkfor, you get this error
if nobcis installed. So this is a bug incurl, if you like.bcis used to
compute the "raw" version.
You see therecpatch=`echo $checkfor | cut -d. -f3 | cut -d- -f1` checknum=`echo "$cmajor*256*256 + $cminor*256 + ${cpatch:-0}" | bc` numuppercase=`echo 073401 | tr 'a-f' 'A-F'` nownum=`echo "obase=10; ibase=16; $numuppercase" | bc`
I see now. I didn't realize curl-config was just a shell script. Then, technically, in order to run it we need to have all of its dependencies as well, hahah.
Well, that has nothing directly to do with this ticket so I'm not worried about it right now. But I will create a ticket for it.
Replying to @dimpase:
Replying to @embray:
Out of curiosity
6 changequote(<,>) 7 xz_version=`$ac_path_XZ --version 2>&1 \ 8 | cut -d' ' -f4 | $SED -n 1p` 9 changequote([,])why the
changequote(<,>)here?It's just copy/paste from
build/pkgs/patch/spkg-configure.m4
Oh, well in the case of patch I can tell you exactly why it's needed: the regular expression I use in the sed call uses square brackets, such as [0-9]. These are the default quote characters used in autoconf, so you need to changequote here (or use hideous m4 escape sequences).
It's not immediately obvious that anything in that command would be interpreted as > quoting. But I must be missing a subtlety--please enlighten me.
Puzzling to me as well.
Did you find it didn't work without it, or is it just cargo cult?
Replying to @embray:
why the
changequote(<,>)here?It's just copy/paste from
build/pkgs/patch/spkg-configure.m4Oh, well in the case of patch I can tell you exactly why it's needed: the regular expression I use in the
sedcall uses square brackets, such as[0-9]. These are the default quote characters used in autoconf, so you need tochangequotehere (or use hideous m4 escape sequences).It's not immediately obvious that anything in that command would be interpreted as > quoting. But I must be missing a subtlety--please enlighten me.
Puzzling to me as well.
Did you find it didn't work without it, or is it just cargo cult?
It escaped me that [] might need a special treatment, I thought it's something to do with potentially rougue numering using , i.p.of .
Consider this a request for comments in that code :-)
Yes, in m4 the default left and right quotes are `', but autoconf immediately sets the quotes to [], because autoconf is basically a big pile of m4 macros for generating shell code where ` and ' are meaningful and common.
Of course, so are [] in the context of tests, but there the [ is just a well-known synonym for test, which is why in autoconf you have to write if test instead of if [ in most places.
However, there are other rare cases where you might want to use [] as literals, such as in regular expressions, so there it's best to temporarily changequotes(). But the only reason you ever need to do that is if you really want to output some square brackets somewhere in your macro.
In addition to an obvious rebase, I also needed
--- a/configure.ac
+++ b/configure.ac
@@@ -754,21 -507,6 +507,9 @@@ AC_CONFIG_COMMANDS(mkdirs
SAGE_INST="$SAGE_SPKG_INST"
])
+dnl A stamp file indicating that an existing, broken GCC install should be
+dnl cleaned up by make.
- if test x$SAGE_BROKEN_GCC = xyes; then
- AC_CONFIG_COMMANDS([broken-gcc], [
- # Re-run the check just in case, such as when re-running config.status
- SAGE_CHECK_BROKEN_GCC()
- if test x$SAGE_BROKEN_GCC = xyes; then
- touch build/make/.clean-broken-gcc
- fi
- ], [
- SAGE_LOCAL="$SAGE_LOCAL"
- SAGE_SRC="$SAGE_SRC"
- ])
- fi
+
AC_OUTPUT()
dnl vim:syntax=m4
to avoid ./bootstrap complaining about broken-gcc already set in gcc/spkg-configure.m4
That stuff is supposed to already be in build/pkgs/gcc/spkg-configure.m4 so any rebase (which would include #25011 now) would indeed have to remove that block from configure.ac (and ensure that the relevant code in build/pkgs/gcc/spkg-configure.m4 also matches).
just ran into a yasm recognition bug:
configure:3791: checking for yasm
configure:3828: result: not needed for amd64
but MPIR duly needs yasm on amd64
And here
$ uname -m
amd64
This isn't detecting packages properly on OS X. Without this branch, installations of curl (already there), git (already there), gcc (because it should use clang), and gfortran (already there) are skipped (config.log says "not installed: configure check"). With this branch, they are all built. Nothing is skipped, it seems.
(I tested by running make distclean; autoreconf; make. Did I miss a step?)
if configure.ac is changed, one should run ./bootstrap, then ./configure, and look at the list of detected/undetected packages it prints.
Dima, John, for both of these issues, do you have the same issue without this patch? Because otherwise it's not relevant to the mechanism being implemented here, as for both yasm and curl this ticket is already using the exact same configuration already in place and has just refactored things a little.
If it works without this ticket then that is a problem with the refactoring. If you have the same problem with or without this ticket then that means it's working more or less as intended.
Replying to @jhpalmieri:
(I tested by running
make distclean; autoreconf; make. Did I miss a step?)
You should run ./bootstrap, not autoreconf.
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
d9e9b4f | Move the curl system package check to its own spkg-configure.m4 |
9c35b95 | Move the git system package check to its own spkg-configure.m4 |
e9b7362 | Move the gcc/gfortran system package checks to their own spkg-configure.m4 |
8821c76 | Slightly rewrote this macro to use some more polymorphic shell macros. |
b4630a5 | Use m4_sinclude instead--this prevents problems when switching between branches where one of these listed files no longer exists |
3cde0d7 | Move these comments inside the macro call to ensure they are actually expanded as part of the macro |
2f73979 | Changed a bit how SAGE_SPKG_CONFIGURE works: |
724c459 | Implement an spkg-configure.m4 for patch, demonstrating use of the new system |
97fec35 | Incorporate fixes from #25188 |
a2bedc5 | this check should be handled in gfortran's spkg-configure.m4 |
Still have a syntax error that crept in somewhere....
Rebased again. Should be good. If this can get positive review along with #25188 and #25198, then I might close those tickets in favor of this one, since it incorporates both of them. If either #25188 or #25198 are too problematic but this ticket is otherwise good, I can separate it from those issues for now and deal with them later. I'm fine with any order of operations.
I made #26281 for the bc issue with configuring curl. Unless that issue really only impacts this ticket I propose we table it for now. IMO it's more of an upstream bug since the curl package should have whatever package includes bc as a dependency, if it's used by curl-config.
OK, I'm testing this now. Would you open a ticket for yasm recognition (cf comment 70)?
Or perhaps just a followup ticket to add more recognition stuff and fix existing?
There is something wrong with yasm/spkg-configure.m4. Namely, ./configure does not report whether the result is yes or no. In config.log I see
...
configure:8328: === checking whether to install the xz SPKG ===
configure:8343: checking for xz >= 5.2.2
configure:8422: result: /usr/bin/xz
configure:8438: === checking whether to install the yasm SPKG ===
configure:8553: checking SPKGs to install
configure:8555: result:
configure:8644: result: 4ti2-1.6.7
...
so it looks like something is not right with it.
I noticed that, but I don't think it's a problem. The check is skipped for yasm entirely in most cases because yasm isn't even installed on all platforms.
That new notice I added just gets printed before each package that's checked for, but the final results are summarized later. Maybe there should be an additional message clarifying what's going on following each of those "checking whether to install..." messages?
On second thought, it looks like the AC_PATH_PROGS_FEATURE_CHECK doesn't automatically output a 'checking...' message in the first place, so maybe I'll add that there.
Branch pushed to git repo; I updated commit sha1. New commits:
fc985cc | added better reporting for yasm program+feature check |
Ok, now it shows
configure: === checking whether to install the git SPKG ===
checking for git... /usr/bin/git
configure: === checking whether to install the yasm SPKG ===
checking for yasm supporting the adox instruction... no
checking for Fortran flag needed to accept free-form source... -ffree-form
configure: === checking whether to install the gfortran SPKG ===
configure: === checking whether to install the curl SPKG ===
checking for curl 7.22... /usr/bin/curl
checking curl/curl.h usability... yes
checking curl/curl.h presence... yes
checking for curl/curl.h... yes
checking for a sed that does not truncate output... /bin/sed
configure: === checking whether to install the patch SPKG ===
checking for GNU patch >= 2.7.0... /usr/bin/patch
the message about the Fortran flag really belongs with the gfortran checks, but for some reason (and I noticed this long before) that check gets inserted into the configure script early, possibly something to do with diversions but I'm not sure. I'm not going to worry about it right now.
New commits:
fc985cc | added better reporting for yasm program+feature check |
Okay, if I do make distclean; ./bootstrap; ./configure, then I see
You are using OS X Lion (or later).
You are strongly advised to install Apple's latest Xcode
unless you already have it. You can install this using
the App Store. Also, make sure you install Xcode's
Command Line Tools -- see Sage's README.txt.
checking Python version to install... 2
checking multiprecision library... MPIR
checking BLAS library... openblas
./configure: line 6910: SAGE_SPKG_CONFIGURE_: command not found
./configure: line 6911: SAGE_SPKG_CONFIGURE_: command not found
./configure: line 6912: SAGE_SPKG_CONFIGURE_: command not found
./configure: line 6913: SAGE_SPKG_CONFIGURE_: command not found
./configure: line 6914: SAGE_SPKG_CONFIGURE_: command not found
./configure: line 6915: SAGE_SPKG_CONFIGURE_: command not found
checking SPKGs to install...
4ti2-1.6.7
alabaster-0.7.10
appnope-0.1.0.p0
arb-2.14.0
...
and all packages are going to be installed. As I said before, with a vanilla Sage, curl, git, gcc, and gfortran are not installed by Sage on this computer, because ./configure correctly detects them.
The configure script looks strange: after the line # Configure all spkgs with configure-time checks, there are about 30 blank lines, followed by
SAGE_SPKG_CONFIGURE_
SAGE_SPKG_CONFIGURE_
SAGE_SPKG_CONFIGURE_
SAGE_SPKG_CONFIGURE_
SAGE_SPKG_CONFIGURE_
SAGE_SPKG_CONFIGURE_
The file m4/sage_spkg_configures.m4 contains this:
m4_sinclude([build/pkgs//curl/spkg-configure.m4])
m4_sinclude([build/pkgs//gcc/spkg-configure.m4])
m4_sinclude([build/pkgs//gfortran/spkg-configure.m4])
m4_sinclude([build/pkgs//git/spkg-configure.m4])
m4_sinclude([build/pkgs//patch/spkg-configure.m4])
m4_sinclude([build/pkgs//yasm/spkg-configure.m4])
SAGE_SPKG_CONFIGURE_
SAGE_SPKG_CONFIGURE_
SAGE_SPKG_CONFIGURE_
SAGE_SPKG_CONFIGURE_
SAGE_SPKG_CONFIGURE_
SAGE_SPKG_CONFIGURE_
Maybe you are using GNUisms in the bash script bootstrap.
Replying to @embray:
the message about the Fortran flag really belongs with the gfortran checks, but for some reason (and I noticed this long before) that check gets inserted into the configure script early, possibly something to do with diversions but I'm not sure. I'm not going to worry about it right now.
Ok, I couldn't help but to worry about it a little bit; I wanted to know what was going on here. It's because each SAGE_SPKG_CONFIGURE_ macro (e.g. SAGE_SPKG_CONFIGURE_GFORTRAN) is defined with AC_DEFUN_ONCE--these are "one shot" macros that are expanded only once. However, if you call an AC_DEFUN_ONCE macro inside an AC_DEFUN_ONCE macro, it orders things so that nested one-shot macros are expanded first (I think this may be to help prevent recursion).
I'm not sure if there's a good way to circumvent that for the sake of the "checking" messages. Perhaps I can move them to outside the SAGE_SPKG_CONFIGURE_ macros themselves. Like I said, it's not terribly important though, and the end result is still consistent.
Replying to @jhpalmieri:
Maybe you are using GNUisms in the bash script
bootstrap.
Thank you. This looks similar to the same silly issue that came up in #26013 with the BSD find not stripping trailing slashes from its argument.
I still do not see a meaningful output on FreeBSD (perhaps due to a GNUism somewhere?):
configure: === checking whether to install the yasm SPKG ===
checking SPKGs to install...
--- this is in part due to absent amd64 case. If I add |amd64 to the corresponding list of acrs I get
configure: === checking whether to install the yasm SPKG ===
checking for yasm supporting the adox instruction... /usr/local/bin/yasm
Branch pushed to git repo; I updated commit sha1. New commits:
8c8e09a | BSD find puts in extra slashes if you leave a trailing slash in the argument |
How about now?
Replying to @sagetrac-git:
Branch pushed to git repo; I updated commit sha1. New commits:
8c8e09a | BSD find puts in extra slashes if you leave a trailing slash in the argument |
this does not change anything in respect to comment 91
Replying to @embray:
How about now?
That helps. Now the pre-existing packages are not installed, so it behaves as it did with the devel branch.
By the way, config.log used to be in logs/pkgs with a symlink to SAGE_ROOT, and now it's just in SAGE_ROOT. I prefer it the old way, or I suppose we could switch it around so that SAGE_ROOT/config.log is a file and there is a symlink in logs/pkgs. In any case, it's nice being able to access all of the logs, together with their modification times, in one place.
Reviewer: John Palmieri, Dima Pasechnik
Looks good to me. I've opened a follow-up ticket #26286.
Please do not forget to add a new configure tarball etc.
Replying to @dimpase:
I still do not see a meaningful output on FreeBSD (perhaps due to a GNUism somewhere?):
configure: === checking whether to install the yasm SPKG === checking SPKGs to install...--- this is in part due to absent
amd64case. If I add|amd64to the corresponding list of acrs I getconfigure: === checking whether to install the yasm SPKG === checking for yasm supporting the adox instruction... /usr/local/bin/yasm
Please just open a separate ticket for that.