GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-45723: Add helper macros and more caching to configure.ac (GH-29429) by tiran · Pull Request #29429 · python/cpython

Conversation

@tiran

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

Uh oh!

There was an error while loading. Please reload this page.

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 [email protected]

https://bugs.python.org/issue45723

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gpshead, pablogsal, and erlend-aasland reacted with heart emoji
All reactions
  • ❤️ 3 reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Nov 5, 2021
@bedevere-bot bedevere-bot added the awaiting core review label Nov 5, 2021
@gpshead
gpshead requested review from erlend-aasland and pablogsal November 5, 2021 21:56
@gpshead

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 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

bedevere-bot commented 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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

pablogsal commented Nov 5, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Outstanding!! 🚀🚀

tiran reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tiran

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tiran
tiran force-pushed the bpo-45723-configure branch from f13a97f to f9bfcf2 Compare November 6, 2021 10:07
@tiran
bpo-45723: Add helper macros and more caching to configure.ac
a52ce97
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 <[email protected]>
@tiran
tiran force-pushed the bpo-45723-configure branch from f9bfcf2 to a52ce97 Compare November 7, 2021 11:05
erlend-aasland
erlend-aasland reviewed Nov 7, 2021
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

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.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented Nov 7, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tiran and others added 2 commits November 8, 2021 00:39
@tiran Erlend Egeberg Aasland
Apply suggestions from code review
69cf063
Co-authored-by: Erlend Egeberg Aasland <[email protected]>
@tiran
Regenerate files
2b6ef5a
@tiran

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.

erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tiran

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented Nov 7, 2021

Copy link
Copy Markdown
Contributor

Sounds like a good plan!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland
erlend-aasland approved these changes Nov 7, 2021
View reviewed changes
@tiran 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
tiran merged commit 57c50c9 into python:main Nov 8, 2021
@tiran
tiran deleted the bpo-45723-configure branch November 8, 2021 07:06
@bedevere-bot bedevere-bot removed the awaiting core review label Nov 8, 2021
remykarem pushed a commit to remykarem/cpython that referenced this pull request Dec 7, 2021
@tiran Erlend Egeberg Aasland remykarem
bpo-45723: Add helper macros and more caching to configure.ac (python…
ed1cf52
…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 <[email protected]>
remykarem pushed a commit to remykarem/cpython that referenced this pull request Jan 30, 2022
@tiran Erlend Egeberg Aasland remykarem
bpo-45723: Add helper macros and more caching to configure.ac (python…
9f59c57
…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 <[email protected]>
@tiran tiran mentioned this pull request Apr 22, 2022
Improve and simplify configure.ac checks #89886
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aasland erlend-aasland erlend-aasland approved these changes
@pablogsal pablogsal Awaiting requested review from pablogsal

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@tiran @gpshead @bedevere-bot @pablogsal @erlend-aasland @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.