Re: [ptxdist] [PATCH v2 1/6] chrony: version bump 3.3 -> 3.5

[Bruno Thomsen <bruno.thomsen@gmail.com>]

Hi Michael

Thanks for review, and sorry for the mailing list spam.
I have collected replies from multiple mails in this.

Den tor. 30. jul. 2020 kl. 12.09 skrev Michael Olbrich
<m.olbrich@pengutronix.de>:
>
> On Wed, Jul 29, 2020 at 09:29:23PM +0200, Bruno Thomsen wrote:

> > Changes since version 1:
> > - Fix typo in subject line.
> > - Updated URL in package help text.
> > - Explicit enable ntp option.
> >   This outputs unrecognized option, but we still get expected result.
> >   Unrecognized option :  --enable-ntp
>
> Please go back to the version and remove the options.
> In this case I'd like to notice it if options become invalid after an
> update. Even if I need to look at the configure output.
>
> The same for the other patches.
>
> And can you add a comment, that this is not autoconf, above the configure
> options? This makes it clear why ptx/endis etc. is not used.

Okay, I will revert option selection to version 1 and add comment about
custom configure script.

> Bonus points if you replace the CROSS_AUTOCONF_USR with the actually
> supported options and add a patch to make the 'Unrecognized option' a fatal
> error. But I'll accept the patches without this part.

I have patched chrony so a fatal error happens when an unrecognized option
is detected, but added it as a separate patch in case you don't like
the solution.
So I had to replace CROSS_AUTOCONF_USR as it now caused configure
issue :)

> Is there a use-case to run chrony as root? I think we can skip the option
> and always run it as user chrony.

I am not aware of any use-case, so let's always run it as the chrony user.
But if we do that I have an extra sandbox patch I could send.

> > +chrony:x:994:989:chronyd:/dev/null:/bin/false
>
> s/chronyd/chrony/
>
> Right?

Well.. I don't think there are any real rules, but let's replace daemon
binary name with package name.

> In this case, you should install $(CHRONY_DIR)/examples/chronyd.service to
> $(CHRONY_PKGDIR)/usr/lib/systemd/system/chronyd.service in the install
> stage. And then use install_alternative.

Okay, I will try to do that.

/Bruno

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de