Patch metadata consistency

[Niko Tyni]

Hi,

looking at the wheezy changes, I noticed a few inconsistencies
in the patch metadata that I'd like to avoid in the future.
My main aim is uncluttering patchlevel.h (and therefore "perl -V"
output) as much as possible.

To that end, I wrote a simple maintainer test script (first version
attached) with the results below. I'll probably be fixing these for 5.16
at some point; this nitpicking doesn't feel like wheezy material to me.

I'd appreciate any opinions, particularly if anything in the discussion
below seems controversial.

#   Failed test 'patch name ends in .diff (fixes/sys-syslog-socket-timeout-kfreebsd.patch)'
#   at debian/t/patch-names.t line 13.

#   Failed test 'patch name ends in .diff (fixes/propagate_tainted_errors.patch)'
#   at debian/t/patch-names.t line 13.

#   Failed test 'patch name ends in .diff (debian/perl5db-x-terminal-emulator.patch)'
#   at debian/t/patch-names.t line 13.

#   Failed test 'patch name ends in .diff (fixes/tainted-smartmatch)'
#   at debian/t/patch-names.t line 13.

#   Failed test 'Debian BTS URL in the canonical form (fixes/propagate_tainted_errors.patch)'
#   at debian/t/patch-names.t line 33.

#   Failed test 'Debian BTS URL in the canonical form (fixes/socket_cache_propagate)'
#   at debian/t/patch-names.t line 33.
# Looks like you failed 6 tests of 268.

For the first class, having consistent patch names would clearly be good.
I'm not overly fond of the .diff extension, ISTR it originally comes
from TopGit. Migrating away from it would make a huge debdiff, so that's
definitely not wheezy stuff. I think at least the .patch extensions
should be fixed (but again, probably not for wheezy), and the easiest
thing to do seems to just stick with .diff for everything.

For the second class, the point is that
 http://bugs.debian.org/659075
is much shorter than
 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=659075
and therefore better for patchlevel.h. We can also do the transformation
in debian/gen-patchlevel, but I think it's generally tidier to use the
short form in the patch metadata in the first place.

I've also added TODO entries for patches missing a debbug URL. I'd love
to make those fail, but there's currently 17 of them on the 5.14 branch
and 15 on the 5.16 one, so there's a bit of documentation work to do
before that. Ideally, I think every patch should have a debbug that
at least explains what it does and why it is or is not suitable for
upstream inclusion.

There are also some inconsistencies with Origin entries. We should
probably be using the upstream|backport|vendor|other prefix more, and
I now realize I've used 'upstream' in a lot of places where 'backport'
would have been more correct.

Also, debian/gen-patchlevel should handle multiple Origin entries so
that CPAN modules and the like could have separate pointers to both
the original upstream change and the perl.git import commit. (See for
example debian/patches/fixes/cgi_no_shellwords_pl.diff)

Finally, debian/gen-patchlevel should probably order the patch metadata
more consistently, but I expect that's a little hard to do with the
current sed code. Enforcing the order of the metadata lines in the actual
patch doesn't seem very productive, though. The current variation like
    DEBPKG:fixes/sysconf.t-posix - [8040185] [perl #102888] http://bugs.debian.org/646016 Fix hang in ext/POSIX/t/sysconf.t on GNU/Hurd
    DEBPKG:fixes/ipc_open3 - [perl #114454] http://bugs.debian.org/683894 IPC::Open3::open3(..., '-') broken
    DEBPKG:fixes/string_repeat_overrun - http://bugs.debian.org/689314 [b675304] avoid calling memset with a negative count
certainly isn't optimal.

-- 
Niko Tyni   ntyni at debian.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-names.t
Type: application/x-troff
Size: 1043 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/perl-maintainers/attachments/20121020/404ff0fd/attachment.t>