retitle 1114691 trailing comma in Build-Depends* causes dak to error globally
affects 1114691 ftp.debian.org
thanks
I dug into this, here's what I know now.
When we do a dak rm, dak will load the archive dependencies to detect
breakage. It does this by SELECT-ing source_metadata (basically, the
keys from a parsed deb822 dsc), source (the source version itself),
and newest_src_association (newest source for a sutie).
Long story short what is _basically_ this SQL is run:
```
SELECT s.source, string_agg(sm.value, ', ') as build_dep
FROM source s
JOIN source_metadata sm ON s.id = sm.src_id
WHERE s.id = 976181
AND sm.key_id IN (18, 8)
GROUP BY s.id, s.source
```
Where "s.id = 976181" is the hdf5 in sid, and the "(18, 8)" are the
metadata fields "Build-Depends" and "Build-Depends-Indep", respectively.
The intent of this code is to create a single Dependency line comprised
of both B-D and B-D-I, which it concats using string_agg with ", ".
You can maybe see where this is going by now :)
The output of that SQL is:
| hdf5 | debhelper (>= 10~), mpi-default-dev, libopenmpi-dev [!armel !armhf !any-i386 !hppa !m68k !powerpc !sh4 !x32], libmpich-dev, zlib1g-dev, libjpeg-dev, gfortran, sharutils, chrpath, libaec-dev, libcurl4-openssl-dev, libssl-dev, default-jdk-headless (>= 2:1.7) [!hppa !hurd-any] <!nojava>, javahelper [!hppa !hurd-any] <!nojava>, junit4 [!hppa !hurd-any] <!nojava>, libslf4j-java [!hppa !hurd-any] <!nojava>, libopenmpi-dev (>= 5.0.8-6.1) [ppc64el],, doxygen, php-cli
| (1 row)
I'm sure everyone's guessed by now, but just to spell it out, While
"foo," is accepted by the Python apt_pkg module, "foo,, bar" is not.
There are a few places where this could have been avoided:
1) humans working with d/control can take steps to ensure there are
not trailing commas.
2) dpkg could strip trailing commas when it generates the dsc, like it
does with dependencies for binaries in debs
3) dak could strip trailing commas on the way into the table
4) the sql query above could handle a trailing comma gracefully
I think #1 is perhaps unhelpful, since other tooling handles trailing
commas well. Similarly, I'm not convinced #3 is a good idea. Mangling
the contents of a dsc into the archive db seems like a real bad idea.
I think there's a good case to be made that #2 is a prudent idea,
although, nothing stops people from creating dscs like this by hand or
using a different tool.
#4 seems like the obvious next step, since we've already got this
package in the archive, and the string handling here is acutely to
blame.
Something like, perhaps:
replace(string_agg(sm.value, ', '), ',,', ',')
Since anything other than a trailing "," here would be invalid basically
everywhere, so seeing a ",,," is asotonishingly unlikely.
I'll send up a PR for review soon :tm:
paultag, without any hats on this time