[Pkg-swan-devel] Bug#848890: Bug#848890: polished remaining delta for re-review

[Christian Ehrhardt]

On Mon, Dec 4, 2017 at 9:56 PM, Yves-Alexis Perez <corsac at debian.org> wrote:
> On Thu, 2017-11-30 at 16:31 +0100, Christian Ehrhardt wrote:
>> Pushed it to the same debian-submission-nov2017 branch as before.
>
> Thanks,
>
> I don't really have good news though. I took a look and first, it seems that
> the git commit entries aren't really good. git log --format=oneline is barely
> readable, for example.

Yeah the format was meant for another tool which made debian/changelog
entries out of it.
I could rewrite them the next time we talk about this topic in
probably 6 months :-)

> For the commit contents:
>
> 1aa409a5 (mass plugin enable): NACK again; I won't enable that many stuff (and
> especially not in one go)

I can understand, this all is just a suggestion.
Things came up (way before my time) due to user requests and if I did
history research correctly at that point it was decided to enable a
few more to not get requests for extra plugins all the time.
You are not taking all - that is fine, if you take a few that is good enough.
Since I wasn't part of that old enabling activity I wanted to sync
with you here and even considered dropping a bunch of them next (post
LTS) cycle.
Nobody remembers the particular reasons for some of them, so for all
that make no sense to you in a hard enough way to not even enable them
for "-extra-plugins I'd consider triggering bug reports in Ubuntu if
somebody really used them.

I'm fine either way - all I request is that we look and discuss about
things every half year or so.
So far we benefitted both each time we did, so it isn't wasted time.

> d83c243b (duplicheck disable): one good reason for the NACK just above: it's
> not enabled in Debian, you just enabled it in 1aa409a5 :)

That is a bummer, at the time I saw it the first time I thought it was
enabled in Debian and since then carried on.
/me feels ashamed and obviously drops that part :-)

> 766d4f0d (ha disable): not really sure; it's way easier to have a custom
> kernel than a custom strongSwan

Ok, so you had real cases where you or a Debian user used that?
Very interesting POV, I think neither is easier than the other but I
see your point.

> 85150f06 (kernel-libipsec enable): for reference, this is #739641 and I'm
> still not sure I like it. I might pick it but end up disabling it before
> release

It is disabled by default as Simon already outlined for the reason to
be an opt-in.

> a587dc11 (TNC move): I might pick this one because TNC is pretty standalone
> per-se and it might make sense to split it, but really that's a lot of new
> binary packages.

I understand the new-queue fight, but it really came handy for users
who wanted it standalone.

> 7629c11a7 (test-vectors move): NACK, what's the justification? Also it'll need
> some breaks/replaces
> f9e7f9007 (CCN move): NACK, what's the justification? Also, the break/replace
> have the ubuntu version in them, which is wrong for Debian.

Sorry for not converting the breaks/replaces for those two to Debian
versions properly.
@Simon - thanks for helping with arguments! (he is one of the
known-to-me Ubuntu Strongswan users).
But in this case it really is CCM.

Every time I come back to this I hate that I only inherited this delta
- too often I don't know and sometimes even fail to find the reasons
for them.
In this case due to your Nack I once again spent some more time to
search for its history.
I didn't find any and honestly I don't care anymore - let me drop
these two changes on my end (with Ubuntu breaks/replaces :-) ).

> 13300d3bf (libstrongswan.install reorder): ACK, I could pick it if there was
> an isolated changelog entry with it

I'll on rebase let this be a change+CL entry.
I'll also reorder the likely to pick changes at the top so you have
not CL conflicts.

> 4fe0861e2 (kernel-netlink conffiles): NACK, these files are installed only on
> Linux and thus the handling is done in debian/rules

Ok, it seems the original author didn't think about non-linux in this case.
I'll convert it to a d/rules change inside the DEB_HOST_ARCH_OS check

> 76535cba5 (libfast disable): Should be ok, if I have an isolated changelog
> entry for this

I'll reorder and add CL for this as well

> 8dbf648b7 (libcharon-standard-plugin): I can understand the rationale (plugins
> for common password-based mobile VPN setup), but I don't really like it. I
> don't really like adding a new binary package, and the name is definitely not
> good. Also, as far as I understand it, the plugins are useful when you're
> actually configuring a client/roadwarrior to imitate a mobile client with its
> limitations. I don't think it's a good thing to do, I'd prefer simplifying the
> secure uses cases, like pubkeys-based ones.

<insert the argument by Simon Deziel's former reply here/>

> 9b5769771 (changelog update): NACK for obvious reason, I'd need isolated
> changes to the changelog (although obviously it's not simple to cherry-pick
> them either, I think I prefer it that way).

On the rebase I will revise the changes into the format that you need.

> Regards, and again thanks for the work you do.

I have to thank you way more - for your review, your discussions and
your patience with me trying to sync&sort-out this rather old delta.
Every cycle we do on this helps me tremendously, and the 30% we take
into Debian each time helps both of us.

I'll come back with the revised version later today with all we
discussed here so far.


Thanks,
Christian Ehrhardt