-
Notifications
You must be signed in to change notification settings - Fork 161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix autoreconf 2.71 missing some files #4377
Conversation
Sorry that CI problems are intruding on your holiday 🙁 But thanks for helping @ChrisJefferson. Sadly the job has failed again, at least so far. If you or anyone else wants to look into this further, then adding this step:
to the workflow (from https://github.com/marketplace/actions/debugging-with-tmate) might be useful, for SSHing into a session where it doesn't compile. |
6874bf2
to
071e149
Compare
I (think) that's working now. The main problem is summed up by the paragraph below (quoted from the release notes). In order to get
|
(there is still a whole bunch of warnings about deprecated things, that might well want fixing some time, but at least right now don't seem to be blocking us running CI, or building packages.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll submit an alternate PR.
But we still need to also deal with this for packages. I actually started looking into this yesterday, when I run into this independently for some GAP packages. I should have notified you folks to avoid duplication of effort, sorry.
@@ -183,7 +183,7 @@ jobs: | |||
sudo apt-get update | |||
sudo apt-get install "${packages[@]}" | |||
elif [ "$RUNNER_OS" == "macOS" ]; then | |||
brew install gmp readline zlib | |||
brew install gmp readline zlib automake |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No no no! We do not want to pull in automake
@@ -186,6 +186,11 @@ run_configure_and_make() { | |||
if [[ -x autogen.sh && ! -x configure ]] | |||
then | |||
./autogen.sh | |||
# (Hopefully) temporary fix for Autoconf 2.71 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can try to simply update config.sub and config.guess, that's something we have to do occasionally anyway (see issue #1478). There are other changes to be made in configure.ac. I'll look into that. But all of this must keep working with autoconf 2.69, which is what almost everyone has. In a quick test of mine, it does so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the issue was actually specific to the io package, which actually had a buggy configure.ac that happened to work in autoconf 2.69, and now 2.70 detected it. To be precise: I set it up to not check for a "canonical host type", but then used autoconf macros which actually require that; in autoconf 2.69 this sailed through (and possibly resulted in buggy behavior, though this unlikely on any modern day OS). In autoconf 2.70 it detects this requirement and computes the canonical host type, which require config.guess and config.sub ... hence the issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't think this is required (nor appropriate), but I am open to have my mind changed about this :-)
The release notes for autoconf 2.70 ( https://lwn.net/Articles/839395/ ) mention it may be necessary to run
autoreconf --install
. This should probably be added to each package, but for now we can add it tobuild_packages.sh
-- this may be too much of a sledge-hammer, let's first see if this fixes problems at all.See #4374.