Re: [ptxdist] (EXT) Re: [PATCH v2] syslogng: update to 3.25.1

[Michael Olbrich <m.olbrich@pengutronix.de>]

On Fri, Feb 21, 2020 at 09:30:30AM +0100, Matthias Schiffer wrote:
> On Fri, 2020-02-21 at 08:47 +0100, Michael Olbrich wrote:
> > On Thu, Feb 20, 2020 at 09:39:47AM +0100, Matthias Schiffer wrote:
> > >  	--with-systemd-journal=$(call ptx/ifdef,
> > > PTXCONF_SYSLOGNG_SYSTEMD,system,no) \
> > >  	--with-systemdsystemunitdir=/usr/lib/systemd/system \
> > >  	--localstatedir=/var/run \
> > > +	--with-ivykis=internal \
> > > +	--with-jsonc=$(if $(PTXCONF_SYSLOGNG_JSON),system,no) \
> > >  	--with-libnet=$(SYSROOT)/usr/bin \
> > > -	--with-python=$(PYTHON_MAJORMINOR)
> > > +	--with-python=$(PYTHON3_MAJORMINOR)
> > 
> > As I noted in my last review, please use configure_helper.py to check
> > and
> > improve the options.
> > - GLOBAL_LARGE_FILE_OPTION should be used
> > - the sorting should be corrected
> > - there are several enable/disable and with/without options that are
> >   missing and may pick up dependencies automatically.
> 
> Ah, I didn't know about GLOBAL_LARGE_FILE_OPTION, and mostly used
> configure_helper.py to compare the old and the new syslogng version.
> 
> Should I list *all* options that configure supports, even when the
> defaults are fine? I thought I had caught everything that has an actual
> effect. I guess disabling libcurl etc. could be made explicit as well,
> but these deps are only used when the corresponding modules are
> enabled.
> 
> I also found the option --disable-all-modules, which overrides the
> defaults for various other options to "no" - which would allow us to
> make the SYSLOGNG_AUTOCONF section shorter, but the diff shown by
> configure_helper.py larger. Which way should I go here?

To make updates easier, I prefer a shorter configure_helper.py output:
- If it makes sense, then the output should be empty (probably not
  reasonable here)
- options that obviously don't matter are find
  - stuff like Windows or OSX specific options
  - some --with-* options for extra search paths
  ...
- any option that avoids / forces a autodetect check _must_ be specified.
  Never depend on the auto detections.
- not adding options that can overwrite a fixed value (paths etc.) is ok
- make sure the options are sorted correctly

I hope this helps.

Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de