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 them is more trouble than it's worth. upstream we've talked about ways to put such data files elsewhere but in a way that they're still easily found by they 13:27 Barry : rbasak: normal apis, but so far that hasn't gone anywhere.
