[Debian-olpc-devel] 'rainbow' uploaded to mentors.debian.net

[Luke Faraone]

On Sun, Aug 30, 2009 at 09:08, Jonas Smedegaard <dr at jones.dk> wrote:

> Why do you patch to install into /usr/sbin rather than /usr/bin?  Aren't
> those binaries supposed to be executed by ordinary users?
>

rainbow-run and rainbow-easy can only be run as root, thus they are (IMHO)
*s*ystem *bin*aries. rainbow-xify, mkenvdir, and rainbow-sugarize probably
belong in /usr/bin though...  I'll fix that shortly.


I notice that you include ${cdbs:Recommends} and similar in debian/control,
> but still include explicit dependencies as well.  The intend of those
> variables is to move all package dependency handling to debian/rules.  See
> debian/rules of e.g. sugar for an example.
>

Okay, I removed the ${cdbs:Recommends}.


> Why provide a virtul package olpc-security? Do you plan to package other
> alternative ABI-compatible security mechanisms?
>
> Similar with nss-rainbow - why provide such virtual package?
>

Both removed.


> Short descriptions should avoid leading "a" or "the", I believe.
>
> Long descriptions says "on the OLPC" which is incorrect, I believe: OLPC is
> an organization - I suspect that you mean "on the XO laptops by OLPC" or
> something similar.
>

Fixed.


> Where is the "upstream" branch holding the virgin upstrema code?  Did you
> not use git-import-orig to bootstrap the packaging?  "pristine-tar" branch
> is missing too.  Please use both --pristine-tar and --sign-tags options to
> inject upstream tarballs.


No idea. I used "git-import-dsc" per the
http://honk.sigxcpu.org/projects/git-buildpackage/manual-html/ , which
seemed an authoritative guide.

Here's how I usually start a new packaging:
>
> wget http/example.org/pkgname-1.2.tgz
> mv pkgname-1.2.tgz pkgname_1.2.orig.tar.gz
> mkdir pkgname
> cd pkgname
> git init
> git-import-orig --sign-tags --pristine-tar ../pkgname_1.2.orig.tar.gz
>
> You can either start from scratch again (and please do apply each change as
> a separate commit to help tracking what is what), or I can try merge a
> virgin proper bootstrap with your branch.  Tell me how you want us to deal
> with it.
>

Can you do it? I tried and can't seem to do so "properly".


> Long decription of the NSS module includes its licensing - I find that
> irrelevant there.
>

Fixed.

Why do you include the version number of the NSS module?  I suspect only
> shared libraries should include a trailing major version, and that NSS
> modules are not normal shared libraries.  Again, compare with other NSS
> modules for how they do things (without duplicating blindly - e.g. we use
> CDBS while some of them might use incompatible debhelper 7 "dh").
>

At least two NSS modules include trailing version numbers. Lintian was
tossing a warning unless I did so. If you'd rather, I can drop it and
silence Lintian.

Why do you install /var - that looks odd to me.
>

That directory is required by Rainbow, as that is where isolated instances
will be located. Rainbow will not work without this directory. Should I
create it in the postinst?


> Are you sure you should install /usr/lib/python*? - I suspect that is taken
> care of by the python-distutils.mk snippet, and installing directly
> actually violates Python policy!
>

If you look at the build .debs, there shouldn't be anything in
/usr/lib/python, but that snippit was required for me to get the right code
in the right place.


> Why do you set DEB_MAKE_MAKEFILE?


Hm? I don't see that anywhere in my debian/rules file.


> Why set DEB_PYTHON_MODULE_PACKAGES - I believe that should be autodetected
> if using proper package names.  Even if not, I still suspect that you should
> rename the package to include a leading "python-" to not violate Python
> Policy.
>

How about this: I'll split it off into three packages: rainbow,
python-rainbow, and libnss-rainbow2.

python-* packages shouldn't install anything into /usr/sbin/ or /usr/bin as
far as I can tell, so just renaming rainbow to python-rainbow would not be
acceptable.

If that's acceptable to you, I'll make the changes. As it is, I think we're
fine with just rainbow and libnss-rainbow.


> I dislike you claiming authorship of the manpages, when in fact
> autogenerated using help2man.  I recommend stripping that sentence from
> debian/help2man.
>

I suggest renaming debian/help2man to either debian/help2man.include or
> debian/help2man.addon. to clarify its purpose.
>

Both done.


> I suggest renaming *.md to *.mdwn (the extension used by e.g. Ikiwiki for
> Markdown files - I am not entirely sure that there is consensus about that
> extension, though).
>

There isn't much consensus, and pandoc uses .md in their examples.

 Did you write those Markdown files from scratch yourself, or did you
> bootstrap from data pulled from e.g. some web pages?  If so, it might make
> sense to setup a target in debian/rules to regenerate using newer upstream
> web sources.  Such target should not be used in normal builds (that's
> illegal!) but can be used by us to check once in a while if there are
> updated documentation.
>

I wrote them myself. I'm talking with Michael Stone to get those integrated
upstream.


> I suggest dropping the patch to clean egg file, and instead do that in
> clean:: of debian/rules: We want to limit patching to the least possible.
>

Done.


> Why not use the patch naming policy that I suggested in previous mail?
>

Also implemented.

Thanks,
Luke Faraone
http://luke.faraone.cc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/debian-olpc-devel/attachments/20090831/4d399e4f/attachment.htm>