Code review comment for ~waveform/ubuntu/+source/open-iscsi:merge-2.1.5-jammy

Revision history for this message
Dave Jones (waveform) wrote :

> I found no auxiliary tags for the merge like old logical and such, I'll try to
> review as-is without them.

Yes, I didn't directly use git-ubuntu as there's (still) no arm builds for it. So
I just did a rebase of the old split plus the subsequent versions on top of it.

> Why does debian not need "d/t/test-open-iscsi.py: adopt to resolvectl (systemd v249
> compat)" they are >=249 too. If the check reveals it can be dropped with 250 please
> mention that.

Because the entire autopkgtest suite is part of our delta and not upstream either.
I'm guessing there are good reasons for that, possibly hinted at by the note in
the git commit "this suite should be replaced by LIO that replaced TGT in main"? In
other words, presumably the test suite is intended to be replaced by something better
(LIO?) at some point (but what's TGT ... and which main? I'm just guessing at
stuff here).

> I've not seen it, did you forward "d/rules: remove duplicated dh_installsystemd
> section" as that seems to affect Debian as well?

I've *reported* it upstream (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1004014),
but not sent our patch as we only activate the socket, whilst Debian activates both
socket *and* service in d/rules, so our patch likely isn't what they want.

That said, I'm not entirely sure what the intent is in Debian for this, as they did
merge other socket-activation-only bits but kept d/rules largely unchanged? Anyway,
not being certain what the intent is, I warned about the duplication but will leave
it to their discretion how they wish to fix it.

> Did you run autopkgtest against this PPA yet?
> I've seen none, remember you can check and trigger results on the PPA via
> $ lp-test-ppa ppa:waveform/open-iscsi --release jammy --showpass
> (from git+ssh://<email address hidden>/~ubuntu-server/ubuntu-helpers)

Erm, not sure I can -- I don't think I have the authority to launch autopkgtests
(your otherwise fine script runs with no error exit code, but doesn't appear to
actually launch for me).
anything for me)

« Back to merge proposal