← 返回首页
bpo-45723: Add helper macros and more caching to configure.ac (GH-29429) by tiran · Pull Request #29429 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

bpo-45723: Add helper macros and more caching to configure.ac (GH-29429)#29429

Merged
tiran merged 3 commits into
python:mainfrom
tiran:bpo-45723-configure
Nov 8, 2021
Merged

bpo-45723: Add helper macros and more caching to configure.ac (GH-29429)#29429
tiran merged 3 commits into
python:mainfrom
tiran:bpo-45723-configure

Conversation

tiran commented Nov 5, 2021
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Almost all checks are now cached by AC_CACHE_CHECK().

Common patterns are replaced by helper macros.

Variable names now use naming scheme ac_cv_func_$funcname,
ac_cv_lib_$library_$funcname, or ac_cv_header_$headername_h.

SYS_SELECT_WITH_SYS_TIME is no longer used.

uuid_create and uuid_enc_be are provided by libc on BSD. It is
safe to use AC_CHECK_FUNCS here.

Caching speeds up ./configure -C from ~ 4s to 2.6s on my system.

Signed-off-by: Christian Heimes christian@python.org

https://bugs.python.org/issue45723

gpshead commented Nov 5, 2021

Copy link
Copy Markdown
Member

I suggest using a buildbot-test of this when it is believed ready. Yay more modern autoconf practices.

tiran force-pushed the bpo-45723-configure branch from d048406 to f13a97f Compare November 5, 2021 22:11
tiran added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 5, 2021

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @tiran for commit f13a97f33c447f309c40315437a38ea4386e46f8 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 5, 2021

pablogsal commented Nov 5, 2021
edited
Loading

Copy link
Copy Markdown
Member

Outstanding!! 🚀🚀

tiran commented Nov 6, 2021

Copy link
Copy Markdown
Member Author

I suggest using a buildbot-test of this when it is believed ready. Yay more modern autoconf practices.

All buildbots are passing except FreeBSD builder. It fails with an unrelated error in test_gdb.

tiran force-pushed the bpo-45723-configure branch from f13a97f to f9bfcf2 Compare November 6, 2021 10:07
Almost all checks are now cached by AC_CACHE_CHECK(). Common patterns are replaced by helper macros. Variable names now use naming scheme ``ac_cv_func_$funcname``, ``ac_cv_lib_$library_$funcname``, or ``ac_cv_header_$headername_h``. ``SYS_SELECT_WITH_SYS_TIME`` is no longer used. ``uuid_create`` and ``uuid_enc_be`` are provided by libc on BSD. It is safe to use ``AC_CHECK_FUNCS`` here. Caching speeds up ./configure -C from ~ 4s to 2.6s on my system. Signed-off-by: Christian Heimes <christian@python.org>
tiran force-pushed the bpo-45723-configure branch from f9bfcf2 to a52ce97 Compare November 7, 2021 11:05

erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

This is so nice; great work! The macros really simplifies a lot of stuff. I left some suggestions, mainly:

  • add missing brackets around text/descriptions (not required, but good practise)
  • remove unneeded double brackets; AFAIK, double brackets are only needed if you need literal brackets in the target text. OTOH, the double brackets make no harm, so I'm fine with leaving it as it is.

There more stuff that can be cached in there, and we could simplify stuff further with more macros (for example tweaking BASECFLAGS), but this PR is already pretty huge, so perhaps it's better to split it up?

Comment thread configure.ac Outdated Show resolved Hide resolved
Comment thread configure.ac Outdated Show resolved Hide resolved
Comment thread configure.ac Outdated Show resolved Hide resolved
Comment thread configure.ac Outdated Show resolved Hide resolved
Comment thread configure.ac Outdated Show resolved Hide resolved
9 hidden conversations Load more…
Comment thread configure.ac Outdated Show resolved Hide resolved
Comment thread configure.ac Outdated Show resolved Hide resolved
Comment thread configure.ac Outdated Show resolved Hide resolved
Comment thread configure.ac Outdated Show resolved Hide resolved
Comment thread configure.ac Outdated Show resolved Hide resolved

erlend-aasland commented Nov 7, 2021
edited
Loading

Copy link
Copy Markdown
Contributor

BTW, is configure up to date? I recreated it using (homebrew supplied) autoconf 2.69 and got this diff:
Regenerated configure with @tiran's autoconf docker; no diff produced.

$ git diff configure
diff --git a/configure b/configure index cec73c2cf9..1e433cbbff 100755 --- a/configure +++ b/configure @@ -793,7 +793,6 @@ infodir docdir oldincludedir includedir -runstatedir localstatedir sharedstatedir sysconfdir @@ -918,7 +917,6 @@ datadir='${datarootdir}' sysconfdir='${prefix}/etc' sharedstatedir='${prefix}/com' localstatedir='${prefix}/var' -runstatedir='${localstatedir}/run' includedir='${prefix}/include' oldincludedir='/usr/include' docdir='${datarootdir}/doc/${PACKAGE_TARNAME}' @@ -1171,15 +1169,6 @@ do | -silent | --silent | --silen | --sile | --sil) silent=yes ;; - -runstatedir | --runstatedir | --runstatedi | --runstated \ - | --runstate | --runstat | --runsta | --runst | --runs \ - | --run | --ru | --r) - ac_prev=runstatedir ;; - -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \ - | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \ - | --run=* | --ru=* | --r=*) - runstatedir=$ac_optarg ;; - -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb) ac_prev=sbindir ;; -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \ @@ -1317,7 +1306,7 @@ fi for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \ datadir sysconfdir sharedstatedir localstatedir includedir \ oldincludedir docdir infodir htmldir dvidir pdfdir psdir \ - libdir localedir mandir runstatedir + libdir localedir mandir do eval ac_val=\$$ac_var # Remove trailing slashes. @@ -1470,7 +1459,6 @@ Fine tuning of the installation directories: --sysconfdir=DIR read-only single-machine data [PREFIX/etc] --sharedstatedir=DIR modifiable architecture-independent data [PREFIX/com] --localstatedir=DIR modifiable single-machine data [PREFIX/var] - --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run] --libdir=DIR object code libraries [EPREFIX/lib] --includedir=DIR C header files [PREFIX/include] --oldincludedir=DIR C header files for non-gcc [/usr/include] @@ -10918,20 +10906,13 @@ $as_echo_n "checking for --enable-loadable-sqlite-extensions... " >&6; } if test "${enable_loadable_sqlite_extensions+set}" = set; then : enableval=$enable_loadable_sqlite_extensions; else - enable_loadable_sqlite_extensions=no + enable_loadable_sqlite_extensions="no" fi + { $as_echo "$as_me:${as_lineno-$LINENO}: result: $enable_loadable_sqlite_extensions" >&5 $as_echo "$enable_loadable_sqlite_extensions" >&6; } -if test "x$enable_loadable_sqlite_extensions" = xyes; then : - - -$as_echo "#define PY_SQLITE_ENABLE_LOAD_EXTENSION 1" >>confdefs.h - - -fi - # Check for --with-tcltk-includes=path and --with-tcltk-libs=path @@ -14869,10 +14850,6 @@ fi # check if sockaddr has sa_len member { $as_echo "$as_me:${as_lineno-$LINENO}: checking if sockaddr has sa_len member" >&5 $as_echo_n "checking if sockaddr has sa_len member... " >&6; } -if ${ac_cv_struct_sockaddr_sa_len+:} false; then : - $as_echo_n "(cached) " >&6 -else - cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ #include <sys/types.h> @@ -14887,22 +14864,17 @@ x.sa_len = 0; } _ACEOF if ac_fn_c_try_compile "$LINENO"; then : - ac_cv_struct_sockaddr_sa_len=yes -else - ac_cv_struct_sockaddr_sa_len=no -fi -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext - -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_struct_sockaddr_sa_len" >&5 -$as_echo "$ac_cv_struct_sockaddr_sa_len" >&6; } -if test "x$ac_cv_struct_sockaddr_sa_len" = xyes; then : - + { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5 +$as_echo "yes" >&6; } $as_echo "#define HAVE_SOCKADDR_SA_LEN 1" >>confdefs.h +else + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 +$as_echo "no" >&6; } fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext # sigh -- gethostbyname_r is a mess; it can have 3, 5 or 6 arguments :-( @@ -18042,10 +18014,6 @@ $as_echo "$ENSUREPIP" >&6; } # check if the dirent structure of a d_type field and DT_UNKNOWN is defined { $as_echo "$as_me:${as_lineno-$LINENO}: checking if the dirent structure of a d_type field" >&5 $as_echo_n "checking if the dirent structure of a d_type field... " >&6; } -if ${ac_cv_dirent_d_type+:} false; then : - $as_echo_n "(cached) " >&6 -else - cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ @@ -18060,23 +18028,19 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext _ACEOF if ac_fn_c_try_link "$LINENO"; then : - ac_cv_dirent_d_type=yes + have_dirent_d_type=yes else - ac_cv_dirent_d_type=no + have_dirent_d_type=no fi rm -f core conftest.err conftest.$ac_objext \ conftest$ac_exeext conftest.$ac_ext +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $have_dirent_d_type" >&5 +$as_echo "$have_dirent_d_type" >&6; } -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_dirent_d_type" >&5 -$as_echo "$ac_cv_dirent_d_type" >&6; } - -if test "x$ac_cv_dirent_d_type" = xyes; then : - +if test "$have_dirent_d_type" = yes; then $as_echo "#define HAVE_DIRENT_D_TYPE 1" >>confdefs.h - fi # check if the Linux getrandom() syscall is available

tiran and others added 2 commits November 8, 2021 00:39
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>

tiran commented Nov 7, 2021

Copy link
Copy Markdown
Member Author

BTW, is configure up to date? I recreated it using autoconf 2.69 and got this diff:

Some distributions have made minor alterations to autoconf or ship different versions of autoarchive. I created https://quay.io/repository/tiran/cpython_autoconf to reduce the noise. It's a minimal Alpine image with autoconf 2.69, autoarchive, and pkg-config m4 macros.

tiran commented Nov 7, 2021

Copy link
Copy Markdown
Member Author

There more stuff that can be cached in there, and we could simplify stuff further with more macros (for example tweaking BASECFLAGS), but this PR is already pretty huge, so perhaps it's better to split it up?

Thanks for the review!

Yes, there are a handful of checks that are not cached yet. I got most slow paths in this PR. Let's handle the remaining places in a new PR. I think we can also adopt autoconf 2.71 soon. It's reducing the runtime of fully cached configure by another second.

Copy link
Copy Markdown
Contributor

Sounds like a good plan!

tiran changed the title bpo-45723: Add helper macros and more caching to configure.ac bpo-45723: Add helper macros and more caching to configure.ac (GH-29429) Nov 8, 2021
tiran merged commit 57c50c9 into python:main Nov 8, 2021
tiran deleted the bpo-45723-configure branch November 8, 2021 07:06
remykarem pushed a commit to remykarem/cpython that referenced this pull request Dec 7, 2021
…GH-29429) Almost all checks are now cached by AC_CACHE_CHECK(). Common patterns are replaced by helper macros. Variable names now use naming scheme ``ac_cv_func_$funcname``, ``ac_cv_lib_$library_$funcname``, or ``ac_cv_header_$headername_h``. ``SYS_SELECT_WITH_SYS_TIME`` is no longer used. ``uuid_create`` and ``uuid_enc_be`` are provided by libc on BSD. It is safe to use ``AC_CHECK_FUNCS`` here. Caching speeds up ./configure -C from ~ 4s to 2.6s on my system. Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
remykarem pushed a commit to remykarem/cpython that referenced this pull request Jan 30, 2022
…GH-29429) Almost all checks are now cached by AC_CACHE_CHECK(). Common patterns are replaced by helper macros. Variable names now use naming scheme ``ac_cv_func_$funcname``, ``ac_cv_lib_$library_$funcname``, or ``ac_cv_header_$headername_h``. ``SYS_SELECT_WITH_SYS_TIME`` is no longer used. ``uuid_create`` and ``uuid_enc_be`` are provided by libc on BSD. It is safe to use ``AC_CHECK_FUNCS`` here. Caching speeds up ./configure -C from ~ 4s to 2.6s on my system. Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Footer

© 2026 GitHub, Inc.