XPost: linux.debian.bugs.dist
From: daniel@debian.org

Hi,

But we also have many cases where there's a trailing comma:

I think this came from a bug in 'wrap-and-sort -bast',
but except for one of my packages (fixed in git) this is no longer the
case at least in testing/unstable (didn't check stable or older).

Regards,
Daniel

--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)

XPost: linux.debian.bugs.dist
From: hertzog@debian.org

Package: dpkg-dev
Version: 1.22.11
Severity: normal
X-Debbugs-Cc: hertzog@debian.org

Hi Guillem,

While maintaining tracker.debian.org, I started to get failures about
invalid "Maintainer" fields.

There's a clear violation with a Maintainer field with two maintainers: https://bugs.debian.org/1076048
Maintainer: Steve Langasek <vorlon@debian.org>, Michael Vogt <michael.vogt@ubuntu.com>

But we also have many cases where there's a trailing comma:

Maintainer: Debian Security Team <team@security.debian.org>,
Maintainer: Daniel Baumann <daniel.baumann@progress-linux.org>,

And yet nothing complained about this (neither dpkg, nor lintian, nor
dak). dpkg-source and dpkg-gencontrol happily copied the invalid data.

After discussion on #debian-qa, we believe that the toolchain should strip
the trailing comma to bring the field back into compliance. Much like it
will clean up commas in dependencies.

But when that is not sufficient, it probably makes sense to fail and
report the problem? There's a single case that would be broken right now.
But a dozen of packages with trailing commas.

Thank you for your work on dpkg!

-- Package-specific info:

-- System Information:
Debian Release: trixie/sid
APT prefers stable-security
APT policy: (500, 'stable-security'), (500, 'unstable'), (500, 'testing'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 6.11.2-amd64 (SMP w/16 CPU threads; PREEMPT)
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages dpkg-dev depends on:
ii binutils 2.43.50.20241126-3
ii bzip2 1.0.8-6
ii libdpkg-perl 1.22.11
ii make 4.3-4.1
ii patch 2.7.6-7
ii perl 5.40.0-8
ii tar 1.35+dfsg-3
ii xz-utils 5.6.3-1+b1

Versions of packages dpkg-dev recommends:
ii build-essential 12.12
ii clang-10 [c-compiler] 1:10.0.1-8+b1
ii clang-11 [c-compiler] 1:11.1.0-6+b2
ii clang-13 [c-compiler] 1:13.0.1-13
ii clang-14 [c-compiler] 1:14.0.6-20
ii clang-15 [c-compiler] 1:15.0.7-15+b1
ii clang-16 [c-compiler] 1:16.0.6-27+b1
ii clang-17 [c-compiler] 1:17.0.6-18
ii clang-18 [c-compiler] 1:18.1.8-12
ii clang-9 [c-compiler] 1:9.0.1-20+b1
ii fakeroot 1.36-1
ii gcc [c-compiler] 4:14.2.0-1
ii gcc-10 [c-compiler] 10.5.0-4
ii gcc-11 [c-compiler] 11.5.0-1
ii gcc-12 [c-compiler] 12.4.0-2
ii gcc-13 [c-compiler] 13.3.0-8
ii gcc-14 [c-compiler] 14.2.0-8
ii gnupg 2.2.45-2
ii gpgv 2.2.45-2
ii libalgorithm-merge-perl 0.08-5

Versions of packages dpkg-dev suggests:
ii debian-keyring 2024.09.22

-- no debconf information

--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)

XPost: linux.debian.bugs.dist
From: guillem@debian.org

Hi!

On Tue, 2024-12-10 at 15:13:03 +0100, Raphaël Hertzog wrote:

Package: dpkg-dev
Version: 1.22.11
Severity: normal
X-Debbugs-Cc: hertzog@debian.org

While maintaining tracker.debian.org, I started to get failures about
invalid "Maintainer" fields.

There's a clear violation with a Maintainer field with two maintainers: https://bugs.debian.org/1076048
Maintainer: Steve Langasek <vorlon@debian.org>, Michael Vogt <michael.vogt@ubuntu.com>

But we also have many cases where there's a trailing comma:

Maintainer: Debian Security Team <team@security.debian.org>,
Maintainer: Daniel Baumann <daniel.baumann@progress-linux.org>,

And yet nothing complained about this (neither dpkg, nor lintian, nor
dak). dpkg-source and dpkg-gencontrol happily copied the invalid data.

Right, these are one of several fields where dpkg tools do not really
parse or normalize the values, but I've been considering that a
misfeature, as then we get this kind of output, where all consumers
then need to handle such unexpected/wrong values.

After discussion on #debian-qa, we believe that the toolchain should strip the trailing comma to bring the field back into compliance. Much like it
will clean up commas in dependencies.

I think that would be fine, but for Maintainers only during a
transitional period and not as a long-term supported feature, because
this is not like a dependency field where you have multiple values,
and a trailing comma makes sense when placing them each on their own
line, or to handle empty substvars, or similar. For Uploaders it makes
sense to always strip them given that it is a comma-separated list
(and where «wrap-and-sort -sat» seems the best format).

For commas in the middle, I think that will currently need some more consideration (see below).

But when that is not sufficient, it probably makes sense to fail and
report the problem? There's a single case that would be broken right now.
But a dozen of packages with trailing commas.

I was checking the state of the archive, slightly after this got
filed, and the problem seems worse to me:

,---
$ grep-deb-sources -e -sPackage,Maintainer,Extra-Source-Only \
-FMaintainer '.*,' | grep ^Package: | wc -l
49
`---

,---
$ grep-deb-sources -e -sPackage,Maintainer,Uploaders,Extra-Source-Only \
-FUploaders '.*,$' | grep ^Package: | wc -l
6574
`---

In addition to the already reported golang-github-mvo5-goconfigparser,
another one of those has a comma in the middle in the Maintainer field,
but is an Extra-Source-Only:yes source package:

Package: darts
Maintainer: Natural Language Processing, Japanese <pkg-nlp-ja-devel@lists.alioth.debian.org>
Extra-Source-Only: yes

Where the version in unstable looks fine.

I then started writing a parser for these fields, went checking for
what would be the allowed documented syntax, and ended up in the same
rabbit hole as the following Debian Policy bug reports #401452, #509935
and #962277.

I do think we should clarify the syntax first, and IMO we should go
for the simplest possible syntax probably based on the RFCs avoiding
all obsolete constructs, but allowing for the currently used names
which include a comma, so to me that means supporting quoted names,
which we already have in debian/changelog trailers, and would need
to keep supporting to be able to parse old entries anyway, and
which we need to match against the Maintainer fields.

I'll try to write a parser for the above, and throw it against the
archive Sources indices, and see what can be warned on, and what
errored out directly, then probably update the Debian Policy bug
reports.

Thanks,
Guillem

--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)