Bug#804169: RFS: libjreen/1.2.0-1

[Gianfranco Costamagna]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On Fri, 20 Nov 2015 12:37:50 +0100 Stefan Ahlers
 wrote:
> Hi,
> 
> I've updated libjreen. Now it uses the new jdns package. For
> finding it correctly, I had to patch the CMakeList.txt file.
> 
> I also extend the package to build Qt5 based packages, too.
> 
> I really do not know what's happened with the QA informations on 
> http://mentors.debian.net/package/libjreen The watch file exists
> and the compatibility level is set. What's wrong?
> 
> Stefan

Built, thanks for your contribution to Debian!

(and sorry for the looong wait)
cheers,

G.
> 
> 
> 
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQIcBAEBCAAGBQJWaDpFAAoJEPNPCXROn13Z8TMP/1VSKcrlAFfKI5uMEuL0W4bH
BcVGdchL+71iW/eUZNtezwuysPSngzl1Roz7UUSoXRgePTiZascimRqwntAe/kSa
zq3H1FVlfA0ykivWPd6VfUdkcDGDLPEnsm64Z/tMqmqxTIp6i7E68X37X+mAaHL0
gFf3tMpNn7OnWkUVeYJPoV9+lcg/gTL/++GtIk1YYkt8hXLyDR+uf4T8JWpx3tyE
V6OcFmTSlYTM7wfOb0LVuQWvCPyRBDBK2Ogz/a1fTih78NTAhHUTuVZrpkawqEmO
80wK6nZIaPUptTGnTk78g1bXVMOi1KBknHY3eRuwQ9ggIyoRERmujhacGlpRvC5T
g1Cg5OZevloFUAVdJNfPcQjMqx9gcn0OE1m2EuJlvtJusIjRcrNCZstNz/cL5DaQ
0DctKenXzCUcH5avJxauiSmTG2Sv/MQF76ISPZH/ImtsMlAVBQw4D0/FPaWi1oK8
l/6021Fmcwo0LjLqoE6j3cgkkww997OfalyS1uY3p7jSvSwYgdSS45GObqpVU6O5
yCftD/AfCh45lc0CFzASh8Vru3DOPYYPEYbvtPviw1j5p4UjHCein4tRpNqGx480
41w4V3zybboHoxB8UEaaqzPgC9aXnrR8P/bwtZIjEQT8s97nEOzvFvJLykDd+AIr
60LikOpJztGGq2PqjBUy
=gcdw
-END PGP SIGNATURE-

Bug#804169: RFS: libjreen/1.2.0-1

[Stefan Ahlers]

Hi,

I've updated libjreen. Now it uses the new jdns package. For finding it
correctly, I had to patch the CMakeList.txt file.

I also extend the package to build Qt5 based packages, too.

I really do not know what's happened with the QA informations on
http://mentors.debian.net/package/libjreen
The watch file exists and the compatibility level is set. What's wrong?

Stefan

Bug#804169: RFS: libjreen/1.2.0-1

[Gianfranco Costamagna]

Hi

>
>I understand the problem. I've taken a look into the cmake file and I

>found out that libjreen does not use "icesupport" automatically.
>
>→ option(JREEN_USE_IRISICE "Use ICE from IRIS" OFF)


ok
>On the other side libjreen is looking for a external JDNS. And so it
>would be really better to create a libjdns package for debian, or?


http://www.rpmfind.net/linux/rpm2html/search.php?query=jdns-devel
yes, and seems that fedora already has it
(assuming it is the same package, I didn't check)


>Please correct me if I'm wrong, but I do not agree in your opinion
>because other distributions uses another way of multiarch support.



yes, correct.

Indeed not all multiarch locations might have a "/" in their path.

>For example openSUSE, they uses the path "/usr/lib64/libjreen.so.1". And
>so I think it is impossible to upstream a patch, which changes
>"DESTINATION lib${LIB_SUFFIX}" into "DESTINATION lib/${LIB_SUFFIX}"

>because this would break the support for the other distributions.

yes, I googled around, looked at codesearch.debian.net and you are right,
thanks for the explanation!

>I really do not know how to fix it to make it work for all
>distributions. Any ideas?


your solution is fine :)

>I've sent the the codespelling patch to upstream.
>The cppcheck errors is only a "Uninitialized variable: c". I think this
>is not critical.
>Most of the "$ grep -riE 'fixme|todo|hack|xxx' ." errors occurs on
>icesupport and this is unused. The rest is not critical, too.
>Same on licensecheck. It does not affect this package.


ack.

let me know then if you have some news about this and I'll look at it again
(specially the external library problem)


cheers,

G.

Bug#804169: RFS: libjreen/1.2.0-1

[Gianfranco Costamagna]

Hi Stefan,


>3)  Sorry, I do not understand how to pack the 3rdparty libraries
>separately. The libraries are not compiled and Jreen only needs them for
>compile itself and they are not shipped in the package afterwards.


yes, but let assume one of the libraries above have a security bug.
you will need to do a source only upload, instead of just fixing the library 
and rebuild the affected packages
(assuming there will be one day more packages using the above libraries

you can always ship them as source only libraries, e.g.
https://packages.qa.debian.org/w/websocketpp.html

(unless the libraries are strictly internal and makes no sense outside this 
particular project)

>4) done – I've change your suggestion a bit to avoid patching cmake by
>using a "/" before "$(DEB_HOST_MULTIARCH)"
>
>→ dh_auto_configure -- -DLIB_SUFFIX=/$(DEB_HOST_MULTIARCH)

>
>It works!


I know, but you are Debian-patching an upstream issue.
you are workarounding it, not fixing (even if it works).

I would appreciate a proper fix and a patch sent upstream (in the meanwhile you 
can of course use this solution,
It came in my mind, but I didn't suggest it because I don't think it is the 
best way)


>5) The file ./alttoolbar_rb3compat.py: does not exists in the source. It>looks 
>like you mix me up with someone else, maybe the maintainer of the
>"rhythmbox-plugin-alternative-toolbar"?


yes, bad copy-paste :)

>Ok, I've checked up the spelling and code errors and I can confirm them,
>but I'm not the developer of jreen. Do I have to patch this mistakes as
>maintainer, too? Most of the errors occurs at the 3rdparty tool
>"icesupport".


well, you can send patches upstream, it is fine by me to avoid debian patches 
as long as the mistakes are in the code
and not seen by normal users (I can also accept a mistake in some obscure code 
that is mostly unreachable, but I would 

bother about sending bugs upstream and fix in a later release)


cheers,

G.

Bug#804169: RFS: libjreen/1.2.0-1

[Stefan Ahlers]

Am 06.11.2015 um 13:24 schrieb Gianfranco Costamagna:

Hi,
> yes, but let assume one of the libraries above have a security bug.
> you will need to do a source only upload, instead of just fixing the
> library and rebuild the affected packages (assuming there will be one
> day more packages using the above libraries you can always ship them
> as source only libraries, e.g.
> https://packages.qa.debian.org/w/websocketpp.html (unless the
> libraries are strictly internal and makes no sense outside this
> particular project) 
I understand the problem. I've taken a look into the cmake file and I
found out that libjreen does not use "icesupport" automatically.

→ option(JREEN_USE_IRISICE "Use ICE from IRIS" OFF)

On the other side libjreen is looking for a external JDNS. And so it
would be really better to create a libjdns package for debian, or?
> I know, but you are Debian-patching an upstream issue. you are
> workarounding it, not fixing (even if it works). I would appreciate a
> proper fix and a patch sent upstream (in the meanwhile you can of
> course use this solution, It came in my mind, but I didn't suggest it
> because I don't think it is the best way) 
Please correct me if I'm wrong, but I do not agree in your opinion
because other distributions uses another way of multiarch support.

For example openSUSE, they uses the path "/usr/lib64/libjreen.so.1". And
so I think it is impossible to upstream a patch, which changes
"DESTINATION lib${LIB_SUFFIX}" into "DESTINATION lib/${LIB_SUFFIX}"
because this would break the support for the other distributions.

I really do not know how to fix it to make it work for all
distributions. Any ideas?

> well, you can send patches upstream, it is fine by me to avoid debian
> patches as long as the mistakes are in the code and not seen by normal
> users (I can also accept a mistake in some obscure code that is mostly
> unreachable, but I would bother about sending bugs upstream and fix in
> a later release) cheers, G. 

I've sent the the codespelling patch to upstream.
The cppcheck errors is only a "Uninitialized variable: c". I think this
is not critical.
Most of the "$ grep -riE 'fixme|todo|hack|xxx' ." errors occurs on
icesupport and this is unused. The rest is not critical, too.
Same on licensecheck. It does not affect this package.

Cheers,
Stefan

Bug#804169: RFS: libjreen/1.2.0-1

[Stefan Ahlers]

Package: sponsorship-requests
Severity: normal [important for RC bugs, wishlist for new packages]

Dear mentors,

I am looking for a sponsor for my package "libjreen"

 * Package name: libjreen
   Version : 1.2.0-1
   Upstream Author : Ruslan Nigmatullin 
 * URL : https://github.com/euroelessar/jreen
 * License : GPL-2+
   Section : libs

 It builds those binary packages:

libjreen-dbg - Qt XMPP library - debugging symbols
libjreen-dev - Qt XMPP library - development files
libjreen1  - Qt XMPP library

 To access further information about this package, please visit the following 
URL:

  http://mentors.debian.net/package/libjreen


 Alternatively, one can download the package with dget using this command:

dget -x 
http://mentors.debian.net/debian/pool/main/libj/libjreen/libjreen_1.2.0-1.dsc

 I'm the maintainer of the tomahawk PPA (https://launchpad.net/~tomahawk) and I 
want do bring tomahawk
 into debian but it's blocked by libjreen.

 This package based on the package of the ubuntu sources but I've added 
multiarch support.
 
(http://packages.ubuntu.com/search?suite=default=all=any=names=libjreen)

 Regards,
   Stefan Ahlers
   (https://launchpad.net/~justin-time)

Bug#804169: RFS: libjreen/1.2.0-1

[Gianfranco Costamagna]

Control: owner -1 !
Control: tags -1 moreinfo



Hi
let's review:


1) changelog: you need to have only one entry and an ITP bug closed
https://www.debian.org/devel/wnpp/

2) priority: optional


"Pre-Depends:" I guess you can drop them, because they aren't needed anymore
for multiarch packages


3) 3rdparty, what about packaging the libraries here separately?

4)debian/*.install
I would avoid this

usr/lib/libjreen.so.1* usr/lib/%DEB_HOST_MULTIARCH%


but I would like something like
usr/lib/*/*.so.*

and so on

you should patch the rules file to use multiarch directly

e.g.
DEB_HOST_MULTIARCH?= $(shell dpkg-architecture -qDEB_HOST_MULTIARCH)

%:
dh $@ --dbg-package=libjreen-dbg --parallel

override_dh_auto_configure:
dh_auto_configure -- -DLIB_SUFFIX=$(DEB_HOST_MULTIARCH)


this will allow you to remove all the .in hacks

BTW this also need to patch cmake like this (I guess, didn't try)
DESTINATION lib${LIB_SUFFIX}/pkgconfig


to
DESTINATION lib/${LIB_SUFFIX}/pkgconfig


(and upstream this patch)

5) copyright:
./alttoolbar_rb3compat.py:# Copyright (C) 2012 - Agustin Carrasco


check-all-the-things:


$ codespell --quiet-level=3
./src/error.cpp:149: occured  ==> occurred
./src/vcard.h:47: adress  ==> address
./src/registrationmanager.cpp:125: Unkown  ==> Unknown
./src/error.h:130: occured  ==> occurred


$ cppcheck -j1 --quiet -f . | grep -vF 'cppcheck: error: could not find or open 
any of the paths given.'

(something)


$ grep -riE 'fixme|todo|hack|xxx' .
(something)


something for upstream
$ licensecheck * -r 2> /dev/null |grep incorrect

3rdparty/icesupport/bytestream.cpp: LGPL (v2.1 or later) (with incorrect FSF 
address)



Please note: some of them might be nitpicks/false positive, please check and 
report back :)
thanks!

(I know it is a lot of work, but the initial review is always the most 
difficult for both parts)

cheers,

Gianfranco

Bug#804169: RFS: libjreen/1.2.0-1

[Stefan Ahlers]

Hi,

thanks for your fast reply!

1) done

2) done

3)  Sorry, I do not understand how to pack the 3rdparty libraries
separately. The libraries are not compiled and Jreen only needs them for
compile itself and they are not shipped in the package afterwards.

4) done – I've change your suggestion a bit to avoid patching cmake by
using a "/" before "$(DEB_HOST_MULTIARCH)"

→ dh_auto_configure -- -DLIB_SUFFIX=/$(DEB_HOST_MULTIARCH)

It works!

5) The file ./alttoolbar_rb3compat.py: does not exists in the source. It
looks like you mix me up with someone else, maybe the maintainer of the
"rhythmbox-plugin-alternative-toolbar"?

Ok, I've checked up the spelling and code errors and I can confirm them,
but I'm not the developer of jreen. Do I have to patch this mistakes as
maintainer, too? Most of the errors occurs at the 3rdparty tool
"icesupport".

Regards,
Stefan Ahlers