Patch metadata consistency

Dominic Hargreaves dom at earth.li
Sat Oct 20 10:06:52 UTC 2012


On Sat, Oct 20, 2012 at 10:22:52AM +0300, Niko Tyni wrote:
> 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.

That's quite some attention to detail - thanks :)

> 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.

.patch is slightly nicer in my view too. I'm on the fence about whether
a huge diff is sensible for 5.16.

This is also not visible to the user, so I agree it's not wheezy material.

> 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.

Yes, probably. I hadn't twigged that concise URLs were important for
patchlevel.h. Not sure it's worth bothering about for wheezy or not.

> 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.

-- 
Dominic Hargreaves | http://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)




More information about the Perl-maintainers mailing list