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

Improve and simplify configure.ac checks #89886

Closed
tiran opened this issue Nov 5, 2021 · 33 comments
Closed

Improve and simplify configure.ac checks #89886

tiran opened this issue Nov 5, 2021 · 33 comments
Assignees
Labels
3.13 bugs and security fixes build The build process and cross-build type-feature A feature request or enhancement

Comments

@tiran
Copy link
Member

tiran commented Nov 5, 2021

BPO 45723
Nosy @tiran, @erlend-aasland
PRs
  • bpo-45723: Add helper macros and more caching to configure.ac (GH-29429) #29429
  • bpo-45723: Prepare support for autoconf 2.71 (GH-29441) #29441
  • bpo-45723: Detect missing pkg-config (GH-29442) #29442
  • bpo-45723: Add macro for disabling/enabling CC warnings (GH-29466) #29466
  • bpo-45723: Improve and simplify more configure.ac checks (GH-29485) #29485
  • bpo-45723: Remove obsolete AC_EXEEXT from configure.ac #29486
  • bpo-45723: Remove dead code for obsolete --with-dyld option #29500
  • bpo-45723: Add --with-pkg-config to configure (GH-29517) #29517
  • bpo-45723: Add helpers for save/restore env (GH-29637) #29637
  • bpo-45723: Use SAVE/RESTORE macros in configure.ac #29701
  • bpo-45723: Sort the grand AC_CHECK_HEADERS check #29846
  • bpo-45723: Normalise configure user communication (GH-30024) #30024
  • bpo-89886: Clean up MACHDEP and _PYTHON_HOST_PLATFORM checks #30026
  • gh-89886: Use AC_CHECK_TYPES iso. AC_CHECK_TYPE #30029
  • bpo-45723: Fix detection of epoll #30449
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/tiran'
    closed_at = None
    created_at = <Date 2021-11-05.10:05:47.030>
    labels = ['type-feature', 'build', '3.11']
    title = 'Improve and simplify configure.ac checks'
    updated_at = <Date 2022-01-12.04:40:54.295>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2022-01-12.04:40:54.295>
    actor = 'zach.ware'
    assignee = 'christian.heimes'
    closed = False
    closed_date = None
    closer = None
    components = ['Build']
    creation = <Date 2021-11-05.10:05:47.030>
    creator = 'christian.heimes'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 45723
    keywords = ['patch']
    message_count = 18.0
    messages = ['405780', '405799', '405865', '405896', '405932', '405977', '405978', '406021', '406027', '406054', '406104', '406137', '406745', '407281', '408191', '409946', '409951', '409953']
    nosy_count = 3.0
    nosy_names = ['christian.heimes', 'erlendaasland', 'byllyfish']
    pr_nums = ['29429', '29441', '29442', '29466', '29485', '29486', '29500', '29517', '29637', '29701', '29846', '30024', '30026', '30029', '30449']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue45723'
    versions = ['Python 3.11']

    Linked PRs

    @tiran
    Copy link
    Member Author

    tiran commented Nov 5, 2021

    The autoconf-based build system has room for improvements. The configure.ac script can be simplified in several places by using AS and AC macros or by defining new custom macros.

    For example we have a lot of blocks that look like this:

    AC_MSG_CHECKING(for chroot)
    AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <unistd.h>]], [[void x=chroot]])],
      [AC_DEFINE(HAVE_CHROOT, 1, Define if you have the 'chroot' function.)
       AC_MSG_RESULT(yes)],
      [AC_MSG_RESULT(no)
    ])

    The block has an issue, too. It does not use AC_CACHE to cache the result. The entire block can be replaced by a custom macro that takes care of everything and implements correct caching:

      PY_CHECK_FUNC([chroot], [#include <unistd.h>])

    We can also move several library and header checks from setup.py into configure.ac, e.g. check for soundcard.h or gdbm libs and headers.

    @tiran tiran added type-feature A feature request or enhancement 3.11 only security fixes labels Nov 5, 2021
    @tiran tiran self-assigned this Nov 5, 2021
    @tiran tiran added build The build process and cross-build type-feature A feature request or enhancement 3.11 only security fixes labels Nov 5, 2021
    @tiran tiran self-assigned this Nov 5, 2021
    @tiran tiran added the build The build process and cross-build label Nov 5, 2021
    @tiran
    Copy link
    Member Author

    tiran commented Nov 5, 2021

    The first PR adds helper macros, AC_CACHE_CHECK() [1] and AS_VAR_IF() [2]. It also unified internal variables to use format "ac_cv_func_$funcname", "ac_cv_func_lib_$library_$funcname", or "ac_cv_header_$headername_h". "ac_cv" stands for autoconf cached value.

    AC_CACHE_CHECK() replaces AC_MSG_CHECKING() and AC_MSG_RESULT(). The syntax is AC_CACHE_CHECK([text], [cache variable], [body]) where body is only excecuted when the cache variable is not set. The body has to set the cache variable to yes or no. Any output and AC_DEFINE must occur outside the body.

    AS_VAR_IF() is a nicer way to write if test $variable = value; then; fi.

    [1] https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Caching-Results.html#Caching-Results
    [2] https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Polymorphic-Variables.html

    @tiran
    Copy link
    Member Author

    tiran commented Nov 6, 2021

    #73627 introduces forward compatibility issues with autoconf 2.71. I took the output of autoupdate and resolved all warnings.

    @tiran
    Copy link
    Member Author

    tiran commented Nov 7, 2021

    New changeset be3cd5c by Christian Heimes in branch 'main':
    bpo-45723: Detect missing pkg-config (GH-29442)
    be3cd5c

    @tiran
    Copy link
    Member Author

    tiran commented Nov 8, 2021

    New changeset 57c50c9 by Christian Heimes in branch 'main':
    bpo-45723: Add helper macros and more caching to configure.ac (GH-29429)
    57c50c9

    @tiran
    Copy link
    Member Author

    tiran commented Nov 8, 2021

    New changeset 9bd0cf5 by Erlend Egeberg Aasland in branch 'main':
    bpo-45723: Add macro for disabling/enabling CC warnings (GH-29466)
    9bd0cf5

    @tiran
    Copy link
    Member Author

    tiran commented Nov 8, 2021

    New changeset cbab997 by Christian Heimes in branch 'main':
    bpo-45723: Prepare support for autoconf 2.71 (GH-29441)
    cbab997

    @erlend-aasland
    Copy link
    Contributor

    AC_CHECK_TYPE is obsolete and it's use is discouraged. There are some gotchas we need to check before switching to AC_CHECK_TYPES.

    See:

    @miss-islington
    Copy link
    Contributor

    New changeset 1855336 by Erlend Egeberg Aasland in branch 'main':
    bpo-45723: Remove obsolete AC_EXEEXT from configure.ac (GH-29486)
    1855336

    @miss-islington
    Copy link
    Contributor

    New changeset 49171aa by Erlend Egeberg Aasland in branch 'main':
    bpo-45723: Remove dead code for obsolete --with-dyld option (GH-29500)
    49171aa

    @tiran
    Copy link
    Member Author

    tiran commented Nov 10, 2021

    New changeset 76d14fa by Erlend Egeberg Aasland in branch 'main':
    bpo-45723: Improve and simplify more configure.ac checks (GH-29485)
    76d14fa

    @tiran
    Copy link
    Member Author

    tiran commented Nov 10, 2021

    New changeset fc9b622 by Christian Heimes in branch 'main':
    bpo-45723: Add --with-pkg-config to configure (GH-29517)
    fc9b622

    @tiran
    Copy link
    Member Author

    tiran commented Nov 22, 2021

    New changeset db2277a by Erlend Egeberg Aasland in branch 'main':
    bpo-45723: Add helpers for save/restore env (GH-29637)
    db2277a

    @miss-islington
    Copy link
    Contributor

    New changeset c1dec95 by Erlend Egeberg Aasland in branch 'main':
    bpo-45723: Sort the grand AC_CHECK_HEADERS check (GH-29846)
    c1dec95

    @tiran
    Copy link
    Member Author

    tiran commented Dec 10, 2021

    New changeset 74b23c9 by Erlend Egeberg Aasland in branch 'main':
    bpo-45723: Normalise configure user communication (GH-30024)
    74b23c9

    @byllyfish
    Copy link
    Mannequin

    byllyfish mannequin commented Jan 7, 2022

    In the conversion to PY_CHECK_FUNC, there's a mistake in HAVE_EPOLL.

    Python 3.10.1 defines HAVE_EPOLL by checking for the epoll_create function. Python 3.11.0a3 checks for the epoll function instead. There is no epoll() function so this always fails.

    The effect is that epoll doesn't exist in the select module on Python 3.11.0a3. Most code that uses epoll falls back when it is not available, so this may not be failing any tests.

    @tiran
    Copy link
    Member Author

    tiran commented Jan 7, 2022

    Thanks William!

    @tiran
    Copy link
    Member Author

    tiran commented Jan 7, 2022

    New changeset 994f90c by Christian Heimes in branch 'main':
    bpo-45723: Fix detection of epoll (bpo-30449)
    994f90c

    @asottile
    Copy link
    Contributor

    asottile commented Jun 2, 2023

    autoconf bumping to 2.71+ is a little annoying because LTS ubuntu 20.04 only ships with 2.69

    should this have landed in 3.12 which is already in beta freeze? I'm building this for deadsnakes and it'd kind of suck to not ship 3.12 for ubuntu 20.04 especially when 3.12b1 built cleanly

    @erlend-aasland
    Copy link
    Contributor

    erlend-aasland commented Jun 3, 2023

    autoconf bumping to 2.71+ is a little annoying because LTS ubuntu 20.04 only ships with 2.69

    should this have landed in 3.12 which is already in beta freeze? I'm building this for deadsnakes and it'd kind of suck to not ship 3.12 for ubuntu 20.04 especially when 3.12b1 built cleanly

    Perhaps you are misunderstanding something. The generated configure script is part of the sources; a clean build includes the generated configure script (try git clean -fdx).

    IOW: You don't need GNU Autoconf at all for a clean build.

    @asottile
    Copy link
    Contributor

    asottile commented Jun 3, 2023

    part of debian packaging is to regenerate configure

    @erlend-aasland
    Copy link
    Contributor

    part of debian packaging is to regenerate configure

    They can still do that with autoconf 2.69. There is no hard requirement to use 2.71. We only require 2.71 for CPython development. Debian should be all ok with 2.69.

    @asottile
    Copy link
    Contributor

    asottile commented Jun 3, 2023

    the sources were updated to require 2.71 in #105207

    @erlend-aasland
    Copy link
    Contributor

    the sources were updated to require 2.71 in #105207

    For CPython development, yes.

    @erlend-aasland
    Copy link
    Contributor

    Ah, there's a requirement check in the configure.ac script. Perhaps we can loosen that.

    @asottile
    Copy link
    Contributor

    asottile commented Jun 3, 2023

    Ah, there's a requirement check in the configure.ac script. Perhaps we can loosen that.

    yes that's the one I've been referring to

    @erlend-aasland
    Copy link
    Contributor

    autoconf bumping to 2.71+ is a little annoying because LTS ubuntu 20.04 only ships with 2.69

    should this have landed in 3.12 which is already in beta freeze? I'm building this for deadsnakes and it'd kind of suck to not ship 3.12 for ubuntu 20.04 especially when 3.12b1 built cleanly

    Anthony: we've discussed this in the core dev group, and we reached the conclusion that we'll not be adjusting the hard requirement in the .ac file. For deadsnakes, I would suggest to simply patch the .ac file before regenerating configure. You should be fine with the following patch:

    diff --git a/configure.ac b/configure.ac
    index a24cd68973..793cd859ad 100644
    --- a/configure.ac
    +++ b/configure.ac
    @@ -15,7 +15,7 @@ dnl
     # Set VERSION so we only need to edit in one place (i.e., here)
     m4_define([PYTHON_VERSION], [3.13])
     
    -AC_PREREQ([2.71])
    +AC_PREREQ([2.69])
     
     AC_INIT([python],[PYTHON_VERSION],[https://github.com/python/cpython/issues/])
     
    

    @erlend-aasland
    Copy link
    Contributor

    I'll close this issue for now; Christian and I made several improvements to the AC code back in 3.10 and 3.11. IMO, future improvements should get their own targeted issues.

    (cc. @tiran, in case you're around)

    @asottile
    Copy link
    Contributor

    @erlend-aasland my fear is that 2.71+ syntax / features will get used -- what should I do then?

    @erlend-aasland
    Copy link
    Contributor

    @erlend-aasland my fear is that 2.71+ syntax / features will get used -- what should I do then?

    I suggest sticking to the recommendations of the devguide:

    $ podman run --rm --pull=always -v $(pwd):/src:Z quay.io/tiran/cpython_autoconf:271
    # ... or:
    $ docker run --rm --pull=always -v $(pwd):/src quay.io/tiran/cpython_autoconf:271

    Use the 269 tag iso. 271 for GNU Autoconf 2.69. Note also, that the autoconf version we use for 3.11 and older is not the stock GNU Autoconf v2.69; it is a patched version. So, once again, you're better off by using the devguide recommendations (or just use the configure script shipped with the source code) :)

    @asottile
    Copy link
    Contributor

    I can't just do that unfortunately. that's outside my control as debian packages both must build from source and use the packages of the distribution

    when 2.71+ syntax is used I'll basically have no choice but to drop the packages which is unfortunate but understandable

    @erlend-aasland
    Copy link
    Contributor

    Well, on the bright side, no "2.71+ syntax" did sneak in yet, so deadsnakes should be fine for now :)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.13 bugs and security fixes build The build process and cross-build type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants