Re: [squid-dev] Proposal: switch to always-build for some currently optional features
On 20/09/22 01:28, Francesco Chemolli wrote: Hi all, 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. If you mean switching their build to default-enable. Sure - but there are often good reasons for each specific item to be default disabled today: * performance expensive logic delay pools, cache digests, adaptation * unavailable dependencies adaptation, auth sub-components * rarely necessary unlinkd, wccp, delay pools, htcp * buggy delay pools, wccp Those reasons are also why we cannot simply remove the ./configure options for them (yet). We would gain: - code clarity This proposal only has a very minor gain for code clarity. The worst of that problem is all the #if/def looking for OS hacks/workarounds, and the unnecessary custom re-implementations still hanging around. - ease of development I do no think there will be any change regarding ease. Just a different way of setting up the testing. - test coverage Disagree. The default/min/max build test "layers" already build as many of these as can be tested. Plus all the reasons Alex already stated. - feature uniformity across builds I agree with most of Alex points on these. In addition, on the security side there are some passive defense benefits from feature obscurity and avoidance of a mono-culture for Squid installations. We would lose: - slightly longer build time Longer build time may not be an issue for users not building Squid often. But it would be compounding the already tough build farm situation. - larger binaries The latter should not be an issue anymore, even the most embedded of embedded systems Squid is likely to be used on has plenty of storage and core, and the former should not be too big a deal It has been 4-5 years since I had any direct customers needing embedded Squid. AIUI the needs there are for software updates on hardware that are difficult to change (eg satellites or remote geographic outposts). HTH Amos ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] Proposal: switch to always-build for some currently optional features
On 9/20/22 02:34, Francesco Chemolli wrote: 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. I agree. We could also work on better isolation, by restricting #ifdef-guarded areas to specific delegate classes, and then using c++ features (e.g. if constexpr, eventually) to disable the callsites. Yes, of course, but I would start with modules/features that require no significant refactoring. I am not sure what the best candidates are, but I would evaluate ident, cache digests, htcp, and wccp first (these are from your own list of candidates). For example, a feature like unlinkd (also on your list) would require adding a default-disabled configuration option (unlinkd_enable, similar to pinger_enable) or introducing support for a special program location spelling like "none". Adding either would break existing configurations that willingly run unlinkd. There are probably a few features/modules that do _not_ require such disruptive configuration changes. I would start with those. 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. Yes, I agree. Glad we are on the same page! I recommend giving Amos more time to leave feedback before spending time on code changes, but if we are all in agreement, then I am looking forward to PRs reducing the number of unnecessary #ifdefs. To minimize overheads, please use one PR per module/feature and avoid opening multiple concurrent PRs (at least until it is clear that we are on the same page regarding the overall approach to the corresponding code modifications). Thank you, Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] Proposal: switch to always-build for some currently optional features
> > > 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. > I agree. We could also work on better isolation, by restricting #ifdef-guarded areas to specific delegate classes, and then using c++ features (e.g. if constexpr, eventually) to disable the callsites. > 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. > Yes, I agree. -- Francesco ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] Proposal: switch to always-build for some currently optional features
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. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
[squid-dev] Proposal: switch to always-build for some currently optional features
Hi all, 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 - test coverage - feature uniformity across builds We would lose: - slightly longer build time - larger binaries The latter should not be an issue anymore, even the most embedded of embedded systems Squid is likely to be used on has plenty of storage and core, and the former should not be too big a deal Opinions? -- Francesco ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
