Bug#1028503: UDD: Unknown "yes" value for Forwarded field in patch metadata

[Guillem Jover <guillem@debian.org>]

Hi!

On Sun, 2023-01-15 at 21:12:53 +0100, Lucas Nussbaum wrote:
> > I'm wondering why diverge from the patch metadata guidelines? If there's
> > a desire to change the field semantics, perhaps it would be better to
> > change the guidelines instead? :)
> >·
> > But given this interchange, perhaps we should try to make it more
> > clear or explicit about some of these aspects there!
>·
> Originally I admit that I misread the DEP3 rules.
> Now the implementation is much closer to DEP3. Unfortunately, DEP3 was
> probably not written with automatic processing in mind.

> The current status of the data is the following[1]:

Thanks for collecting and analyzing these!

> With some comments:
>  no              | No Forwarded, No Bug field                                                   | 100145 |   68.12
> => that sticks to the DEP3 specification. (no forwarded: + no bugs: => implicit no)
>·
>  not-needed      |                                                                              |  19329 |   13.15
> => that's an explicit "Forwarded: not-needed"
>·
>  yes             |                                                                              |  12217 |    8.31
> => Those are the cases where it's clear that the bug has been forwarded
>·
>  no              | Explicit Forwarded=no                                                        |  11448 |    7.79
> => that's an explicit "Forwarded: no"

All ack.

> The remaining lines are the ones that could require discussion.
> First, it's worth noting that they only account for 2.63% of the
> patches.
>·
>  invalid         | Invalid value for Forwarded field                                            |   2270 |    1.54
> => I looked at the values considered invalid[2], and could not really convince
> myself that they should automatically fit in one of the above
> categories.

I see what you mean. Although from skimming over these values, the two
things that become clear are things that I've already wondered about,
forwarding to email addresses (there are "many" instances of those
there), and how to represent "no" due to no-upstream (which TBH it
seems we are lacking a more general way to mark packages as such,
because there is also the debian/watch and several lintian tags that
trigger but should probably not be considered issues when there is no
upstream).

I guess several of these bogus ones could perhaps be fixed up by the
janitor.

>  invalid         | Forwarded=yes, but no Bug field                                              |   1119 |    0.76
> => I think that we should strongly encourage leaving a public trace when
> forwarding bugs

While I agree that would be ideal, that has the problem I mentioned
when upstream does not have any such public avenue. Since then I
adopted the suggestion by Paul Wise to use a mail address, but from
the previous entry, I think that makes parsing probably tricky. :/

The other reason I've set Forwarded=yes in the past is when I've
submitted the patches and they have been merged very quickly, and
having to go look for the web archive URL references or similar seems
more work than it's worth, when just adding the commit id in the Origin
field is quicker (but see below.)

>  invalid         | No forwarded, Debian bug in Bug: field. Bug-Debian: should be used instead.  |    283 |    0.19
> => quite obviously invalid

Ack.

>  no              | No Forwarded, Upstream bug, but not a URL                                    |    190 |    0.13
> => those are mostly patches misusing the Bug: field for the Debian
> bug.[3]. Since the specification requires a URL here, it could be
> considered invalid as well.

Is this perhaps being too strict? Some of the URLs seem to be valid,
just prefixed with whitespace? The rest seem indeed invalid.

>  invalid         | Forwarded=yes, Debian bug in Bug: field. Bug-Debian: should be used instead. |      8 |    0.01
> => quite obviously invalid

Ack.

> So, all in all, my implementation[4] sounds sane.

I think the guidelines might need some work to add clearly missing
states or declarative information; and then having this kind of
more strict validator (or ideally automatic fixer) will eventually
make the data more correct. Lintian seems rather lenient in its
parsing, but following the guidelines more closely perhaps.

> > I otherwise do not know how we can mark patches as forwarded when
> > for example you send them directly to upstream via email or to a mailing
> > list that has no public archive or similar.
>·
> That's true. Other than blinding accepting "Forwarded: yes", I don't
> know how this could be addressed.

I still see value in a "faith" based "yes", but I can see how this
can be seen as rather unsatisfactory from a global point of view. But
perhaps this is tied with the following point, and "yes" should only
be accepted when there's "proof" from other fields that it got
forwarded (say because it's merged already).

> > I think the Origin field needs to be taken into account too, in
> > particular when it has an "upstream" or a "backport" value. As well as
> > the Applied-Upstream field.
>·
> I would prefer the Forwarded field to be a simple field only allowing
> 'no', 'not-needed', and maybe 'yes' (with the pointer to where the patch
> was forwarded in a separate field).
>·
> Or if the Forwarded field's value is supposed to be computed, I think
> that we should provide a more detailed specification of the algorithm.

Sorry, I think the above paragraph was not very clear. What I meant is
that when analyzing when a patch needs work (for example in the DMD
dashboard), then these other fields need to be taken into account. But
I'll file that one as an independent report. I agree that if the value
needs to be computed from other sources then this should be specified
more clearly.

(I guess I'll try to collect other issues with the guidelines and
start a thread later on.)

Thanks,
Guillem