| Message ID | 1411032426-11463-2-git-send-email-guillaume.gardet@oliseo.fr |
|---|---|
| State | Superseded |
| Headers | show |
Dear Guillaume GARDET, On Thu, 18 Sep 2014 11:27:04 +0200, Guillaume GARDET wrote: > This patch adds ICAP server support to squid and also defines logdir, > pidfile and swapdir. > > Signed-off-by: Guillaume GARDET <guillaume.gardet@oliseo.fr> Thanks for this contribution. A few comments below. > > diff --git a/package/squid/squid.mk b/package/squid/squid.mk > index 6dffae8..f68a402 100644 > --- a/package/squid/squid.mk > +++ b/package/squid/squid.mk > @@ -26,7 +26,11 @@ SQUID_CONF_OPT = --enable-async-io=8 --enable-linux-netfilter \ > --enable-auth-negotiate="wrapper" \ > --enable-auth-ntlm="fake" \ > --disable-strict-error-checking \ > - --enable-external-acl-helpers="file_userip" > + --enable-external-acl-helpers="file_userip" \ > + --with-logdir=/var/log/squid/ \ > + --with-pidfile=/var/run/squid.pid \ > + --with-swapdir=/var/cache/squid/ \ > + --enable-icap-client > > # On uClibc librt needs libpthread > ifeq ($(BR2_TOOLCHAIN_HAS_THREADS)$(BR2_TOOLCHAIN_USES_UCLIBC),yy) > @@ -45,6 +49,10 @@ define SQUID_CLEANUP_TARGET > cachemgr.conf mime.conf.default squid.conf.default) > endef > > -SQUID_POST_INSTALL_TARGET_HOOKS += SQUID_CLEANUP_TARGET > +define SQUID_CREATE_MISSING_FOLDER > + mkdir -p $(TARGET_DIR)/var/log/squid/ > +endef > + > +SQUID_POST_INSTALL_TARGET_HOOKS += SQUID_CLEANUP_TARGET SQUID_CREATE_MISSING_FOLDER > > $(eval $(autotools-package)) I believe the changes of adding the log directory, pidfile and swapdir on one side, and adding the ICAP client option on the other side should be two separate patches. Also, how much does the ICAP client support adds to Squid size? I'm trying to see if this should be optional instead of always enabled like you're proposing. Thanks, Thomas
Hi Thomas, Le 05/10/2014 23:52, Thomas Petazzoni a écrit : > Dear Guillaume GARDET, > > On Thu, 18 Sep 2014 11:27:04 +0200, Guillaume GARDET wrote: >> This patch adds ICAP server support to squid and also defines logdir, >> pidfile and swapdir. >> >> Signed-off-by: Guillaume GARDET <guillaume.gardet@oliseo.fr> > Thanks for this contribution. A few comments below. > >> diff --git a/package/squid/squid.mk b/package/squid/squid.mk >> index 6dffae8..f68a402 100644 >> --- a/package/squid/squid.mk >> +++ b/package/squid/squid.mk >> @@ -26,7 +26,11 @@ SQUID_CONF_OPT = --enable-async-io=8 --enable-linux-netfilter \ >> --enable-auth-negotiate="wrapper" \ >> --enable-auth-ntlm="fake" \ >> --disable-strict-error-checking \ >> - --enable-external-acl-helpers="file_userip" >> + --enable-external-acl-helpers="file_userip" \ >> + --with-logdir=/var/log/squid/ \ >> + --with-pidfile=/var/run/squid.pid \ >> + --with-swapdir=/var/cache/squid/ \ >> + --enable-icap-client >> >> # On uClibc librt needs libpthread >> ifeq ($(BR2_TOOLCHAIN_HAS_THREADS)$(BR2_TOOLCHAIN_USES_UCLIBC),yy) >> @@ -45,6 +49,10 @@ define SQUID_CLEANUP_TARGET >> cachemgr.conf mime.conf.default squid.conf.default) >> endef >> >> -SQUID_POST_INSTALL_TARGET_HOOKS += SQUID_CLEANUP_TARGET >> +define SQUID_CREATE_MISSING_FOLDER >> + mkdir -p $(TARGET_DIR)/var/log/squid/ >> +endef >> + >> +SQUID_POST_INSTALL_TARGET_HOOKS += SQUID_CLEANUP_TARGET SQUID_CREATE_MISSING_FOLDER >> >> $(eval $(autotools-package)) > I believe the changes of adding the log directory, pidfile and swapdir > on one side, and adding the ICAP client option on the other side should > be two separate patches. Ok, will submit two patches then. > > Also, how much does the ICAP client support adds to Squid size? I'm > trying to see if this should be optional instead of always enabled like > you're proposing. Without: 2.7 MB,and with c-icap: 3.0 MB Should I make it optionnal? Guillaume
Hello, On Wed, 08 Oct 2014 22:03:27 +0200, Guillaume GARDET - Oliséo wrote: > > I believe the changes of adding the log directory, pidfile and swapdir > > on one side, and adding the ICAP client option on the other side should > > be two separate patches. > > Ok, will submit two patches then. Great thanks. > > Also, how much does the ICAP client support adds to Squid size? I'm > > trying to see if this should be optional instead of always enabled like > > you're proposing. > > Without: 2.7 MB,and with c-icap: 3.0 MB > > Should I make it optionnal? I would tend to say yes. Making it optional not only saves space, but it also makes it explicit that our Squid build can support the ICAP client functionality. Thanks! Thomas
On 10/08/2014 05:18 PM, Thomas Petazzoni wrote: >> Without: 2.7 MB,and with c-icap: 3.0 MB >> >> Should I make it optionnal? > > I would tend to say yes. Making it optional not only saves space, but > it also makes it explicit that our Squid build can support the ICAP > client functionality. I wouldn't mind keeping it as is, squid is a big RAM and storage sinkhole so it's a no-go for memory-constrained targets in general, after all it's a caching (in disk) proxy with the RAM usage for keeping indexes at hand. Regards.
Le 08/10/2014 22:24, Gustavo Zacarias a écrit : > On 10/08/2014 05:18 PM, Thomas Petazzoni wrote: > >>> Without: 2.7 MB,and with c-icap: 3.0 MB >>> >>> Should I make it optionnal? >> I would tend to say yes. Making it optional not only saves space, but >> it also makes it explicit that our Squid build can support the ICAP >> client functionality. > I wouldn't mind keeping it as is, squid is a big RAM and storage > sinkhole so it's a no-go for memory-constrained targets in general, > after all it's a caching (in disk) proxy with the RAM usage for keeping > indexes at hand. > Regards. > > Then, should I make it optionnal or not? I think it should be ok to keep it included as it will never be used on underpowered SoC. Thomas, would you be ok with that? Guillaume
Dear Guillaume GARDET - Oliséo, On Thu, 16 Oct 2014 11:52:19 +0200, Guillaume GARDET - Oliséo wrote: > Then, should I make it optionnal or not? I think it should be ok to > keep it included as it will never be used on underpowered SoC. > > Thomas, would you be ok with that? Yes, fair enough, keeping it always enabled is fine with me. Thanks, Thomas
diff --git a/package/squid/squid.mk b/package/squid/squid.mk index 6dffae8..f68a402 100644 --- a/package/squid/squid.mk +++ b/package/squid/squid.mk @@ -26,7 +26,11 @@ SQUID_CONF_OPT = --enable-async-io=8 --enable-linux-netfilter \ --enable-auth-negotiate="wrapper" \ --enable-auth-ntlm="fake" \ --disable-strict-error-checking \ - --enable-external-acl-helpers="file_userip" + --enable-external-acl-helpers="file_userip" \ + --with-logdir=/var/log/squid/ \ + --with-pidfile=/var/run/squid.pid \ + --with-swapdir=/var/cache/squid/ \ + --enable-icap-client # On uClibc librt needs libpthread ifeq ($(BR2_TOOLCHAIN_HAS_THREADS)$(BR2_TOOLCHAIN_USES_UCLIBC),yy) @@ -45,6 +49,10 @@ define SQUID_CLEANUP_TARGET cachemgr.conf mime.conf.default squid.conf.default) endef -SQUID_POST_INSTALL_TARGET_HOOKS += SQUID_CLEANUP_TARGET +define SQUID_CREATE_MISSING_FOLDER + mkdir -p $(TARGET_DIR)/var/log/squid/ +endef + +SQUID_POST_INSTALL_TARGET_HOOKS += SQUID_CLEANUP_TARGET SQUID_CREATE_MISSING_FOLDER $(eval $(autotools-package))
This patch adds ICAP server support to squid and also defines logdir, pidfile and swapdir. Signed-off-by: Guillaume GARDET <guillaume.gardet@oliseo.fr> --- package/squid/squid.mk | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)