Skip to content
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

Update to autotools 2.72 #289

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

skangas
Copy link

@skangas skangas commented Jul 24, 2024

This provides a more complete fix for #281 than PR #283 does. I've also included the manual steps below, which should be easily reproducible. This is an alternative to merging this patch outright (depending on your level of cautiousness after the xz backdoor).

Please see below the commit message.

Thanks!


This essentially just does the following, with autotools 2.72 installed:

$ cp /usr/lib/share/autoconf/build-aux/config.guess \
    tool/autoconf/build-aux/config.guess
$ cp /usr/lib/share/autoconf/build-aux/config.sub \
    tool/autoconf/build-aux/config.sub
$ autoreconf
configure.ac:28: warning: The macro 'AC_LANG_C' is obsolete.
configure.ac:28: You should run autoupdate.
./lib/autoconf/c.m4:72: AC_LANG_C is expanded from...
configure.ac:28: the top level
$ autoupdate

After running autoupdate, the following manual fix was needed:

-        [m4_esyscmd_s(sed -n '/^#define MPS_RELEASE "\(.*\)"$/{s/.*"\(.*\)"/\1/;p;}' code/version.c)],
+        [m4_esyscmd_s([sed -n '/^#define MPS_RELEASE "\(.*\)"$/{s/.*"\(.*\)"/\1/;p;}' code/version.c])],

This essentially just does the following, with autotools 2.72 installed:

    $ cp /usr/lib/share/autoconf/build-aux/config.guess \
        tool/autoconf/build-aux/config.guess
    $ cp /usr/lib/share/autoconf/build-aux/config.sub \
        tool/autoconf/build-aux/config.sub
    $ autoreconf
    configure.ac:28: warning: The macro 'AC_LANG_C' is obsolete.
    configure.ac:28: You should run autoupdate.
    ./lib/autoconf/c.m4:72: AC_LANG_C is expanded from...
    configure.ac:28: the top level
    $ autoupdate

After running autoupdate, the following manual fix was needed:

-        [m4_esyscmd_s(sed -n '/^#define MPS_RELEASE "\(.*\)"$/{s/.*"\(.*\)"/\1/;p;}' code/version.c)],
+        [m4_esyscmd_s([sed -n '/^#define MPS_RELEASE "\(.*\)"$/{s/.*"\(.*\)"/\1/;p;}' code/version.c])],
@rptb1
Copy link
Member

rptb1 commented Nov 26, 2024

Executing proc.review.express:

  1. Start time 13:21.
  2. Applied entry.universal and entry.impl. Entry passed.
  3. Called in @thejayps to the review.
  4. @thejayps agrees this is a low risk change and that express review is appropriate.
  5. https://www.gnu.org/software/autoconf/ is not immediately available -- connection timeout! Fortunately we the Wayback Machine copy.
  6. Reminded ourselves of the checking roles.
  7. to be continued...

@rptb1
Copy link
Member

rptb1 commented Nov 26, 2024

  1. Checking started 13:37.
  2. Checking changes against Autoconf documentation.
  3. AC_LANG_C is indeed marked as obsolete in the macro index and removed in https://github.com/Ravenbrook/mps/pull/289/files#diff-49473dca262eeab3b4a43002adb08b4db31020d190caaad1594b47f1d5daa810L28-R28 . AC_LANG is documented here.
  4. The Wayback Machine documentation is for Autoconf 2.72 which matches what's specified in the change.
  5. Checked out the branch and ran configure locally on Kiwi-Ubuntu.
  6. Autoconf on Ubuntu 24 is version 2.71, so autoreconf cannot run there. This means autoreconf cannot be run with this change on a supported platform.
  7. Halting review with a major defect.

@rptb1
Copy link
Member

rptb1 commented Nov 26, 2024

There are possibly some undocumented requirements and step here that have led to this change failing review. Essentially, we need to decide whether we require autoreconf to work on target platforms, of it's acceptable for there to be extra steps (such as installing a later GNU autoconf).

@waywardmonkeys
Copy link
Contributor

waywardmonkeys commented Nov 26, 2024

That might be why I only did an update of config.guess [in #283 ] as my ancient memories are that config.guess can be updated separately from the rest and that used to be a recommendation some 20-30 years ago when porting to newer systems.

@rptb1
Copy link
Member

rptb1 commented Nov 26, 2024

That might be why I only did an update of config.guess as my ancient memories are that config.guess can be updated separately from the rest and that used to be a recommendation some 20-30 years ago when porting to newer systems.

Your recollection is supported by https://web.archive.org/web/20240813003154/https://www.gnu.org/software/gettext/manual/html_node/config_002eguess.html which says:

You can obtain the newest version of config.guess and config.sub from the ‘config’ project at https://savannah.gnu.org/.

@rptb1 rptb1 self-assigned this Nov 26, 2024
@rptb1 rptb1 added build Problems with builds and build automation blocked Unable to proceed. See comment for reason. labels Nov 26, 2024
@rptb1
Copy link
Member

rptb1 commented Nov 27, 2024

The MPS does not really need autoconf. About the only thing we use it for is to get the target platform, which is the output of config.guess, just to offer an easy "./configure && make" command line. We could possibly go with our own, very simple, configure script and remove the autoconf dependency. While autoconf is around there's a risk that someone might start using it!

@rptb1 rptb1 added needs analaysis The issue needs analysis before it can be resolved. and removed blocked Unable to proceed. See comment for reason. labels Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Problems with builds and build automation needs analaysis The issue needs analysis before it can be resolved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants