Generic mechanism for system package checks at configure time

[Erik Bray]

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.

[Erik Bray]

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.

[Erik Bray]

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.

[Jeroen Demeyer]

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, ...

[Erik Bray]

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.

[Jeroen Demeyer]

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.

[Jeroen Demeyer]

The branch doesn't merge.

[Erik Bray]

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.

[Erik Bray]

Replying to jdemeyer:

The branch doesn't merge.

I know--at the very least #24907 conflicted with this, which is why I'd like to come up with a different solution to that issue that is more compatible.

[git]

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

[Erik Bray]

Rebased, but I think some other cleanup of configure.ac is now needed as a prerequisite (e.g. #25011).

[Dima Pasechnik]

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.

[Erik Bray]

Replying to dimpase:

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.

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.

[Dima Pasechnik]

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

[git]

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

[Erik Bray]

Replying to dimpase:

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

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.

[Erik Bray]

Seems to have a bug since the last rebase...

[git]

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

[Dima Pasechnik]

that's better, at least make distclean succeeds after ./bootstrap...

[Dima Pasechnik]

Replying to embray:

Replying to dimpase:

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

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".

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)

[Dima Pasechnik]

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.

[Erik Bray]

Replying to dimpase:

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.

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...

[Erik Bray]

Replying to dimpase:

that's better, at least make distclean succeeds after ./bootstrap...

Does it get any further than that? :)

Thanks for testing this out btw.

[Dima Pasechnik]

Replying to embray:

Replying to dimpase:

that's better, at least make distclean succeeds 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?

[Dima Pasechnik]

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)

[Vincent Delecroix]

Would be wonderful if it can handle #25158 (cmake which is an optional package).

[Dima Pasechnik]

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...)

[Erik Bray]

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.

[Erik Bray]

I could have sworn I rebased this recently. Maybe it was just on my todo list.

[git]

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

[Erik Bray]

Just noticed a typo in how this builds on top of #25011.

[Erik Bray]

I believe this issue can reasonably be addressed for Sage 8.4.

[git]

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

​f14f85b

Incorporate fixes from #25188

[Erik Bray]

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.

[Dima Pasechnik]

Why is it in "needs work" status?

[Timo Kaufmann]

Good work! What happens if the system package is updated to an incompatible version or even removed?

[Erik Bray]

Replying to gh-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.

[Dima Pasechnik]

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'

[Dima Pasechnik]

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...

[Dima Pasechnik]

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.

[Dima Pasechnik]

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.

[Dima Pasechnik]

xz spkg-configure.m4

[Dima Pasechnik]

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 ?

[Dima Pasechnik]

Replying to dimpase:

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.

This m4 file will also need proper check for liblzma being installed with headers and all.

[Erik Bray]

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.

[Erik Bray]

Replying to dimpase:

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.

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?

[Erik Bray]

Replying to dimpase:

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.

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.

[Erik Bray]

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.

[Dima Pasechnik]

Replying to embray:

Replying to dimpase:

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.

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.

[Erik Bray]

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.

[Dima Pasechnik]

Replying to embray:

I'll have to figure out what exactly was trying to use bc and 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`

[Dima Pasechnik]

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.

[Dima Pasechnik]

Replying to dimpase:

As spkg-configure.m4 invokes curl-config --checkfor, you get this error if no bc is 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?

[Erik Bray]

Replying to dimpase:

Replying to embray:

I'll have to figure out what exactly was trying to use bc and 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`

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.

[Erik Bray]

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?

[Dima Pasechnik]

Replying to embray:

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?

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 :-)

[Erik Bray]

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.

[Dima Pasechnik]

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

[Erik Bray]

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).

[Dima Pasechnik]

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

[John Palmieri]

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?)

[Dima Pasechnik]

if configure.ac is changed, one should run ./bootstrap, then ./configure, and look at the list of detected/undetected packages it prints.

[Erik Bray]

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.

[Erik Bray]

Replying to jhpalmieri:

(I tested by running make distclean; autoreconf; make. Did I miss a step?)

You should run ./bootstrap, not autoreconf.

[git]

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

[Erik Bray]

Still have a syntax error that crept in somewhere....

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​f01eb8a	Incorporate fixes from #25188
​d89f08a	this check should be handled in gfortran's spkg-configure.m4
​db1bbe0	add a diagnostic message that is occasionally helpful for understanding the configure output

[Erik Bray]

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.

[Erik Bray]

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.

[Dima Pasechnik]

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?

[Dima Pasechnik]

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.

[Erik Bray]

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?

[Erik Bray]

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.

[git]

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

​fc985cc

added better reporting for yasm program+feature check

[Erik Bray]

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

[John Palmieri]

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.

[Erik Bray]

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.

[Erik Bray]

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.

[Dima Pasechnik]

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

[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

[Erik Bray]

How about now?

[Dima Pasechnik]

Replying to 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

[John Palmieri]

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.

[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.

[Erik Bray]

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 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

Please just open a separate ticket for that.

[Erik Bray]

Thanks.

[Erik Bray]

This is probably going to blow up at least one of the buildbots without a new configure package, but I'd rather wait and see before going through that song and dance again.

[Dima Pasechnik]

This patch does seem not to work for sage -i and sage -f - e.g. even if I have patch recognised by configure as not needed to be installed, sage -i patch goes and installs it from source...

[Jeroen Demeyer]

Replying to dimpase:

This patch does seem not to work for sage -i and sage -f - e.g. even if I have patch recognised by configure as not needed to be installed, sage -i patch goes and installs it from source...

I think that's a feature. Even if a package is installed by the system, it should still be possible to manually install the Sage package.

[Volker Braun]

Merge conflict

[Erik Bray]

Replying to jdemeyer:

Replying to dimpase:

This patch does seem not to work for sage -i and sage -f - e.g. even if I have patch recognised by configure as not needed to be installed, sage -i patch goes and installs it from source...

I think that's a feature. Even if a package is installed by the system, it should still be possible to manually install the Sage package.

Yep. I said the same to Dima on the bus the other day. I can see his point and I did debate this myself, but ultimately I see sage -i as meaning "install the Sage SPKG". It can still be uninstalled too with sage-spkg-uninstall. This could/should be made easier of course.

[Erik Bray]

It's actually a very nice feature. For example, once I clean up the BLAS detection stuff a bit, it will become easier to use Sage with whatever the system's BLAS is (if indeed we do detect a usable one). But then, if need be, one can easily (in theory) use sage -i openblas to build and install their own tuned openblas for their Sage, for example.

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

​029584a	Slightly rewrote this macro to use some more polymorphic shell macros.
​e13304c	Use m4_sinclude instead--this prevents problems when switching between branches where one of these listed files no longer exists
​4b6c2ac	Move these comments inside the macro call to ensure they are actually expanded as part of the macro
​31fd98b	Changed a bit how SAGE_SPKG_CONFIGURE works:
​58acae8	Implement an spkg-configure.m4 for patch, demonstrating use of the new system
​273f25a	Incorporate fixes from #25188
​7577f7a	this check should be handled in gfortran's spkg-configure.m4
​ff087ef	add a diagnostic message that is occasionally helpful for understanding the configure output
​94f18b1	added better reporting for yasm program+feature check
​a1a384f	BSD find puts in extra slashes if you leave a trailing slash in the argument

[Erik Bray]

I rebased on current develop and there was no merge conflict. So I guess it's another mystery conflict...

[Dima Pasechnik]

tried to merge the vbraun's branch, it works now.

[Volker Braun]

Merge conflict

Also if you are merging my private branch then prepare for a world of pain as I'm going to rewrite it before publishing...

[Dima Pasechnik]

Replying to vbraun:

Merge conflict

Also if you are merging my private branch then prepare for a world of pain as I'm going to rewrite it before publishing...

No, this was not done, certainly. Now we have the beta branch to rebase this against, could you please put in into the next beta?

[Dima Pasechnik]

rebased

[Dima Pasechnik]

hmm, something went wrong during rebase...

[Dima Pasechnik]

cutting/pasting from git blame build/make/deps:

in the current develop branch (notice xz):

15c7b88bd76 build/make/deps     (Erik M. Bray    2018-07-17 15:59:31 +0000  81) TOOLCHAIN_DEPS = zlib xz $(MP_LIBRARY) mpfr mpc

in this ticket's branch (notice there is no xz):

9c1b4f6c243 build/make/deps     (Erik M. Bray    2018-07-17 15:59:31 +0000  80) TOOLCHAIN_DEPS = zlib $(MP_LIBRARY) mpfr mpc

and we have two very similar commits, but not quite:

$ git show 15c7b88bd76 | cat
commit 15c7b88bd76c58b769a06c2d4c8dede4ef33d850
Author: Erik M. Bray <erik.bray@lri.fr>
Date:   Tue Jul 17 15:59:31 2018 +0000

    move this list into a TOOLCHAIN_DEPS variable we can use to inspect this list elsewhere

diff --git a/build/make/deps b/build/make/deps
index 822b91b175..632ffa7c0e 100644
--- a/build/make/deps
+++ b/build/make/deps
@@ -78,12 +78,11 @@ toolchain: $(foreach pkgname,$(TOOLCHAIN),$(inst_$(pkgname)))
 # Note: This list is determined from the dependencies of the TOOLCHAIN
 # packages which include gcc, and optionally ccache; in principle this
 # list is redundant.
+TOOLCHAIN_DEPS = zlib xz $(MP_LIBRARY) mpfr mpc
 toolchain-deps:
-	$(MAKE) $(inst_zlib)
-	$(MAKE) $(inst_xz)
-	$(MAKE) $(MP_LIBRARY)
-	$(MAKE) $(inst_mpfr)
-	$(MAKE) $(inst_mpc)
+	for pkg in $(TOOLCHAIN_DEPS); do \
+	    $(MAKE) $$pkg; \
+	done
 
 all-toolchain: base-toolchain
 	$(MAKE) toolchain-deps
dima@hilbert /mnt/opt/Sage/sage-clang $ git show 9c1b4f6c243 | cat
commit 9c1b4f6c24391f952c73783503a33eea0469fdc2
Author: Erik M. Bray <erik.bray@lri.fr>
Date:   Tue Jul 17 15:59:31 2018 +0000

    move this list into a TOOLCHAIN_DEPS variable we can use to inspect this list elsewhere

diff --git a/build/make/deps b/build/make/deps
index 24508b82a4..4f3dc4588c 100644
--- a/build/make/deps
+++ b/build/make/deps
@@ -75,11 +75,14 @@ toolchain: $(foreach pkgname,$(TOOLCHAIN),$(inst_$(pkgname)))
 # unconditionally. We still use the dependency checking from $(MAKE),
 # so this will not trigger useless rebuilds.
 # See #14168 and #14232.
+# Note: This list is determined from the dependencies of the TOOLCHAIN
+# packages which include gcc, and optionally ccache; in principle this
+# list is redundant.
+TOOLCHAIN_DEPS = zlib $(MP_LIBRARY) mpfr mpc
 toolchain-deps:
-	$(MAKE) $(inst_zlib)
-	$(MAKE) $(MP_LIBRARY)
-	$(MAKE) $(inst_mpfr)
-	$(MAKE) $(inst_mpc)
+	for pkg in $(TOOLCHAIN_DEPS); do \
+	    $(MAKE) $$pkg; \
+	done
 
 all-toolchain: base-toolchain
 	$(MAKE) toolchain-deps

This looks very fishy, as the dates/times are the same, only hashes are different. And merging this ticket branch into develop does not quite work, as e.g. build/make/deps does not get updated!

Erik, Volker, any ideas what's up there?

---

New commits:

​de2746f

Merge #24919

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

​241bb4c	py3: fix doctests in ribbon tableaux
​5dec1a7	Resolved problem in parallel a*a
​d07180d	Changed name
​e16e128	remove cephes - we do not need it anymore
​098f249	Trac #26510: Fix combinat.subset for python3
​1b5fd0e	Trac #26545: Reverse 'islice' corrections and rebase ...
​3ea41e3	remove deprecated stuff in manifolds
​c5d4518	py3: fix some doctests in combinat/species
​e319a2b	trac 26610 reviewer's request
​aa6932d	Updated SageMath version to 8.5.beta2

[Dima Pasechnik]

and if instead I rebase this ticket's branch over 8.4.beta2, I get a branch with a merge conflict.

At seems that at this point something needs to be done manually, for a reason I cannot comprehend...

[Erik Bray]

Please just let me....

[Erik Bray]

This is rebased on latest develop and cleaned up now. I think everything is good, but I want to re-check some of the gcc-related stuff just in case.

---

Last 10 new commits:

​506ffb3	Move the gcc/gfortran system package checks to their own spkg-configure.m4
​e16b957	Slightly rewrote this macro to use some more polymorphic shell macros.
​b7ba60e	Use m4_sinclude instead--this prevents problems when switching between branches where one of these listed files no longer exists
​8f338fb	Move these comments inside the macro call to ensure they are actually expanded as part of the macro
​ef1edb0	Changed a bit how SAGE_SPKG_CONFIGURE works:
​73a6d0d	Implement an spkg-configure.m4 for patch, demonstrating use of the new system
​c4083a6	this check should be handled in gfortran's spkg-configure.m4
​a512752	add a diagnostic message that is occasionally helpful for understanding the configure output
​3ab0529	added better reporting for yasm program+feature check
​5de0eb5	BSD find puts in extra slashes if you leave a trailing slash in the argument

[Dima Pasechnik]

I think you ended up with the same slightly broken branch as I did in my rebase attempts - check that your TOOLCHAIN_DEPS variable in build/make/deps does not include xz --- cf comment 114.

[Erik Bray]

I see what you're saying now. That has nothing directly to do with this ticket actually. I think that what happened was that I merged an old version of the branch from #25857 into the branch for #25188. But then #25188 got merged into develop later, and brought in an old version of that commit (​15c7b88b) into develop, superseding the correct version (​9c1b4f6c) that was merged in when #25857 was merged.

That whole mess is my fault, and I apologize for the confusion it's caused for you. I'll open a separate ticket to fix it. But ultimately it doesn't impact this ticket.

[Erik Bray]

E.g. you can see:

$ git diff 8.5.beta1..8.5.beta2 -- build/make/deps

build/make/deps

@@ -75 +75 @@
 # unconditionally. We still use the dependency checking from $(MAKE),
 # so this will not trigger useless rebuilds.
 # See #14168 and #14232.
-#
-# Note: This list consists of only the *runtime* dependencies of the toolchain.
-TOOLCHAIN_DEPS = zlib $(MP_LIBRARY) mpfr mpc
-TOOLCHAIN_DEP_INSTS = \
-       $(foreach pkgname,$(TOOLCHAIN_DEPS),$(inst_$(pkgname)))
-
+# Note: This list is determined from the dependencies of the TOOLCHAIN
+# packages which include gcc, and optionally ccache; in principle this
+# list is redundant.
+TOOLCHAIN_DEPS = zlib xz $(MP_LIBRARY) mpfr mpc
 toolchain-deps:
-       for target in $(TOOLCHAIN_DEP_INSTS); do \
-           $(MAKE) $$target; \
+       for pkg in $(TOOLCHAIN_DEPS); do \
+           $(MAKE) $$pkg; \
        done
 
 all-toolchain: base-toolchain
@@ -129 +127 @@
                $(inst_arb) \
                $(BLAS) \
                $(inst_brial) \
-               $(inst_cephes) \
                $(inst_cliquer) \
                $(inst_cypari) \
                $(inst_cysignals) \

so that just started in 8.5.beta2 since #25188 was merged in that build. It was never intended that xz should be added to TOOLCHAIN_DEPS as part of that ticket. I just screwed up :(

[Dima Pasechnik]

Replying to embray:

I see what you're saying now. That has nothing directly to do with this ticket actually.

I think in Luxembourg I showed you a puzzling error that we traced to my using a branch with wrong TOOLCHAIN_DEPS - could you fix it here, instead of delaying to another ticket?

[Erik Bray]

Replying to dimpase:

Replying to embray:

I see what you're saying now. That has nothing directly to do with this ticket actually.

I think in Luxembourg I showed you a puzzling error that we traced to my using a branch with wrong TOOLCHAIN_DEPS - could you fix it here, instead of delaying to another ticket?

Please see above. It really has nothing to do with this ticket and doesn't impact it one way or the other. It can be fixed in a separate ticket totally independent from this one. The only impact of this mistake is that the xz package was added to the toolchain dependencies, but Jeroen (I think) had pointed out to me that it's not actually needed there so it's sort of a superfluous dependency that gets built earlier than necessary.

This was only messing you up in Luxembourg since you were experimenting with adding a configure-time check for xz (thank you!) but since that hasn't actually been added yet there's no issue. Also IIRC the bug with that was not actually because xz was included in TOOLCHAIN_DEPS, but due to a bug that I fixed in a later version of #25188. I got confused because I was certain xz shouldn't be there and yet it was, due to the mistake I described above (which I was unaware of until now).

[Dima Pasechnik]

If I add a build/pkgs/xz/spkg-configure.m4, e.g. from #26286, it won't work. This is a problem to fix on this ticket, no?

[Dima Pasechnik]

I believe there was also a discrepancy in another file in build/make, not only deps.

[Erik Bray]

Replying to dimpase:

If I add a build/pkgs/xz/spkg-configure.m4, e.g. from #26286, it won't work.

But for reasons that have nothing to do with this ticket. What I mean is, even if you added a configure-time check directly to configure.ac (e.g. ignoring the mechanisms implemented by this ticket) the same problem you were having (the SPKG being forcibly built when it wasn't needed) would occur.

If you want the specifics, there was an older version of #25857 that had this bug, which Jeroen pointed out: ​https://trac.sagemath.org/ticket/25857#comment:26 I fixed the bug, and #25857 was merged without any (known) bugs. I then accidentally reintroduced that bug in #25188 when I merged an old version of my branch for #25857 into that branch. I simply need to add a new ticket to re-revert that bug.

[Erik Bray]

Replying to dimpase:

I believe there was also a discrepancy in another file in build/make, not only deps.

Yes, it looks like a change to build/make/Makefile.in was also accidentally reverted as part of the same bogus merge, though that one is merely cosmetic.

[Dima Pasechnik]

How about fixing it on #26286 ?

[Erik Bray]

I still have some doubts about whether the gcc-related checks are working as intended. I need to re-review my original fixes for those and ensure that this isn't causing a regression (I think it might be).

Previously I had incorporated all those fixes into this ticket but everything's been through so many revisions/rebases it's not clear that it's working as I expect.

[Erik Bray]

In particular I think #25188 may have totally regressed; specifically the question of whether or not ./configure lists the sage gcc package as installed.

[Erik Bray]

Yeah, I made at least one mistake while rebasing. ​This should be spelled sage_spkg_install_gcc=yes now.

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​8fd4dec	Move the gcc/gfortran system package checks to their own spkg-configure.m4
​21fbddd	Slightly rewrote this macro to use some more polymorphic shell macros.
​b4cc8eb	Use m4_sinclude instead--this prevents problems when switching between branches where one of these listed files no longer exists
​8d01c6e	Move these comments inside the macro call to ensure they are actually expanded as part of the macro
​cf6367a	Changed a bit how SAGE_SPKG_CONFIGURE works:
​16c22de	Implement an spkg-configure.m4 for patch, demonstrating use of the new system
​6526868	this check should be handled in gfortran's spkg-configure.m4
​a5f783e	add a diagnostic message that is occasionally helpful for understanding the configure output
​3bedcd6	added better reporting for yasm program+feature check
​79de3bd	BSD find puts in extra slashes if you leave a trailing slash in the argument

[Erik Bray]

Okay, now it should be good to go.

[Erik Bray]

Specifically, the commit at ​https://git.sagemath.org/sage.git/commit/?id=8fd4decd9e2d2f33ec9d3fa6a488c0df23b88764 is updated with the corrected spelling of that line.

[Dima Pasechnik]

see #26715 for a gfortran-related collateral damage caused here.