Bug#1010494: Re: Bug#1010494: RFS: usbrelay/1.0-1 -- USB HID relay driver
Hi Jan and Tobias,
@Jan good to hear from you.
@Tobias, thanks for useful suggestions, see my reactions below.
On Tue, May 03 2022, Jan Dittberner wrote:
> As you might have guessed I'm busier than I would like and do usually need a
> bit of time to respond.
>
> It would have been a good idea to contact me directly or via a wishlist bug
> requesting a package update. I would have answered a wishlist bug
> requesting an usbrelay upstream version update. The RFS came a bit
> surprisingly. A short mail before starting the work would have been nice. I
> had contact with Darryl Bond in the past and responded to his
> requests.
I apologize for not contacting you. I'm also in contact with Darryl Bond
and I understood that he tried to contact you recently, but without
success. It might have been my misunderstanding.
> I would be happy to have a co-maintainer for usbrelay. @Michal you may add
> yourself to Uploaders if you are interested in taking care of the package in
> the future.
Yes, I'll add myself. I'm using this software quite extensively and will
be happy to help with packaging.
@Jan How shall I proceed now? I've implemented the changes suggested by
Tobias, but I need to test the result again with the hardware, for which
I'll need a day or two. What then? Shall I upload a new version to
mentors.d.o or send salsa merge request or both?
> Please run lintian with the flags -i -v -E --pedantic to get the maximum
> output. I recommend using pbuilder or sbuild to build in a clean
> environment. I usually use sbuild in combination with git-buildpackage to do
> my package builds. Both sbuild and pbuilder can lintian automatically after
> a finished package build.
Thanks.
On Tue, May 03 2022, Tobias Frost wrote:
> Ok, let me check the package: (I'm using the mentors version for the review)
> - As said earlier, depending if you want a Team upload or follow the ITS, this
> needs some entry in d/changelog, depending how you want to proceed...
> - debian/io.github.darrylb123.usbrelay.metainfo.xml should possible brought upstream,
> shouldn't it (no need to change for the upload, but I guess this is
> not debian specific)
I'll send that (as well as other parts) upstream. I first wanted to know
what changes are required for Debian and then send the final version
upstream.
> - d/rules
> - (bikeshed -- no need to change) I'd prefer to use d/clean instead of
> overriding the clean target; overrides are harder to maintain :)
I didn't notice this possibility - it's definitely nicer!
> - the man page is still in the debian directory -- it can be deleted as
> it is now part of upstream.
Upstream has usbrelay.1, I've added usbrelayd.8. As mentioned above,
I'll send it upstream later.
> - same for the udev rules.
Upstream rules are slightly different - looser permissions, different group.
> - d/usbrelay.install has a hard-coded architecture-dependent path. That will break build on
> archs != amd64.
Good catch.
> - d/postinst (and postrm):
> The username is not correct. According to Debian Policy, 4.9.1, it must start with an "_"
> I guess also that you don't want/need a homedirectory for that uses, so its $HOME
> should be /nonexistent in this case. (Policy 9.2.3)
> HOWEVER, let me suggest something else:
> Use the DynamicUser feature from systemd:
>
> DynamicUser=yes
> SupplementaryGroups=plugdev
>
> in the service file should make postint/postrm no longer be needed.
That's definitely a good thing.
> - Lintian has a few remarks: (my related remarks in brackets)
> W: usbrelay source: no-nmu-in-changelog
> W: usbrelay source: source-nmu-has-incorrect-version-number 1.0-1
> (will be gone once the changelog mentions the team upload or after the salvage procedure is done)
> I: usbrelay source: debian-rules-parses-dpkg-parsechangelog (line 20)
> (see abovr)
> I: usbrelay source: older-debian-watch-file-standard 3
> (could be updated to version 4, just s/3/4/ in the header)
done
> I: usbrelayd: package-supports-alternative-init-but-no-init.d-script lib/systemd/system/usbrelayd.service
> (can be ignored)
> I: usbrelay source: patch-not-forwarded-upstream debian/patches/0002-Mention-README-in-the-man-page.patch
> I: usbrelay source: patch-not-forwarded-upstream debian/patches/gcc9.patch
> (see below)
> I: usbrelayd: systemd-service-file-missing-documentation-key [lib/systemd/system/usbrelayd.service]
> (possibly ask upstream to ammend the service file accordingly)
Added, will send upstream later.
> X: usbrelay source: debian-watch-does-not-check-gpg-signature [debian/watch]
> (it would be nice if upstream could pgp-sign their tarballs, so noone can tamper with them.
> They sign their commits already, so a key would be available. Not
> required for this upload.)
I think that tarballs are created automatically by GitHub upon tagging
the release. I guess that for that, we would need to generate tarballs
locally, sign them and upload to github. I think we will change the
release procedure anyway, so we will discuss about this too.
> P: usbrelay source: maintainer-manual-page debian/usbrelayd.8
> (see above -- deletign the file will fix this.)
> X: usbrelay source: prefer-uscan-symlink filenamemangle s/.*(\d[\d\.]*)\.tar\.gz/usbrelay-$1.tar.gz/ [debian/watch:3]
> (ignore that)
> P: usbrelay source: silent-on-rules-requiring-root [debian/control]
> (Add "Rules-Requires-Root: no" to d/control ->
> https://lintian.debian.org/tags/silent-on-rules-requiring-root)
Done.
> X: usbrelayd: systemd-service-file-missing-hardening-features [lib/systemd/system/usbrelayd.service]
> (DynamicUser=yes should fix that too)
> X: usbrelay source: upstream-metadata-file-is-missing
> (would be nice to have, but not required: https://wiki.debian.org/UpstreamMetadata
> example: https://salsa.debian.org/games-team/darkradiant/-/blob/master/debian/upstream/metadata
Will prepare that later.
> You have mentioned that you are involved with upstream: In this case, can
> you check if the debian/patches can be brought upstream? I don't think they are
> to be Debian specific, especially the gcc-9 patch. (no need to do that for this
> upload, but if you forward them now, please add the dep3-Tag "Forwarded" to the
> d/patch)
As mentioned above, that's planned.
> Overall, package quality is nice, only a few things to fix before this can be uploaded.
I've pushed the changes to git. I don't have the hardware with me right
now, so I cannot test whether the changes didn't break something. I'll
test it in the upcomming days and let you know.
Thank you
-Michal
Reply to: