Bug#849754: RFS: guerillabackup/0.0.0-1
Hello halfdog,
Thanks for your interest in debian packaging....
On Fri, Dec 30, 2016 at 03:16:55PM +0000, halfdog wrote:
> Package: sponsorship-requests
> Severity: normal
>
> Dear mentors,
>
> I am looking for a sponsor for my package "guerillabackup"
[...]
> dget -x https://mentors.debian.net/debian/pool/main/g/guerillabackup/guerillabackup_0.0.0-1.dsc
[...]
>
> As also stated in comment to https://mentors.debian.net/package/guerillabackup
> to avoid reviewers wasting time searching for dirty little package
> secrets, here are some pointers to things, I had problems with,
> when creating the package. Reviewers might disagree with my proposed
> solution, any feedback is very welcome!
>
> * Upstream Debian file templates: to support building of native
> packages using only the upstream source, Debian package files
> and build instructions are included already in upstream. To
> avoid duplication of them when not (yet) needed, they are copied
> within "rules" in target "override_dh_auto_configure"
Not a fan here. :P
>From a Debian(-only) perspective this complicates things for no
real gain. If you package things in Debian it's probably very
unlikely people will get their packages from elsewhere, specially
if they need to both build it themselves and follow a procedure
for doing so that's completely alien...... (but what do I know
about the actual problem you're trying to solve.)
I'm hoping DEP14 can instead be a replacement solution
for handling this (and also other reasons).
>
> * (Re)starting units on upgrade: As stated in documentation, two
> services can be used also from commandline (on demand) or as
> non-root user, depending on end user use cases. Thus it is intended,
> that the two systemd units are not enabled by default. Also
> a user may start them manually without enabling them. With upgrade
> following problem may arise: with standard debhelper means it
> was not possible to
> 1) stop all running units and
> 2) after update start only those, that were running beforehand.
> Solution:
> 1) do not use debhelper for stopping/starting of units,
> 2) find out in "prerm" which units are currently running, stop them and
> 3) in "postinst" start only those, that were running in step 1)
Pretty please do not try to reinvent systemd integration on your own.
This is just way to easy to get wrong. If the current helpers does
not work for you it's either likely because you're using them wrong
and/or because they should be improved. Anything else is likely just
causing extra work and pain.
Please swing by either the irc channel or contact the mailing list
for the Debian systemd maintainers. They're very skilled and usually
happy to help (time permitting). They are likely also the people
you need to get to review your package anyway if you invent your
own maintainer script scheme.
>
> * Use of .pyc files: As I do not fully understand the consequences
> of using .pyc files, especially in conditions where backup might
> be more important, e.g. when disks start already failing and
> py/pyc files might fall out of sync, I decided not to use them
> until I understand the possible risks. As codebase is very small
> and program is long-running, overhead from JIT-compiling should
> be not an issue.
Not an expert on python packaging myself, but I think Debian python
packaging helpers should generate postinst code to automatically
generate the .pyc files on install. I guess the reason you can't
ship them is because then you need to build them with the lowest
supported capability set of the architecture (which itself is
likely hard to do). In short, the debian way is to just rm *.pyc *.pyo
and trust the helpers to do the right thing.
Below is a full packaging review with some pointers, hope it helps:
Vcs-* -> should point to debian packaging repository, not upstream.
(Note: does not have to be a separate repository. See DEP14 branch
naming scheme for example. Still, Vcs-* fields should point to Debian
specific parts. For information pointing to upstream see
https://wiki.debian.org/UpstreamMetadata )
Section -> please consider looking up the correct section for backup
utilities at https://packages.debian.org/unstable/
(Please note the section name is not the one on the list, click the link
to see the actual name.)
python2 only is unfortunate. Please note that porting everything in
Debian to python3 was already a goal for this release. If you're not
using python3 in a new package at this point that leaves me wondering
about your chances for ever being part of the next release.
(Python is not by strongest side though, specially not debian packaging
of it, so please find more advanced advice on this from someone else.)
dh compat 9 -> consider using latest dh compat 10 instead. This
automatically includes dh-systemd, etc. It will also use 'restart
/after/ upgrade' by default.
* DEP14 rather than copying packaging during configure.
* restart instead of stop+start. Also contact debian systemd maintainers
for advice.
Multiple services: Need to manually list each for dh_systemd_* to pick
them up.
Maintainer scripts: Avoid them.
Please consider shipping the files with correct permissions rather than
setting them in postinst. (That would also close the unpack->configure
window of them having the incorrect permissions.)
Please do not invent your own systemd integration glue rather than using
the existing helpers. It's just way to easy to get it wrong and you're
adding extra work on people reviewing your package.
Your postrm file seems to contain nothing useful. Just remove it and
it'll be completely generated when needed. Following earlier advice
the postrm, etc. files should also become pointless -> removed.
Regards,
Andreas Henriksson
Reply to: