[Debian-olpc-devel] python-xklavier -- Python bindings for libxklavier

[Jonas Smedegaard]

On Fri, Apr 16, 2010 at 12:47:22PM -0400, Luke Faraone wrote:
>On 04/16/2010 11:51 AM, Jonas Smedegaard wrote:
>> You should probably include python-autotools.mk (not autotools.mk). 
>> As is declaring DEB_PYTHON_SYSTEM has no affect, and quite likely the 
>> binary package do not follow Python Policy properly.
>
>Done. This is not included in the version of CDBS in Lucid, so we'll 
>have to backport it if we want to ship this for PPA users :(

There are two options:

  a) Backport CDBS as well
  b) include all(!) cdbs snippets with package

The first is IMO the most elegant (if backporting done elegantly ;-) ).

If Ubuntu policy or whatever force you to the second, I would appreciate 
if doing so as a separate branch to not clutter the packaging for Debian 
use.  And beware that it is not enough to include python-autotools.mk 
locally, as it requires recent autotools*, makefile* and core* snippets!


>> xklavier/Makefile.in needs to have ./ prepended in line 25 of 
>> debian/copyright (so that it works with "find -path").  Or more 
>> elegantly it can simply be removed, as "Makefile.in" covers all files 
>> by that name recursively (which conversely means you should be 
>> cautious to not accidentally covver too many files - prepend ./ if in 
>> uncertain).
>
>Corrected. In this case, all "Makefile.in"s are under that license, 
>afaict.

Yes. For Makefile.in files that's what I would do too.

But (warning: nitpicking ahead!) I would reorder the copyright file 
differently to help avoid accidentally shadowing specific entries by 
"jokers" like that:

  1. catch-all joker, i.e. that optional file section with implicit "*"
  2. any-file jokers, e.g. ./src/doodle/*
  3. single-file jokers, e.g. Makefile.in
  4. single files and same-dir jokers together, e.g. ./echo.[ch], ./zx*

In other words, I would split out Makefile.in as a separate file section 
right below the catch-all section.

I would also separate licenses as separate sections at the bottom, even 
if only used in a single file section, as it IMO improves readability.

...only after above grouping would I sort alphabetically, to ease 
diff'ing (4th group) against the hints file.


>> Also, I recommend to make use of CDBS build-dependency 
>> auto-resolving:
>>
>>  1. copy debian/control to debian/control.in
>>  2. edit debian/control.in to build-depend on @cdbs@ (not cdbs)
>>  3. Auto-resolve:
>>     DEB_MAINTAINER_MODE=1 fakeroot debian/rules clean
>>  4. Check resulting debian/control, remove excess explicit
>> build-dependencies (e.g. debhelper), and repeat step 3.
>
>Done, and committed.

I noticed you commited a bunch of changes together.  Most ideal for 
others to review is if you commit each change as a separate commit.  But 
I get lazy too and commit multiple changes together.  One thing I do 
recommend, though, is to not commit auto-generated stuff together with 
hand-edited changes.  That makes it easier to later revert a change or 
cherrry-pick across branches.

The files debian/control (when using debian/control.in), 
debian/changelog (when using "git dch") and debian/copyright_hints are 
sometimes auto-generated and should IMO always be commited separately 
from other commits.


>> I also recommend to enable copyright-check to track new or changed
>> licensing or copyrights:
>>
>>  1. include CDBS rule utils.mk
>>  2. touch debian/copyright_hints
>>  3. Generate and preserve new hints:
>>     DEB_MAINTAINER_MODE=1 debian/rules pre-build
>>     mv debian/copyright_newhints debian/copyright_hints
>>  4. check that all files, copyrights and licenses in hints file match
>>     those manually declared in copyright file (you may want to order the
>>     contents of the copyright file similar to the hints file to ease
>> future updates with simple diff).
>
>Done. It noted some things which I missed, but also a bunch of things 
>with no copyright which should have been covered by the implicit "*" in 
>the first copyright declaration.

You should only use the copyright hints as, well, hints: INSTALL 
contains licensing info so is catched by copyright-check, but really 
isn't itself copyright-protected and should not be documented as such in 
debian/copyright.

Also, I recommend to use the multi-line format, to ease diff'ing later.

And ltmain.sh (and a few others, I believe) really is dual-licensed - 
have a look at e.g. the sugar-base package for inspiration.

Ant there is no short-form license in DEP5 rev. 135 named "X11".  I 
would use "other-X11" but am uncertain if that is actually illegal too 
and only "other" is allowed (which ruins my style of putting all 
licenses at the bottom).


>Anything else that needs fixing?

The binary package has an empty Conflicts: line.

Apart from all this nitpicking: No, I really like your packaging!

:-)


How is it - are you a DM or a DD now? Can you release the package on 
your own officially for Debian or would you want me to do it?


  - Jonas

-- 
* Jonas Smedegaard - idealist & Internet-arkitekt
* Tlf.: +45 40843136  Website: http://dr.jones.dk/

  [x] quote me freely  [ ] ask before reusing  [ ] keep private
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/debian-olpc-devel/attachments/20100416/0c29b7c7/attachment.pgp>