[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#951202: 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-727@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
> .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 checklist (REJECT-FAQ):
>
>  * Description - Ok
>  * Repackage orig.tar.gz - not repacked => Ok
>  * Native/Non-Native - non-native => Ok
>  * Multiple versions - only 1 version => Ok
>  * debian/rules - Ok
>  * .ex files - Ok
>  * Standards-Version - Ok
>  * Manpages - no manpages, but not a requirement
>  * Useless Settings - Ok
>


Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: