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
