Bug#1024608: RFS: n2n/3.1.1-0.1 [NMU] -- Peer-to-Peer VPN network daemon

[Andreas Henriksson <andreas@fatal.se>]

Hello Tianyu Chen,

I've had a quick look at your proposed NMU and have some comments below.

On Tue, Nov 22, 2022 at 12:45:17PM +0800, Tianyu Chen wrote:
> Package: sponsorship-requests
> Severity: normal
> X-Debbugs-Cc: billchenchina2001@gmail.com
> 
> Dear mentors,
> 
> I am looking for a sponsor for my package "n2n":
> 
>  * Package name     : n2n
>    Version          : 3.1.1-0.1
>    Upstream contact : Luca Deri <deri@ntop.org>
>  * URL              : http://www.ntop.org/products/n2n/
>  * License          : Expat, __AUTO_PERMISSIVE__, GPL-3.0+
>  * Vcs              : https://github.com/leggewie-DM/n2n
>    Section          : net
> 
> Though "n2n" is already in debian, but it's already behind upstream(See #914321
> and #1024139). I'm uploading a NMU for that.
[...]
> Changes since the last upload:
> 
>  n2n (3.1.1-0.1) unstable; urgency=medium
>  .
>    * Non-maintainer upload.
>    * New upstream version 3.1.1. (Closes: #914321, #1024139)

First, an NMU is expected to be minimal changes needed. It is thus
unexpected to see that you're completely gutting the existing packaging
and replacing it with what is essentially a new from-scratch packaging.
(specially since debian/changelog gives no clues about this.)

I understand that a major new upstream version bump it might not have
been possible to preserve much of the old stuff, but still.... it makes
it quite hard for the reviewer.

I'm also wary about your transition to machine-readable debian/copyright
which throws away all existing copyright notices for existing package.
I'm not a lawyer, so can't say what's correct.... I guess it can be
argued both ways that you've completely replaced debian/ with new stuff
or that no matter what it's still a derived work of the existing
package. Either way, the new debian/copyright not having any explicit
stanza for `Files: debian/*` makes me a bit worried.

Also a nitpick, guidance like the below should IMHO be removed once
you've done it:

```
# Please double check copyright with the licensecheck(1) command.
```

Many other files also has this comment (which you can also remove once
you've followed the advice):

```
# You must remove unused comment lines for the released package.
```

(Another nitpick is that most files in debian/* starts with an empty
line.... remove it?!)


The old package had a /etc/init.d/n2n and in the new package you've
introduced systemd service files.... with different naming!
- How will upgrades works for existing users?
- you're manually installing from debian/systemd/* (possibly because
  using template/instance units) which means you get no automatic
  debhelper integration generating maintscript code for you... and I
  don't see any manual either. Even if you want to ship
  disabled-by-default (as from docs there's no default configuration
  file installed, just a sample), you should likely issue a restart on
  upgrades atleast.


In debian/rules you're manually hooking in dh_update_autotools_config
... when using dh compat 13 this is AFAIK handled automatically.
You're also build-deping on cmake while at the same time specifying in
debian/rules explicitly that you want to use autotools build system.
Why?


I'm having a hard time getting through all of this as a reviewer. One
trivial example that makes it more annoying then necessary for a
reviewer is debian/patches/move-man-pages.patch which makes the debdiff
very verbose. Why not just submit something like this to upstream and
then pull it in when upgrading to next upstream release?!
If you really want to move files around, making a less noisy diff can be
accomplished by using mv in debian/rules instead. Then it would be much
more descriptive for a reviewer exactly what you're doing (rather than
including a patch that is suitable for upstream submission).

As is I can't comfortably sponsor this (but maybe someone else having a
look has a different view), specially this close to the freeze.

My personal recommendation would be that if it's too late to do minor
incremental updates then pursue your ITS (rather than a NMU).
Also please consider how upgrading existing users will work out. If you
see no other way than the local sysadmin doing manual changes, then
atleast write about this in debian/NEWS (see man dh_installchangelogs)
and possibly also write some guidance in debian/README.Debian about
how the integration works in Debian specifically and how the old
installations configuration maps to the new.


Regards,
Andreas Henriksson