Skip to content

Document ideal configure.ac structure #2341

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.

Sign up for GitHub

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

Draft
yadij wants to merge 1 commit into
base: master
Choose a base branch
from
+86 −1
Draft

Document ideal configure.ac structure #2341

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 86 additions & 1 deletion configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,28 @@
## Please see the COPYING and CONTRIBUTORS files for details.
##

#
# The layout of this file is intended to be modular with
Copy link
Contributor

@rousskov rousskov Jan 7, 2026

Choose a reason for hiding this comment

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

It is not clear to me what "modular layout" implies in this context. Please drop that phrase or rephrase to clarify.

# the following sections:
#
# 1. Autoconf initialization
#
# 2. Toolchain detection
#
# 3. Build Environment detection
#
# 4. Library detection
#
# 5. Feature detection
Comment on lines +14 to +20
Copy link
Contributor

@rousskov rousskov Jan 7, 2026

Choose a reason for hiding this comment

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

This is the primary change request.

The difference/scope of these items is unclear to me. For example, I expect "build environment" to include "toolchain" (listed above) and "library" presence (listed below). These items are further detailed down below, but those details do not help enough to address this concern and raise additional difference/dependency/scope red flags.

If we want to sort ./configure items, then let's start with what non-obvious rules or principles/ideas we want that order to enforce (instead of starting with a specific list that reflects some rules and ideas unknown to PR readers). In other words, propose the comparison function rather than the result of sorting with a comparison function unknown to the reader...

Does autoconf documentation offer any relevant advice?

#
# 6. Autoconf finalization
#
# TODO: move checks that are out of their intended section.
#

#
# Section 1: Autoconf initialization
#
AC_INIT([Squid Web Proxy],[8.0.0-VCS],[https://bugs.squid-cache.org/],[squid])
AC_PREREQ(2.61)
AC_CONFIG_HEADERS([include/autoconf.h])
Expand All @@ -15,6 +37,7 @@ AC_REVISION($Revision$)dnl
AC_PREFIX_DEFAULT(/usr/local/squid)
AM_MAINTAINER_MODE

# TODO: sort alphabetically, move library specific files to the libray section.
m4_include([acinclude/ax_with_prog.m4])
m4_include([acinclude/init.m4])
m4_include([acinclude/squid-util.m4])
Expand All @@ -29,14 +52,29 @@ m4_include([acinclude/lib-checks.m4])
m4_include([acinclude/ax_cxx_compile_stdcxx.m4])
m4_include([acinclude/win32-sspi.m4])

dnl BUG: auto-tools are not supposed to change user (C/CPP/CXX/LD)FLAGS variables.
Copy link
Contributor

@rousskov rousskov Jan 7, 2026

Choose a reason for hiding this comment

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

This code does not change those user variables, and our configure code is not auto-tools, so the BUG comment feels misleading or misplaced.

N.B. AX_CXX_COMPILE_STDCXX() (a part of "auto-tools") does change these variables. We use that macro.

Please use XXX instead of BUG.

Suggested change
dnl BUG: auto-tools are not supposed to change user (C/CPP/CXX/LD)FLAGS variables.
dnl XXX: Do not change and, hence, do not save user-controlled variables.

dnl TODO: set AM_*FLAGS as-needed instead
Copy link
Contributor

@rousskov rousskov Jan 7, 2026

Choose a reason for hiding this comment

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

In some cases, instead of using AM_*FLAGS, we should remove our code that changes those user-controlled variables to implement unnecessary environment-specific workarounds (e.g., mips-sgi-irix6 hacks).

Suggested change
dnl TODO: set AM_*FLAGS as-needed instead
dnl TODO: Remove the corresponding code or refactor it to set AM_*FLAGS where still warranted.

PRESET_CFLAGS="$CFLAGS"
PRESET_CXXFLAGS="$CXXFLAGS"
PRESET_LDFLAGS="$LDFLAGS"

dnl Set default LDFLAGS
AS_IF([test "x$LDFLAGS" = "x"],[LDFLAGS="-g"])

# check for host OS detail
#
# Section 2: Toolchain detection
#
# Locate the binaries needed to compile, distribute and install Squid.
#
# Identify any special parameters need to run the tools correctly.
# For example; "rm" vs "rm -f", "ar" vs "ar r", "grep -E" vs "egrep", or "-std=17"
Comment on lines +69 to +70
Copy link
Contributor

@rousskov rousskov Jan 7, 2026

Choose a reason for hiding this comment

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

But these "special parameters" may depend on "build environment" that is detected later on. Even a tool itself may be environment-specific.

Copy link
Contributor Author

@yadij yadij Jan 9, 2026

Choose a reason for hiding this comment

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

My concept for these at present is that this Toolchain section detects the mandatory program names and flags.
The user should not be casually setting things (can via environment, but should not).

The later Environment section has a lot of overlap, but all things in there are essentially "optional" and may be altered by the user easily with --enable/--with controls.

#
# Does not check for availability of optional settings. That happens in section 3.
#
# User override should be provided as ./configure VARIABLE="VALUE"
#

dnl check for host OS detail
AC_CANONICAL_HOST
AC_MSG_CHECKING([simplified host os])
simple_host_os=`echo $host_os|sed 's/[0-9].*//g;s/-.*//g'`
Expand Down Expand Up @@ -184,6 +222,26 @@ LT_LIB_DLLOAD
AC_SUBST(INCLTDL)
AC_SUBST(LIBLTDL)

#
# Section 3: Build Environment detection
#
# Check for which OS we are building on.
#
# Check for any OS-specific types, system includes, and toolchain
# options needed for tests in later sections to be run.
#
# Check user-provided settings for custom integration with the
# OS to be installed on.
#
# Settings found in this section will determine which components are
# able to be detected in later sections.
#
# All tests in later sections should be able to include "compat/"
# files and use their definitions.
Comment on lines +239 to +240
Copy link
Contributor

@rousskov rousskov Jan 7, 2026

Choose a reason for hiding this comment

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

We should not use compat/ sources in ./configure checks because that would imply that compat/ sources cannot reliably/safely use ./configure results. I hope such use is unnecessary. I suggest studying a specific example that does, in your opinion, require such use.

Copy link
Contributor Author

@yadij yadij Jan 9, 2026

Choose a reason for hiding this comment

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

My reason for this is to avoid duplicating the autoconf checks logic in both the AC test and compat/ files. We have repeatedly had portability issues with the two locations getting out of sync.

#
# User settings should be provided as --with-default-LABEL=VALUE.
#

AC_MSG_CHECKING(whether to use loadable modules)
AS_IF([test "x${enable_shared:=yes}" = "xyes"],[AC_DEFINE(USE_LOADABLE_MODULES,1,[Support Loadable Modules])])
AM_CONDITIONAL(ENABLE_LOADABLE_MODULES, test "x${enable_shared:=yes}" = "xyes")
Expand Down Expand Up @@ -278,6 +336,8 @@ AC_ARG_WITH(swapdir,
])
AC_SUBST(DEFAULT_SWAP_DIR)

dnl BUG: auto-tools are not supposed to change user (C/CPP/CXX/LD)FLAGS variables.
dnl TODO: set AM_*FLAGS instead
dnl Set Default CFLAGS
AS_IF([test "x$PRESET_CFLAGS" = "x"],[
AS_IF([test "$squid_cv_compiler" = "gcc"],[
Expand Down Expand Up @@ -382,6 +442,17 @@ dnl Nasty hack to get autoconf 2.64 on Linux to run.
dnl all other uses of RUN_IFELSE are wrapped inside CACHE_CHECK which breaks on 2.64
AC_RUN_IFELSE([AC_LANG_SOURCE([[ int main(int argc, char **argv) { return 0; } ]])],[],[],[:])

#
# Section 4: Library detection
#
# Check for all libraries used by Squid.
#
# Which libraries are available will determine which features
# can be enabled later.
Comment on lines +450 to +451
Copy link
Contributor

@rousskov rousskov Jan 7, 2026

Choose a reason for hiding this comment

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

I doubt we should segregate --with-foo from --enable-bar. Both can be treated as "features", and there is often no need for default-enabled library checks if a certain feature is disabled (and vice versa).

Copy link
Contributor Author

@yadij yadij Jan 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I have segregated with libraries first because many libraries are shared between multiple features. Also, it makes more sense to produce the "cannot enable feature" error message when checking the --enable than when checking the library after already having enabled the feature build.

Both can be treated as "features"

Per autoconf documentation;

  • section 15.2 --with is for external software packages (libraries or programs).
  • section 15.3 --enable is for optional features of the software being built.

#
# User settings should be provided as --with(out)-LIBRARY[=PATH]
#

AH_TEMPLATE(XMALLOC_STATISTICS,[Define to have malloc statistics])
AC_ARG_ENABLE(xmalloc-statistics,
AS_HELP_STRING([--enable-xmalloc-statistics],
Expand Down Expand Up @@ -459,6 +530,17 @@ AS_IF([test "x$ac_cv_search_shm_open" != "xno"],[
AC_DEFINE(HAVE_SHM,1,[Support shared memory features])
])

#
# Section 5: Feature detection
#
# Check which features can be built.
#
# User settings should be provided as --(en/dis)able-FEATURE[=LIST]
#
# Where LIST is a whitespace separated list of module or helper names
# to build, (if any).
#

squid_disk_module_candidates=
AC_ARG_ENABLE(disk-io,
AS_HELP_STRING([--enable-disk-io="list of modules"],
Expand Down Expand Up @@ -2458,6 +2540,9 @@ AC_MSG_NOTICE([Multi-Language support enabled: ${enable_auto_locale:=yes}])
SQUID_DEFINE_BOOL(USE_ERR_LOCALES,$enable_auto_locale,
[Use multi-language support on error pages])

#
# Section 6. Autoconf finalization
#

dnl Need the debugging version of malloc if available
XTRA_OBJS=''
Expand Down
Loading