[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFS: libapache2-mod-socket-policy-server (an Apache2 module for serving Adobe socket policy files)



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
<debian-packaging-team@socketpolicyserver.com>  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:
 .
 licensing@rocksolidinnovations.com -OR- sales@rocksolidsolutions.org

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
<debian-packaging-team@socketpolicyserver.com>

And don't you think that just:
Daniel Kauffman <debian-packaging-team@socketpolicyserver.com>

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)


Reply to: