Code review comment for ~paelzer/ubuntu/+source/strongswan:merge-5.7.2-1-eoan

> Thanks for the review!
> Answers:
> - Some commit messages change more files than mentioned
> As explained on IRC this is due to merging changes.
> I fixed those two up.

Thanks. This saves time during reviews.

> - Q: "What happened to the pool feature?"
> A: This is only needed due to the "mass enablement", therefore it is now
> part of that (not lost and intentionally there)
> You found that later yourself.
> But I added mentioning it in the commit message as well as better indent in
> the changelog to reflect it is now part of it.

Thanks. This saves time, as the reviewer then doesn't have to go hunting down the missing piece(s).

> - Q: apparmor fixes for contianer
> A: Fixed typo and indented the sub-entries
> - Q: typos in 97c74a909
> A: yes since this will be gone I'm not cleaning it up

Agreed

> - Q: Provides: strongswan-tnc-base, but not listing others
> A: Provides is optional and only needed for packages which have dependencies
> "to them". In this case this is only strongswan-tnc-base

ok

> - Q: transitionals to get a version depends?
> A: The example at
> https://wiki.debian.org/RenamingPackages#Transition_package_method has no
> version, but it feels right. You don't need a version there, when you have a
> repo that makes the new transitonal available you also have the new version of
> the replacing package.
> I have checked other packages and they behavie ... differently.
> Some have "(= ${source:Version})" and that would work and not hurt. I'm
> adding that.

The #6 example in https://wiki.debian.org/PackageTransition actually does use a version for everything with the exception of the optional provides:

A and B existed, all from A goes into B, A becomes transitional
A: strongswan-tnc-*
B: libcharon-extra-plugins

Flags for new A package: Depends: B (>=2)
Flags for new B package:
Breaks: A (<<2)
Replaces: A (<<2)
optional* -- Provides: A

Just making sure we are reading the same thing, up to you :)

I did a quick test with the packages that are in the ppa. They don't have these latest changes yet.

Starting with these installed packages: https://pastebin.ubuntu.com/p/kgcGXbDqxX/

Dist upgrade prompted me like this:
The following packages will be REMOVED:
  strongswan-tnc-base strongswan-tnc-client strongswan-tnc-ifmap
  strongswan-tnc-pdp strongswan-tnc-server
The following packages will be upgraded:
  charon-cmd libcharon-extra-plugins libcharon-standard-plugins libstrongswan
  libstrongswan-extra-plugins libstrongswan-standard-plugins strongswan-charon
  strongswan-libcharon strongswan-starter strongswan-swanctl

I found the removal part odd. I was expecting the packages to be upgraded to the transitional ones, which I would later remove via "apt autoremove".

Furthermore, there was this issue during the transaction:
(...)
Fetched 2535 kB in 27s (92.7 kB/s)
(Reading database ... 29859 files and directories currently installed.)
Removing strongswan-tnc-client (5.7.1-1ubuntu2) ...
Removing strongswan-tnc-pdp (5.7.1-1ubuntu2) ...
Removing strongswan-tnc-server (5.7.1-1ubuntu2) ...
Removing strongswan-tnc-ifmap (5.7.1-1ubuntu2) ...
dpkg: strongswan-tnc-base: dependency problems, but removing anyway as you requested:
 libcharon-extra-plugins depends on strongswan-tnc-base.

Removing strongswan-tnc-base (5.7.1-1ubuntu2) ...
(Reading database ... 29787 files and directories currently installed.)
(...)

In the end, it seemed to have worked. But maybe with the latest changes it's better?

Installed packages after the dist-upgrade: https://pastebin.ubuntu.com/p/cXd9yyGfgD/

Dist-upgrade log: https://paste.ubuntu.com/p/M3348KsfXh/

> - Q: "Reorder conf" listed twice
> A. Fixed
>
> Thanks for the questions, I'll push an updated branch any minute ..