Review new sbws release (1.9.0)

For tpo/network-health/sbws#40183 (closed)

Sorry @lavamind, we need to do this again :)

The branch at https://salsa.debian.org/juxor-guest/sbws/-/commits/debian/unstable_wip is not ready to upload yet (the new sbws 1.9.0 isn't even released yet), but because the it should fix a bunch of Debian issues too, we can start with that. If this is fine for you, the commits starting by wip have questions to address.

changed title from Review new sbws release (1.91.) to Review new sbws release (1.9.1)

We can't release 1.9.1 until 1.9.0 is pushed, I think maybe you want to apply the fixes in debian/ to 1.8.1 -2, correct?

changed title from Review new sbws release (1.9.1) to Review new sbws release (1.9.0)

/cc @gk

added Backlog label

assigned to @lavamind

added Next label and removed Backlog label

added Doing label and removed Next label

Check that this is the correct way to do not stop services on upgrades

I think this will do it. But then, the bandwidth scanner will never be restarted after being upgraded using eg. apt upgrade, and it will have to be manually restarted. I assume this is what you want?

Check that this is a correct way to add ways in which the operators could modify the unit service to receive email on failures

• in debian/examples/status_mail_user@.service I would use /usr/local/bin/status_mail.sh instead of /usr/bin/status_mail.sh, since status_mail.sh is not actually installed on the system
• in debian/examples/sbws_generate.d/mail_on_failure.conf you need to specific a [Section] in the override, in this case it should be [Unit]

Check that this is the correct way to add apparmor profile and that the profile is fine (i think too permissive so far)

I'm really not familiar with AppArmor profiles, so I'm not sure I can give good advice on this topic. That said, the current update doesn't seem to be shipping this addition.

This is as far as I've gotten, because I wanted to build this package but the upstream branch is missing the commit for version 1.9.0. If you could push it I can continue the review :)

added Needs Information label and removed Doing label

I think this will do it. But then, the bandwidth scanner will never be restarted after being upgraded using eg. apt upgrade, and it will have to be manually restarted. I assume this is what you want?

I thought so, but after confirming with bwauth, it should be restarted on upgrade and it should be installed and enabled on install.

I've moved debian/unstable branch to debian/unstable_old1 with the fixes for this.

debian/unstable branch has those fixes squashed.

I'm really not familiar with AppArmor profiles, so I'm not sure I can give good advice on this topic. That said, the current update doesn't seem to be shipping this addition

Yes, sorry i didn't mention i removed this commit for now, because i'm not familiar either.

This is as far as I've gotten, because I wanted to build this package but the upstream branch is missing the commit for version 1.9.0. If you could push it I can continue the review :)

Thank you for this review! I've pushed it now, also pristine-tar. What i haven't pushed yet is the signed new debian tag.

Let me know if anything else is missing.

added Next label and removed Needs Information label

added Doing label and removed Next label

Alright, I think everything checks out.

Let me know when you want me to upload!

Whenever you can. We'll instead wait for the bookworm backports until there's a new tor minor release.

I can push new signed tag and updated branches to canonical repo, later, i guess

added Needs Review label and removed Doing label

added Doing label and removed Needs Review label

sbws_1.9.0-1_source.changes ACCEPTED into unstable

Thanks! Pushed to https://salsa.debian.org/pkg-privacy-team/sbws the branches debian/unstable, pristine-tar and upstream, the tags debian/1.9.0-1 and upstream/1.9.0 and set debian/unstable as default branch.

closed

mentioned in issue tpo/network-health/sbws#40183 (closed)