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 gracefullyI 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 -- ⢀⣴⠾⠻⢶⣦⠀ Paul Tagliamonte <paultag> ⣾⠁⢠⠒⠀⣿⡁ https://people.debian.org/~paultag | https://pault.ag/ ⢿⡄⠘⠷⠚⠋ Debian, the universal operating system. ⠈⠳⣄⠀⠀ 4096R / FEF2 EB20 16E6 A856 B98C E820 2DCD 6B5D E858 ADF3
signature.asc
Description: PGP signature
