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

Christian Ehrhardt christian.ehrhardt at canonical.com
Tue Dec 5 07:31:20 UTC 2017

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.

Christian Ehrhardt

More information about the Pkg-swan-devel mailing list