Path: csiph.com!aioe.org!bofh.it!news.nic.it!robomod
From: Georg Faerber <georg@riseup.net>
Newsgroups: linux.debian.maint.python
Subject: Re: RFS: mwic 0.7.4-1
Date: Tue, 20 Mar 2018 18:20:01 +0100
Message-ID: <vvt5v-79z-1@gated-at.bofh.it>
References: <vu5eV-1xH-7@gated-at.bofh.it> <vuaHE-5ht-1@gated-at.bofh.it> <vugN4-zo-19@gated-at.bofh.it> <vuxkS-2NZ-7@gated-at.bofh.it>
Old-Return-Path: <georg@riseup.net>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="6Jy4/gb9ZgnsEOX8"
Content-Disposition: inline
Approved: robomod@news.nic.it
Lines: 243
Organization: linux.* mail to news gateway
Sender: robomod@news.nic.it
Xref: csiph.com linux.debian.maint.python:10281


--6Jy4/gb9ZgnsEOX8
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi pabs,

Thanks a lot for your review, and sorry for not responding earlier, too
much stuff on my table:

On 18-03-18 11:33:59, Paul Wise wrote:
> The copyright years are missing 2012, I'm not sure if the ftp-masters
> would reject the package on that basis.

I've referred to the upstream git history, instead of the licence file.
Fixed.

> I require these things to be fixed before I would sponsor the package:
>=20
> doc/mwic.1 and tests/coverage are generated files, they should be
> removed from the upstream VCS and tarballs and be always built from
> source. If upstream refuses, you should remove them in `debian/rules
> clean` and very early in `debian/rules build` so that the package will
> never depend on the existing upstream files.

I've searched quite a bit on the Internets how to do this, and leverage,
for now, dh_clean. I hope that this is an acceptable method for this, I
didn't found any "official" source. I'll approach upstream regarding
these files.

Also, the man page is now generated during dh_auto_install. I've added a
build dependency on python3-docutils for this.

> Some things that you may want to improve at some point:
>=20
> Please include the upstream signature in the source package. uscan
> does the right thing these days, or you can rename it to
> mwic_0.7.4.orig.tar.gz.asc and place it alongside the orig.tar.gz file
> and rebuild the source package.

Thanks for this pointer -- highly appreciated!

Actually, I've debugged this for quite some time during the last year,
and exchanged mails regarding this with Holger Levsen and Guido G=C3=BCnther
seeking a way to include the upstream signature. Back then, I've read
[1], and the man pages as well, but still couldn't figure it out -
dropping the .asc into the top level directory didn't made a difference.
After quite some time, I finally gave up. Up until yesterday I thought
the tooling is currently limited.

After reading your mail I debugged this again: It's not about the top
level directory, at least if using gbp buildpackage, but about
'export-dir'. Placing a symlink inside there which points to the actual
signature makes this work! :)

The new upload should look better now in this regard.

For now, I've wrote a wrapper script just for personal use, but I'll
work on [2] and [3] to get this implemented in a proper way.

> I like to wrap each of the fields in debian/watch on new lines.

Although I think this is a matter of personal taste: fixed.

> I like to use these wrap-and-sort parameters:
>=20
> wrap-and-sort --short-indent --wrap-always --sort-binary-packages
> --trailing-comma

See above. (Personally, I really dislike the trailing comma.)

> misc/mwic4po seems like it would be worth installing in the package?

There is no man page currently, and also, more importantly, out of the
box this doesn't run in a non-interactive way. I guess, at least some
people, might run mwic via a CI pipeline, that's why I don't ship this.
I'll approach upstream regarding this.

> Please add some upstream metadata:
>=20
> https://wiki.debian.org/UpstreamMetadata

I've did this as well, although I'm unsure why this isn't defined in the
policy. I'm aware that this was recently added to lintian, and reading
the bug, again, makes me wonder what's the process of getting a new
check into lintian. As it stands currently, some people take care of
this, and some don't. This is of course not set in stone, but...well,
not sure.

> Output from automated checks is available at the very end of this
> email.

Thanks!

> > Could you move the repo as well, or create a new one within the
> > Python team?
>=20
> I'll leave that to someone who knows how to do that, I haven't dealt
> with salsa yet.

Alright!

> > Yeah, I've used this as well, but in this case it's not that helpful
> > as it finds the code defining the error [2].
>=20
> I'd guess this is the source of the line you quoted:
>=20
> https://sources.debian.org/src/autopkgtest/5.1/runner/autopkgtest/?hl=3D1=
44#L138

Correct. Still, my question stands: How do I enable autopkgtests for
this package?

> X: mwic source: upstream-metadata-file-is-missing

Fixed.

> check-all-the-things
>=20
> $ env PERL5OPT=3D-m-lib=3D. cme check dpkg
> ...
> Warning in 'control source Build-Depends:0' value 'debhelper (>=3D
> 11~)': should be (>=3D 11) not (>=3D 11~) because compat is 11
> Warning in 'control source Build-Depends:1' value 'python3 (>=3D 3.2)':
> unnecessary greater-than versioned dependency: python3 (>=3D 3.2).
> Debian has oldoldstable -> 3.2.3-6; oldstable -> 3.4.2-2;
> oldstable-kfreebsd -> 3.4.2-2; stable -> 3.5.3-1; unstable -> 3.6.3-2;
> testing -> 3.6.4-1; unstable -> 3.6.4-1;
> Warning in 'control binary:mwic Depends:0' value 'python3 (>=3D 3.2)':
> unnecessary greater-than versioned dependency: python3 (>=3D 3.2).
> Debian has oldoldstable -> 3.2.3-6; oldstable -> 3.4.2-2;
> oldstable-kfreebsd -> 3.4.2-2; stable -> 3.5.3-1; unstable -> 3.6.3-2;
> testing -> 3.6.4-1; unstable -> 3.6.4-1;

Fixed.

> $ doc8
> Scanning...
> Validating...
> .../mwic-0.7.4/doc/manpage.rst:19: D001 Line too long
> .../mwic-0.7.4/doc/manpage.rst:20: D001 Line too long
> .../mwic-0.7.4/doc/manpage.rst:53: D001 Line too long

I'll take these upstream.

> # You may want to file a bug on license-reconcile
> # about these false positives re Expat vs MIT/X11
> $ env PERL5OPT=3D-m-lib=3D. license-reconcile
> ...
> License mismatch: File private/check-rst has license MIT/X11 (BSD
> like) which does not match Expat. at
> /usr/share/perl5/Debian/LicenseReconcile/App.pm line 222, <GEN0> line
> 3.
> <lots more>

The texts are the same, there are only differences in special chars like
", so I guess this is a false positive.

> # This command checks style. While a consistent style
> # is a good idea, people who have different style
> # preferences will want to ignore some of the output.
> # Do not bother adding non-upstreamable patches for this.
> $ find . -type f -iname '*.py' -exec pycodestyle --ignore W191 {} +
> <lots>
>=20
> # This command checks style. While a consistent style
> # is a good idea, people who have different style
> # preferences will want to ignore some of the output.
> # Do not bother adding non-upstreamable patches for this.
> $ pydocstyle .
> <lots>
>=20
> $ find . -type f -iname '*.py' -exec pylint3 --rcfile=3D/dev/null
> --msg-template=3D'{path}:{line}:{column}: [{category}:{symbol}] {obj}:
> {msg}' --reports=3Dn {} +
> <lots>
>=20
> $ python3-bandit -r .
> <lots>
>=20
> $ grep -nHriE 'fixme|todo|hack|xxx+|broken' .
> ./tests/multiword-t-he.txt:6:# FIXME:
> ./tests/test_extdict.py:121:    t({'aasumes'}, {'assumes'})  # FIXME? 'as=
sumes'
> ./tests/test_extdict.py:122:    t({'Addtional'}, {'Additional'})  #
> FIXME? 'addtional'
> ./tests/multiword-t-he.exp:17:FIXME:
> ./tests/multiword-t-he.exp:18:| # FIXME:
> ./private/run-pylint:32:log=3D$(mktemp -t pylint.XXXXXX)
> ./.pylintrc:5:    fixme,
>=20
> $ vulture .
> lib/cli.py:54: Unused variable 'namespace'
> lib/cli.py:54: Unused variable 'option_string'
> lib/cli.py:187: Unused variable 'ch'
> lib/cli.py:192: Unused variable 'ch'
> lib/cli.py:310: Unused variable 'args'
> lib/colors.py:32: Unused variable 'warn'
> lib/colors.py:35: Unused variable 'unreverse'
> tests/test_blackbox.py:35: Unused attribute 'maxDiff'
> tests/test_blackbox.py:85: Unused attribute 'redundant'
> tests/test_blackbox.py:90: Unused variable 'enabled'
> tests/test_blackbox.py:101: Unused function 'loadTestsFromFile'
> tests/test_blackbox.py:105: Unused function 'wantFunction'
> tests/test_blackbox.py:115: Unused function '_test'

I'll take these to upstream, too.

Changes pushed to git and uploaded to m.d.n [4].

Thanks again,
cheers,
Georg


[1] https://lists.debian.org/debian-devel/2017/08/msg00072.html
[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=3D874550
[3] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=3D872864
[4] https://mentors.debian.net/debian/pool/main/m/mwic/mwic_0.7.4-1.dsc

--6Jy4/gb9ZgnsEOX8
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEE4p3Q++yWB9ljva4NWtdbQU6kFmcFAlqxQNIACgkQWtdbQU6k
Fmc/rQ//cxNaGIg0umfP0H3G6mWOd4XYNU3S0u9O8S6rk9Y9r1J7osiKzxIveCem
zu/3F6xrXYYcUcB8enLQLUEQHR7Z0DdkX25WTCorIpXLLhimEu0c/0Epy0sY7AX4
FkWVtMP/gfYxTkH8xumJSotjjhbH1cSx6FFB+9CGFZBXP6+ActDeHBnlet6UMGyQ
N6mfy7iUejvEyLvXACbmuLUw3HFGTE6CyRcg1L/TlD4m+kqFrkGgGJ/0xDahwWMq
sfeTGCrPTLFfEL49thbl7mb/ENwaghtmiKsD8CTSOkanTD2LvaxfsHAP5ha3Hupp
BJj+RixVBEds11XM1nRv1vpzrFeuQgMI50jfD45yR0GfldOlBOZNRM7IQ2qQtr8o
9aGSjWUpeb44HAcB1nrvZ/JSqaQ+0vSAf2jevUAtH8CpbbAr4xx4jtBqijOUmZ3I
CB6sMGGpIvSC8P5+GafFBnYj0MxE/5PtrL9+HFOE5V31vFI7jkY+F+6RagBgCwjh
K5lOkp3I1HP8oxRQrk7dnNel/BnLQpPsYq03GB/26xxK7hq78be1aQn5D1C57i03
t4qkH9eBWKHeY9fM9vrF+EKE7OP9Q9FIdLCDg+XvsMHxaEXB5gx9MqFPU3GqeY6Q
34KrbRYSUNxr5nwRFjacqwZxZJ/DUkRjpSoSp82hxPyDU3wKVHw=
=L9B9
-----END PGP SIGNATURE-----

--6Jy4/gb9ZgnsEOX8--