Re: Intro and Intent to Package

[Samuel Henrique <samueloph@debian.org>]

Hello Christian,

>   https://mentors.debian.net/package/nsjail

Awesome, you don't need to bother with mentors since we are working as
a team and we have salsa, anytime you need a review/sponsor you can
ask for one and we will take a look at the head of debian/master, that
will make things slightly easier.

> A few things:
> - The package looks lintian clean, except for the distribution. Is
> "UNRELEASED" the
>   correct distribution for now? Or should I change this to "experimental"?

The correct distribution will be unstable, that is unless you have an
explicit reason to keep it experimental, but those cases are rare and
I don't think it applies here.
Usually is a good practice to only change when the package is ready to
be uploaded, but that's on a best effort basis as a way of minimizing
the overhead, this means you are free to err on the side of setting
the release to unstable before the package is ready instead of waiting
for someone to review to perform the change, sometimes the
reviewer/sponsor will do the change themselves when it's ready.
So the gist of it is that you can either set it to unstable when you
think the package is ready or wait for the sponsor to do so, it's up
to you.

> - Since I used CDBS, the maximum debhelper compat version I could use is 10.
>   IIUC, this is fine?

I took a look at d/rules and it looks like you're not using anything
specific to cdbs, I recommend switching to debhelper fully as cdbs is
considered legacy.
The switch should be fairly easy, you just need to remove cdbs from
B-D and add the vanilla debhelper catch-all target on d/rules:
%:
    dh $@

Note the tab usage (on d/rules), you can take a look at other packages
from the team as a reference. I also recommend using
"debhelper-compat" in d/control, as the other packages, so you can
remove the file d/compat (debhelper-compat declares both the compat
level and the debhelper version).

With the move to debhelper, you will be able to use version 13.

> - Upstream does not provide GPG signatures, so I can ignore the
>   debian-watch-does-not-check-gpg-signature warning, right?

That's right, strictly speaking only lintian errors are blockers, the
rest of them are good to fix but sometimes it's not something doable
from the packaging side. In this specific case we are talking about a
possible improvement that needs to be done on the upstream side
(provide a key and signature of tarballs). Considering you're also
upstream you can fix it, but it's totally up to you.

> Let me know what you think. I'm also a bit unsure about the repo management.
> Upstream lives in GitHub and uses Git submodules. For now I uploaded a snapshot
> of the full source tree to debian/master, is that acceptable? What's
> the best practice
> here?

Sorry I didn't review the package yet, so I will comment with a
general case in mind;
If you package a snapshot of a repo, the version needs to have this
information appended in the upstream part of it, like in the example
of one package we have:
3.7.18+git20200706.10c91c7-1
instead of 3.7.18-1
Note how the version gives information about the date of the snapshot
and the commit picked. This version manipulation is something we do on
the packaging side when importing the upstream tarball.

Submodules are not an issue, but they might be a little bit
cumberstone if the tarballs provided by upstream don't have them
checked in (that is, when the submodule usage is not abstracted away
in the tarball).
Sometimes there are issues related to submodules, but they happen
because the submodule is a library that should be packaged separately.
Not all libraries need/should be packaged separately, but some do, so
it's on a per case basis.

I will review the package soon, so I will be able to give a more
specific feedback,

Thanks for your work,


--
Samuel Henrique <samueloph>