[DTrace-devel] [PATCH] dtprobed: make sure the daemon is restarted

[Kris Van Hees]

On Fri, Feb 23, 2024 at 11:02:57PM +0000, Nick Alcock wrote:
> On 23 Feb 2024, Kris Van Hees said:
> 
> > On Fri, Feb 23, 2024 at 08:57:50PM +0000, Nick Alcock wrote:
> >> diff --git a/dtrace.spec b/dtrace.spec
> >> index 278bdf53b7e1e..f583d59132e9e 100644
> >> --- a/dtrace.spec
> >> +++ b/dtrace.spec
> >> @@ -207,6 +207,20 @@ rm -rf $RPM_BUILD_DIR/%{name}-%{version}
> >>  /sbin/ldconfig
> >>  %udev_rules_update
> >>  %systemd_post dtprobed.service
> >> +# Force a daemon restart on upgrade even if the previous package did
> >> +# not use presets and did not request a restart on uninstallation.
> >> +# (In OL10, when upgrades from 1.13.1 and below are no longer possible,
> >> +# we can remove this.)
> >
> > I would drop the "In OL10".  All that matters is the DTrace version.
> 
> Not really -- in OL10 we can drop it because no affected DTrace version
> would ever have been installed. Before then, we have to retain this just
> in case someone avoids updating from an affected version until some much
> later release in OL[789] -- but you can't update an OL9 package to an
> OL10 one, you have to reinstall. So we're safe :)

Not talking about safety.  Just don't see a need to mention OL10 (which does
not even exist).  The sentence is perfectly fine without it.

> >> +# systemd_postun_with_restart does the right thing here, though we need
> >> +# to wrap it in an extra conditional to make it not run on install.
> >> +if [ $1 -gt 1 ] ; then
> >
> > This is confusing because your commit msg mentions >= 2 (which is the same,
> > but appears confusing that you wouldn't use the same expression in both
> > places).
> 
> This is the mechanism suggested by Fedora's packaging policy. I checked
> and it really is idiomatic (if also a bit weird). I could say "greater
> than 1", but really what matters is that the value is 2 -- it's just
> that RPM implementation details around installation of multiple packages
> at once might make it 3 or 5 or 50 but you're meant to treat it as if it
> were 2. :)

You mean to say that RPM uses a shell here that does not support $1 -ge 2 ??
Seriously?

> (Is RPM kinda crazy, yes, yes it is.)