Bug#950988: RFS review of cglm 0.7.1-1

2020-04-27 Thread Thomas Goirand

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

2020-04-27 Thread Jordan Justen
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