Patch management: tracking forwarded status

[Niko Tyni]

On Sat, Sep 20, 2014 at 01:15:48PM +0100, Dominic Hargreaves wrote:

> We have for a long time operated a policy (unwritten except possibly
> in <https://wiki.debian.org/PerlMaintenance>) that patches which aren't
> genuinely specific to Debian shouldn't be applied without being forwarded
> upstream. This is to ensure we don't end up with an increasing burden
> of local patch maintenance, and also so upstream finds it easier to
> support our packages (knowing the deviation from upstream is minimised).

Ack.

> To this end we have fixes/ and debian/ patch directories. My understanding
> of this has been that anything potentially portable upstream should live
> in fixes/ and only things genuinely specific to debian/ should go in
> debian/. Recently we had a few patches we wanted to get in promptly and
> to mark the fact we hadn't forwarded upstream we put them in debian/.

That was probably more an oversight than an intentional decision,
at least on my part. Apologies for the inconvenience I've caused.

I guess my thinking was along the lines of 'already forwarded patches
can go in fixes/, others go in debian/', but I can see that's not really
the best practice.

> Was this the correct thing to do? My general workflow for patch review
> is to assume I don't need to worry about tracking the status of upstream
> acceptance with anything in debian/, and that's no longer true.

I think it wasn't the correct thing, and I'm happy to adopt a more
rigorous policy on this.

> I would quite like to see some tooling to help us here. A maintainer
> test to report on patches which haven't yet been forwarded upstream
> on patches in fixes/ would be nice. We would need a way to positively
> assert that we're deferring the forwarding, for those cases where it's
> not appropriate to forward upstream before a release. Perhaps a new tag
> Forwarded-Reason? That would be either a textual note, or (preferably)
> a URL pointing to a divergence tracking bug report such as #762269[1].

My first feeling was that we could just enforce that a Forwarded field
exists, and the "shame" of having to say "Forwarded: no" would be enough
of a stick to improve the situation. But after some more thinking I
agree that requiring a justification is a good idea.

The DEP-3 wording is indeed a bit unfortunate. Using
 Forwarded: no, needs reworking for upstream inclusion
 Forwarded: no, http://bugs.debian.org/762269
would feel natural, particularly as something very similar is allowed
in the Origin field. I wonder if we should just do this regardless?

'Forwarded-Reason' doesn't quite cut it for me, partly because it seems
it should really be called 'Not-Forwarded-Reason' :) But I've got no
better suggestions, so I guess feel free to use that.

> The logic[2] for patches in fixes/ should be:
> 
> if Forwarded == 'not-needed'
>  fail # should be in debian/ instead
> else if Forwarded = 'no' and Forwarded-Reason is null
>  fail # should justify why we haven't forwarded it yet
> else
>  pass
> 
> The logic for patches in debian/ should be:
> 
> if Forwarded != 'not-needed'
>  fail # should be in fixes/ instead
> else
>  pass

The logic looks good to me, and I agree that a maintainer test doing
this would improve things. Possibly mark the current failures TODO if
fixing them straight away is non-trivial.

I note that much of the stuff in debian/ should arguably be forwarded
upstream too in an ideal world, just with a more general implementation
that makes the Debian-specific thing configurable somehow. (Of course,
in the real world coming up with the general implementation may well
be not worth the effort required.) The only patches really specific to
Debian are cases where upstream would reject not only our patch, but also
the notion that the software should ever work that way.  So perhaps we
should require some justification also for 'not-needed'?

Thanks for bringing this up! It's something of a pain point for me as
I'm generally a bit too shy of taking imperfect stuff (like bugs without
a patch or non-portable patches) upstream, but I very much appreciate
your effort to improve things.
-- 
Niko Tyni   ntyni at debian.org