[squid-dev] Proposal: switch to always-build for some currently optional features

[Alex Rousskov]

On 9/19/22 09:28, Francesco Chemolli wrote:

> there is a bunch of features that are currently gated at compile 
> time: among others, I see:

> - adaptation (icap, ecap)
> - authentication
> - ident
> - delay pools
> - cache digests
> - htcp
> - cache digests
> - wccp
> - unlinkd
> 
> I'd like to propose that we switch to always-build them.

> We would gain:

> - code clarity
 > - ease of development

The above two items do not fully apply to features that depend on 
external libraries (which may be absent): eCAP, some authentication 
modules, and possibly others. Their code and related development 
overheads will remain largely unchanged. I suspect that you actually did 
not want to include optional modules with external dependencies in your 
proposal, but please clarify.


> - test coverage

To be more precise, we would gain reduction of feature _combinations_ 
that should be tested (which is a significant gain!). Basic code 
coverage by tests would remain unchanged because nearly any test can 
enable (and test) all features that can be built.


> - feature uniformity across builds

Yes, fewer features that can be enabled/disabled at build time helps 
with support.


> We would lose:
> - slightly longer build time
> - larger binaries

And:

- Larger attack surface if we always build modules like ESI. This can be 
partially mitigated by making sure we default-disable them. This is one 
of the reasons for the precondition at the end of my email.

- Some loss of performance. For example, the cache digests module, when 
enabled, builds cache digests by default (from squid.conf point of 
view). Similarly, ESI parses applicable content. There are probably also 
non-trivial slow ACL-driven checks that a module may bring in by default 
(from squid.conf point of view) if enabled.


> Opinions?

I agree that modules that can always be built, should be. Such modules 
should have no guarding #ifdefs. I think this is the set of modules that 
your proposal is targeting, but please correct me if I am wrong. FWIW, 
this design stems from an even more general/fundamental rule of thumb: 
Do not add unnecessary #ifdefs.

However, there is a precondition: Any always-built optional feature with 
a potentially significant performance impact or a controversial side 
effect should be disabled by default (via squid.conf). Satisfying this 
precondition will require code changes.


Cheers,

Alex.