Code review comment for ~enr0n/ubuntu/+source/systemd:ubuntu-kinetic-merge-251.2-2

Revision history for this message
Nick Rosbrook (enr0n) wrote :

Thanks for the review, Lukas!

> #0 PPA autopkgtests are looking good so far, have there been any test against
> all the affected packages (i.e. Bileto), to notify them (or at least the
> 'main' packages) about any fallout?
> https://autopkgtest.ubuntu.com/results/autopkgtest-kinetic-
> enr0n-systemd-251/?format=plain

No, unfortunately I never got around to using Bileto.

> #1 d/patches/hwdb-implement-root-option-for-systemd-hwdb-query.patch,
> debian/patches/sd-hwdb-add-sd_hwdb_new_from_path.patch, please mention the
> upstream commit/fix in patch headers, e.g. using the "Forwarded:" DEP-3
> header. It's not clear at first glance that this is a cherry-pick, missing any
> reference.

Whoops, thanks for catching this.

> #2 d/patches/test-deny-TEST-29-PORTABLE-again.patch: is this needed for all
> architectures? We used to disable it on ppc64el only in the past, IIRC.

I will take another look at this. I was seeing it locally on amd64.

> #3 d/control: Package: systemd-oomd => Is "Section: admin" needed? It is
> dropped in Debian and seems like unnecessary Ubuntu delta.

I agree, this delta can be dropped.

> #4 d/patches/test-install-libgcc_s.so.1-for-TEST-70-TPM2.patch: please add
> some DEP-3 headers for this new patch. At least a bug reference and/or
> upstream fix. So we can easily decide when is the correct time to drop that
> again in the future.

Ack.

> #5 d/libsystemd0.symbols: Fix lintian error about version string, using "~" as
> suggested.
> E: libsystemd0: symbols-file-contains-current-version-with-debian-revision on
> symbol sd_hwdb_new_from_path@LIBSYSTEMD_251 (libsystemd.so.0) [symbols]
> N:
> N: Debian revisions should be stripped from versions in symbols files. Not
> N: doing so leads to dependencies unsatisfiable by backports (1.0-1~bpo <<
> N: 1.0-1 while 1.0-1~bpo >= 1.0). If the Debian revision can't be stripped
> N: because the symbol really appeared between two specific Debian revisions,
> N: you should postfix the version with a single "~" (example: 1.0-3~ if the
> N: symbol appeared in 1.0-3).
> N:
> N: This problem normally means that the symbols were added automatically by
> N: dpkg-gensymbols. dpkg-gensymbols uses the full version number for the
> N: dependency associated to any new symbol that it detects. The maintainer
> N: must update the debian/<package>.symbols file by adding the new symbols
> N: with the corresponding upstream version.

Ack.

> #6 d/control: dh-sequence-package-notes >= 0.8 (Doesn't work with the Jammy
> version) – Not sure if it's worth introducing Ubuntu delta for this, though.
> As the people building systemd isn't too big and people are usually in the
> known.

Okay, I will leave it as-is for now then.

> #7 Debian's 251.2-6 seems to have some interesting changes (features & fixes),
> but we can do a smaller merge later in the cycle to get those included.

Yeah, I have kept an eye on Debian's revisions, and I figured we would do a smaller merge later (based on the history in debian/changelog).

« Back to merge proposal