Re: autoconf-refactor done

Kinkie wrote:
> On Wed, Aug 4, 2010 at 9:29 PM, Alex Rousskov
> <rousskov_at_measurement-factory.com> wrote:
>> On 08/04/2010 11:32 AM, Kinkie wrote:
>>> Hi all,
>>> I've pushed the hopefully final revision of the autoconf-refactor
>>> branch to launchpad (lp:~kinkie/squid/autoconf-refactor/)
>>> It's a bit out of date with trunk, I plan to update it tomorrow
>>> evening, after that it can be merged anytime.
>>> It's a 50k lines patch, so I doubt it can be posted for review here..
>>>
>>> In order to check it, I've matched that (at least with the default
>>> configure switches) the generated .h files and configure logs are
>>> consistent with those generated by trunk.
>>> The branch passes the build-test on my ubuntu box.
>> Please post the proposed commit message with the description of changes.
>> This may help with the review.
>
> Purpose of the branch is to try and give an uniform style to the
> configure.in and auxiliary scripts.
> Except for what was already merged there are no further behaviour changes.
>
> Main changes are:
> - definition of a common naming convention for shell variables
> - definition of auxiliary macros to deal with common constructs
> (--enable-* and --with-*)
> - definition of auxiliary macros to deal with autoconf defines
> - reindent to increase readability (no automated tools exist unfortunately)
> - general streamlining of the configure.in process logic, to clarify
> as much as possible what is the flow that leads to enabling or
> disabling a certain feature (e.g. IO loop engine selection)
> - modularization of configure.in by moving many inline operations to
> external macros included from auxiliary scripts; aim is to host the
> logic in configure, and the mechanisms in auxiliary scripts.
> - simplify as possible, by removing redundant checks
> - adhere to common configure.in conventions and increase portability
> (e.g. by moving away from test -z and test -n constructs, in favour of
> safer alternatives)
>
> All those goals require a few exceptions to manage the enormous
> variety of configuration options squid has; the result is IMVHO much
> more readable and easily extendable than the initial version.
>

Thank you very much for all this.

I've taken a quick look over it:

* The copy on LP still appears to need the trunk merge done.

* --disable-strict-error-checking should use SQUID_YESNO([$enableval]

* --with-logdir documentation expansion still is not working.
   can you make it say a hard-coded "Default: PREFIX/var/logs" please?

* documentation on --with-pidfile needs upper case D on default: as well.

* I think CACHE_HTTP_PORT and CACHE_ICP_PORT if statements can be
replaced with the AC_DEFINE(CACHE_HTTP_PORT,${CACHE_HTTP_PORT:=3128})
syntax.
    - Although I have doubts about whether we really need to have them
in configure any more. The ports are defacto standards and changing on
build only really effects the documentation, squidclient and cachemgr
default ports not Squid itself.

* AS_HELP tring for --disable-optimizations
      s/also enabled --disable-inline/also sets --disable-inline/

* can you wrap in this gratuitous typo fix please?
   "BSD dont include the depenencies" -> dependencies.

* can check for mtyp_t be moved up with the other type checks?

* the after CHECK_LIB(gnumalloc "esac" and "fi" are both indented too far.

* s/force-enbable/force-enable/ ... "on old solaris and nextstep"

* SQUID_CHECK_RESOLVER_FIELDS is only needed if --disable-internal-dns

  I think the update from u_int* to uint* should have been left to
another patch. But too late now.

Now that configure is refactored on to Makefile.am ;P

The following are optional and should be left until after your current
refactor is committed. I'm just noting some followup steps that would be
worth looking at.

  * uniform used of ENABLE_FOO for all AC_CONDITIONAL macros

  * the complication setting $ECAP_LIBS to ecap/libecap.la can be
replaced by the automake "if USE_ECAP" conditional to build and link the
ecap subdir library in src/adaptation/Makefile.am instead of configure.

  * same for $ICAP_LIBS.

  * wrapping of the auth libraries build+link in the ENABLE_AUTH_*

  * I don't recall why ipcache defines _dns_ttl_. We will have to look
towards removing it. Which will make SQUID_CHECK_LIBRESOLV_DNS_TTL_HACK
only be needed for --disable-internal-dns as well.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.6
   Beta testers wanted for 3.2.0.1