Hi,
I tried to improve things, and for the ones that needs more time or that are
more nice-to-haves, I've put a notice in the README.source as advised by Robie.
Blocker for upload as there might be upgrade path issues later
otherwise: ln -sf /usr/share/doc/kimchi/examples/kimchi.sub.nginx
/etc/nginx/sites-available/kimchi (from postinst) will break policy
https://www.debian.org/doc/debian-policy/ch-docs.html#s12.3 Packages
must not require the existence of any files in /usr/share/doc/ in order
to function and also the user expectation that a file in /etc can just
be modified and normally conffile handling will occur. I suggest that
you just install (duplicate the example in
/usr/share/doc/kimchi/examples if you wish) to /etc as a regular
conffile (so putting it in kimchi.install will do) instead.
I moved the copy of kimchi.sub.nginx to debian/rules.
I kept this in kimchi.postinst though :
ln -sf /etc/nginx/sites-available/kimchi /etc/nginx/sites-enabled/kimchi
With just this fixed I will be happy to upload the package to Ubuntu and
recommend to a DD that it is suitable for Debian (though it will still
need the DD to review).
package-contains-broken-symlink might be worth a lintian override as the
symlink gets resolved when dependencies are fulfilled so is a false
positive.
done
I don't think the pedantic lintian warnings are an issue - if upstream
don't sign releases then you can't verify them and you're already
dealing with minified JS by minifying in the build and/or depending on
the jquery package as appropriate. It might be worth writing a note for
the Debian ftpmaster and/or Ubuntu archive admin explaining it though,
perhaps in README.source?
done
The postinst and postrm restart kimchid and nginx daemons. Does this
intefere with dh_installinit #DEBHELPER# snippets at all? I see in the
binary deb after build that it looks like it'll cause kimchi to be
restarted twice on future upgrade, for example. I'm not sure how this
could be improved though due to the need to restart nginx from kimchi's
postinst. Maybe systemd might have some capability in this area?
This needs more investigation on my side or maybe other reviewers here can have
more knowledge on this. The point is :
- kimchi generate certificate at first run
- nginx needs to be restarted to take those into account
Added to README.source
Maybe with systemd we can do something smarter but the support of sysV init
scripts in debian won't help on solving the problem in a global manner.
override_dh_auto_test is used to skip tests. It would be nice to be able
to run any upstream tests at package build time, and/or have dep8 tests
that can run upstream tests, but I appreciate that this may be difficult
due to virt requirements.
This can not be simply done indeed as the tests create VMs. I've added this to
the
README.source and will check with upstream, what can be done.
Upstream currently generate custom DH parameters at build time. This
will not help with uniqueness in binary distributions like Debian and
Ubuntu and also break reproducible builds. Instead this could be done at
install time in the postinst, though that might result in entropy
issues, or we could ship well-known parameters instead, or by some kind
of user choice between the two. But I am advised by a colleague in
Ubuntu security team that this isn't an immediate security issue as-is.
ok ; added this to README.source
I see debian/kimchid.init but this doesn't appear in the built binary.
Should this be fixed or removed? Shipping just a systemd unit file
is fine for Ubuntu; AIUI in Debian it's friendly to maintain it for
users who choose a different init system.
I renamed kimchid.init to kimchi.init so that it's taken into account.
I renamed the service file so that it matches kimchi.service else lintian
complains for having the init file and service file with diffferent package
name.
Maybe remove /usr/share/kimchi/doc/kimchid.8 as it is shipped in
/usr/share/man/man8/kimchid.8.gz?
Added rm in d/rules
Should /usr/lib/python2.7/dist-packages/kimchi/API.json be in
/usr/share/kimchi instead? I'm not really sure about this one. It does
seem unusual to have it where it is though.
As you told me after checking, it's ok there ; including here your discussion
with Barry :
rbasak : barry: could you please advise if installing eg.
./usr/lib/python2.7/dist-packages/kimchi/plugins/ginger/API.json is acceptable
or if that should be in /usr/share/kimchi instead? It looks unusual to me so
I thought I'd check. This is review for a new package.
10:06 rbasak : barry(I'd normally expect to see only .py files in
/usr/lib/python2.7/dist-packages)
10:07 Barryr : basak: tl;dr: it's fine. many python libraries do include data
files in
their directory structure, and that works well with pkg_resources. they can
use that api to find the data files easily. it does seem odd that those data
files don't go in /usr/share but it's normal. moving
