On 09/23/2011 03:54 AM, Daniel Kauffman wrote: > On 09/22/2011 02:38 AM, Thomas Goirand wrote: >> On 09/22/2011 03:32 PM, Daniel Kauffman wrote: >>> >>> I didn't see a reference to quilt in the Debian Policy Manual and the >>> Debian New Maintainers' Guide section 2.9 seems to suggest that a >>> native package is ok where there is no upstream. >> >> No. It's ok to do so when the software is intended only for >> Debian (like, the Debian installer for example... I'm not even >> sure that's the case, just an example), which isn't your case. >> > > Thanks for the clarification regarding native vs. quilt, rebuilt using > quilt. The updated package can be downloaded from: > > http://socketpolicyserver.com Hi Daniel,
Since I reviewed once your package, I feel like I have to do it again, and continue the mentoring process. So let me do again, this time more deeply. 1/ debian/changelog Since your package has never reached Debian yet, your changelog should be only: libapache2-mod-socket-policy-server (0.0.1) unstable; urgency=low * Initial release. (Closes: #642282) -- Debian Packaging Team <[email protected]> Tue, 20 Sep 2011 21:25:00 -0800 So please remove all other entries. We don't really care what upstream version you use, so it could be 0.0.4-1 if you feel like it, and if it's more convenient for you to not downgrade in your repository. Also, it seems that your /changelog is the same as what's in the debian/changelog, and you are packaging it twice!!! My advice here: write things in /changelog that are only for upstream code, and in debian/changelog only things that concerns the packaging itself and nothing else. 2/ debian/copyright It's basically not respecting the specifications. It should be something like: <header paragraph> <files paragraph> [license paragraph] Your copyright file doesn't have a header. Also, I think that something like this: License: Other For alternate licensing arrangments, please contact: . [email protected] -OR- [email protected] isn't appropriate and doesn't fit the spirit of DFSG (I may be wrong here, I have never been really good with licenses, but I just feel it isn't right). Especially, on the Apache 2.0 license, it says: "THIS SOFTWARE MAY NOT BE USED EXCEPT UNDER LICENSE", then just later, you are writing that "alternate licensing arrangments" could be made... Just leaving the Apache 2.0 license, which give a very big amount of freedom, seems enough to me. I fail to see how it could be more free than with the Apache 2.0 license! :) 3/ debian/patches/debian-changes Don't you think that a Debian specific patch could be avoided? Basically, there's only 2 things that are done in the debian-specific Makefile: build and install. These could easily be put in your debian/rules by overriding the dh install and build target (namely, if I'm not mistaking, dh_auto_install and dh_auto_build). That would be a lot more simple than having to manage a patch like this. Not that this is just a suggestion, what you did works also, I just think it's a bit over-engineered, and that overriding the dh rules targets would be more simple to maintain. 4/ debian/postinst While it's fine to automatically install a site in sites-available, and install and activate a .load Apache module, actually activating the default vhost that you created might not be appropriate, especially since what you are doing is activating a policy with <site-control permitted-cross-domain-policies="none" /> in it. Am I right that basically, it will do the same as if nothing was done at all, and make it completely restrictive? 5/ Your package doesn't build twice That's the worst issue of your package. Running dpkg-buildpackage twice in a raw fails with some "error: cannot represent change". This most of the time happens when the clean target is wrong. Please fix this. 6/ debian/watch file Please add a watch file. It would be quite easy to write, since you are using Apache directory listing. Something like this will do the trick: version=3 http://socketpolicyserver.com/downloads/libapache2-mod-socket-policy-server_(.*).orig.tar.gz 7/ debian/control Are you sure that this is accurate: Maintainer: Debian Packaging Team <[email protected]> And don't you think that just: Daniel Kauffman <[email protected]> wouldn't be better? I mean, is there really a team behind this Debian packaging? I'm not saying this is bad to write what you did, just that it's always better to have something that reflects reality (and this is just a suggestion, nothing really bad). I can read: Depends: ${misc:Depends}, ${shlibs:Depends}, apache2 | apache2-mpm, apache2.2-common But apache2 depends on apache2.2-common, and so are all the apache2.2-mpm-* packages that are providing apache2.2-mpm, so I guess that depending on apache2.2-common is useless here (if you don't think so, feel free to correct me). I think that's it for the moment. Let me know when you've processed all the above issues, and I'll review your package once more. Then I'd prefer if another DD that did some apache module packaging had a last look before I sponsor the package for you (because I may well have missed something). Cheers, Thomas Goirand (zigo) -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected] Archive: http://lists.debian.org/[email protected]
