Code review comment for ~philroche/ubuntu/+source/google-guest-agent:feature/ubuntu/mantic-devel-no-start-services-on-upgrade

The history of these is:
20200617.00-0ubuntu1
   dh_installsystemd --no-restart-after-upgrade --no-restart-on-upgrade google-startup-scripts.service google-shutdown-scripts.service
20200617.00-0ubuntu5
      * debian/rules: Don't stop or start google-startup-scripts and
        google-shutdown-scripts services
- dh_installsystemd --no-restart-after-upgrade --no-restart-on-upgrade google-startup-scripts.service google-shutdown-scripts.service
+ dh_installsystemd --no-start --no-restart-after-upgrade google-startup-scripts.service google-shutdown-scripts.service
And now due to the bug changing it (for the same reason again of not starting).
- dh_installsystemd --no-start --no-restart-after-upgrade google-startup-scripts.service google-shutdown-scripts.service
+ dh_installsystemd --no-start --no-stop-on-upgrade google-startup-scripts.service google-shutdown-scripts.service

That looks reasonable.

With that the commit and changelog message is a bit odd, you are not "restoring" --no-stop-on-upgrade.
You are restoring the behavior of not stopping, but not the argument, that was never there so far.
And only due to changes in the defaults it now causes issues.

The whole second part of the changelog "Previously an upgrade of the google-guest-agent package would restart google-startup-scripts.service and google-shutdown-scripts.service which was unexpected and if google-shutdown-scripts was performing destructive actions then this could cause issues." seems like a detailed description of the bug. It is to some extent style and you might want to keep it, but TBH I'd remove that part completely and only reference the bug which describes it in even more detail already.

Furthermore I think we need to track down and explain why this is a problem in Jammy but not in focal. The only reason you'd usually have is a change in the compat level, but we have:
$ git show pkg/ubuntu/jammy-devel:debian/control | grep debhelper-compat
Build-Depends: debhelper-compat (= 12),
$ git show pkg/ubuntu/focal-devel:debian/control | grep debhelper-compat
Build-Depends: debhelper-compat (= 12),

Is it really not also affecting focal?
Before doing a hurried change, you should test building this on focal and check the resulting maintainer scripts. How do they behave and why?

Furthermore,
almost always when people use "--no-start", they should also think about "--no-enable".
Because as proposed it would install (and not start the service).
But once rebooted the service would be active.
Is that the behavior you want, or should we - while touching it - also add that flag?
And if you want it started after boot, why would you not want to start it on install?
Could it be that --no-start is only there because of past tries to get a no-restart behavior?
If so, could it be that you might want to start one but not the other?
You see, there are questions that should be answered to make this good in this upload and not cause another few to be needed :-)

So overall, would you mind:
1. reworking the changelog entry to state more correctly what we do (e.g. not restoring)
2. once proven stating why it is a problem in jammy now, but not in e.g. Focal (can be done very shortly or not in changelog, but in details in the SRU template on the bug)
3. add --no-enable or state in the bug in detail why you do not want/need it

In general, this is mantic only for now, before SRUs the SRU template on the bug needs to be completed. I generally recommend to fill this out right in the beginning, as it helps to know you can answer all those questions when the process comes to them.