Bug#950988: RFS review of cglm 0.7.1-1
Hi Leon, Jordan did detect something was wrong with the doc. Indeed, there's some issues. It's overall a good review, though missing some advice, which I intend to give myself here. 1/ Sphinx documentation Your package must be using: --with sphinxdoc and the -doc package must depends on ${sphinxdoc:Depends} so that everything is handled automatically for you. I also wouldrecommend the following sequence which I use myself (not mandatory, but nice...): override_dh_sphinxdoc: ifeq (,$(findstring nodoc, $(DEB_BUILD_OPTIONS))) python3 -m sphinx -b html doc/sources \ $(CURDIR)/debian/libcglm-doc/usr/share/doc/libcglm-doc/html dh_sphinxdoc endif then you don't need these: Package: libcglm-doc Depends: fonts-font-awesome, fonts-lato, libjs-jquery, libjs-underscore, neither you need a debian/libcglm-doc.links file, as dh_sphinxdoc will do that for you. 2/ copyright Jordan is right about debian/copyright needing a debian/* section. Though the license that you're using is "Expat", which is one flavor of the MIT license (there's multiple MIT licenses, unfortunately, and Debian recognize this one as "Expat"). 3/ Missing hardening options You may want to add this to your debian/rules: export DEB_BUILD_MAINT_OPTIONS = hardening=+all 4/ Missing .symbol file Please read on here: https://wiki.debian.org/UsingSymbolsFiles 5/ Others You don't need this file: debian/libcglm0.dirs since you're installing things in /usr/lib with debian/libcglm0.install, then there's no need to re-declare /usr/lib a 2nd time. Same thing for debian/libcglm-dev.dirs and debian/libcglm-doc.dirs. 6/ General advice Leon, I strongly recommend that you run Lintian this way on your package before asking for a review: lintian -Ii -E --pedantic *.changes you would have seen the issues, and win time for everyone. Now, if you come back with a corrected package, maybe Jordan can review it again. If he does, I agree to do an initial sponsoring of it (though not the follow-up ones) if Jordan is volunteering for the subsequent ones. Jordan, just let me know when the package is ok. No need to CC me or the archive-...@nm.debian.org anymore. Cheers, Thomas Goirand (zigo) On 4/27/20 12:24 PM, Jordan Justen wrote: > Leon, > > I reviewed the 0.7.1-1 packaging you posted on mentors.debian.net. I > didn't see any major issues, but maybe some suggestions. > > The license is MIT, and debian/copyright has it listed properly. > > I think packages often will call out the debian directory in > debian/copyright, even if it matches the upstream license. I don't > know that this is required, but here's an example: > > https://salsa.debian.org/xorg-team/lib/libglvnd/-/blob/debian-unstable/debian/copyright#L50 > > You can also move the license text to a separate section if multiple > sections list the same license. (Like the MIT license in the example > above.) > > I did see that `lintian --display-info` prints some issues relating to > fonts in the doc package. From `build-rdeps python3-sphinx-rtd-theme`, > I found the dbus-python package also uses the rtd theme. I notice they > use dh_sphinxdoc, and I wonder if it could be useful for the cglm > package. (And, perhaps fix the lintian note.) > > lintian --display-info also flags no-symbols-control-file > usr/lib/x86_64-linux-gnu/libcglm.so.0.0.1. (See dpkg-gensymbols) > > lintian --pedantic --display-experimental flags > debian-watch-does-not-check-gpg-signature, but I see upstream doesn't > sign the releases. You could ask upstream to do this, and pointing > them at https://wiki.debian.org/Creating%20signed%20GitHub%20releases > might make it easier for them. > > I don't these the info or experimental lintian items would block the > package from debian, since they aren't even at the warning level. > > One easy cleanup change for the package would be to run wrap-and-sort. > > Do you plan to try to maintain the package going forward? (Watch for > new upstream releases, work on bugs, etc?) > > Do you have plans to manage the package files in git, perhaps on > salsa.debian.net? I'm not sure if it is allowed to start using salsa > for a project before it makes it into debian. The salsa FAQ is vague > on this point: > > https://wiki.debian.org/Salsa/FAQ#What_can_be_hosted_on_salsa > > But I think it is often allowed. I know of at least 2 cases where a > repo was created for a package before it got included into Debian. I > expect some basic review of the package is probably good first, and > perhaps this email can serve for that. > > If not salsa.debian.net, you could still host it in a github repo and > include the links to it in the control file. (And, that could move to > salsa later too.) > > -Jordan > > Some more info, mainly for Thomas to know what I checked. > > I looked over https://wiki.debian.org/SponsorChecklist. > > I checked the provided orig tarball vs upstream. > > I built the package with sbuild. > > I looked over the lib, dev and doc packages in the control file and > .de
Bug#950988: RFS review of cglm 0.7.1-1
Leon, I reviewed the 0.7.1-1 packaging you posted on mentors.debian.net. I didn't see any major issues, but maybe some suggestions. The license is MIT, and debian/copyright has it listed properly. I think packages often will call out the debian directory in debian/copyright, even if it matches the upstream license. I don't know that this is required, but here's an example: https://salsa.debian.org/xorg-team/lib/libglvnd/-/blob/debian-unstable/debian/copyright#L50 You can also move the license text to a separate section if multiple sections list the same license. (Like the MIT license in the example above.) I did see that `lintian --display-info` prints some issues relating to fonts in the doc package. From `build-rdeps python3-sphinx-rtd-theme`, I found the dbus-python package also uses the rtd theme. I notice they use dh_sphinxdoc, and I wonder if it could be useful for the cglm package. (And, perhaps fix the lintian note.) lintian --display-info also flags no-symbols-control-file usr/lib/x86_64-linux-gnu/libcglm.so.0.0.1. (See dpkg-gensymbols) lintian --pedantic --display-experimental flags debian-watch-does-not-check-gpg-signature, but I see upstream doesn't sign the releases. You could ask upstream to do this, and pointing them at https://wiki.debian.org/Creating%20signed%20GitHub%20releases might make it easier for them. I don't these the info or experimental lintian items would block the package from debian, since they aren't even at the warning level. One easy cleanup change for the package would be to run wrap-and-sort. Do you plan to try to maintain the package going forward? (Watch for new upstream releases, work on bugs, etc?) Do you have plans to manage the package files in git, perhaps on salsa.debian.net? I'm not sure if it is allowed to start using salsa for a project before it makes it into debian. The salsa FAQ is vague on this point: https://wiki.debian.org/Salsa/FAQ#What_can_be_hosted_on_salsa But I think it is often allowed. I know of at least 2 cases where a repo was created for a package before it got included into Debian. I expect some basic review of the package is probably good first, and perhaps this email can serve for that. If not salsa.debian.net, you could still host it in a github repo and include the links to it in the control file. (And, that could move to salsa later too.) -Jordan Some more info, mainly for Thomas to know what I checked. I looked over https://wiki.debian.org/SponsorChecklist. I checked the provided orig tarball vs upstream. I built the package with sbuild. I looked over the lib, dev and doc packages in the control file and .deb outputs. I *did not* use the library with any code, but I did note that the upstream package has 633 tests which all passed during the sbuild build. I ran piuparts, and it reported that all tests passes. I reviewed each item on https://ftp-master.debian.org/REJECT-FAQ.html: Serious violations checklist (REJECT-FAQ): * OpenSSL - not linked => Ok * CDBS - doesn't use => Ok * PHP License - MIT license => Ok * License - upsteam has LICENSE file => Ok * Transition - Ok * Experimental Library - not linked => Ok * Hijack - no => Ok * Package split * FTBFSIASW - built with sbuild => Ok * debian/control breakage #2 - Ok * Copyright - Ok, with comment * optionally, could add a separate debian section * https://salsa.debian.org/xorg-team/lib/libglvnd/-/blob/debian-unstable/debian/copyright#L50 * Need to check for multiple copyright holders * License II - upsteam has LICENSE file => Ok * License III - Ok * Using `licensecheck`, it appears "Recep Aslantas" is the only listed copyright holder * Non-Main - checked Depends on .deb files, Build-Depends, Recommended manually => Ok * Non-Main II - Ok * Package Content - Ok * Policy Violation - * manually inspected - looks ok * lintian reports some font issues at "info" level * font-in-non-font-package usr/share/doc/libcglm-doc/html/_static/fonts/Lato-Bold.woff2.gz * and, several more. this appears to come from debian/libcglm-doc.links * Contents of orig.tar.gz - not being rebuilt; no *.{a,so} => Ok * Lintian (with --display-info) * libcglm-dev: capitalization-error-in-description api API * libcglm-doc: font-in-non-font-package usr/share/doc/**/*.woff2.gz * libcglm-doc: font-outside-font-dir usr/share/doc/**/*.woff2.gz * libcglm0: no-symbols-control-file usr/lib/x86_64-linux-gnu/libcglm.so.0.0.1 * rpath - not used => Ok * Package name - Ok * Common license - MIT not under common-license => Ok * Renaming source for DFSG-removals - not dfsg repacked => Ok * Wrong license pointer - no common-license is used => Ok * Symbol file wrong - no symbol file currently (but, suggested) => Ok * Source missing - no missing source => Ok * Generated files - none seen, or flagged by lintian => Ok * Package uses waf as build system - autotools => Ok * Source package missing source - no source missing => Ok Minor issue che