Bug#689041: Re: Orthanc

2012-10-08 Thread Sebastien Jodogne

Hello Mathieu,

I have uploaded another version of the package that should fix your 
comments:

http://mentors.debian.net/debian/pool/main/o/orthanc/orthanc_0.2.2-6.dsc


1.
$ man -l -k docs/orthanc.1
docs/orthanc.1: nothing appropriate.
You can use help2man to quickly generate a proper man page.


I now use help2man.


Technically it would be nice to use DEP3 for patch, but I assume
'upstream' is aware of those ...


I have included the DEP3 required fields.

Thanks again,
Sébastien-


--
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/5072c8ae.1020...@chu.ulg.ac.be



Bug#689041: Re: Orthanc

2012-10-07 Thread Mathieu Malaterre
Sébastien,

On Fri, Oct 5, 2012 at 6:35 PM, Mathieu Malaterre
 wrote:
> Why did you choose gnutls ? Your code seems to be using the openssl
> one. And your copyright is explicitely setup to deal with openssl
> exception anyway ?

Another set of minor comments.

1.
$ man -l -k docs/orthanc.1
docs/orthanc.1: nothing appropriate.
You can use help2man to quickly generate a proper man page.

2.
Some leftover:
$ more debian/copyright
[...]
Files: ThirdPartyDownloads/jsoncpp-src-0.5.0.tar.gz
Copyright: Baptiste Lepilleur 
License: Public Domain

3.
d/changelog, one entry is enough.

4.
You can use quilt refresh to refresh dynamic-jsoncpp, to get rid of:
dpkg-source: warning: diff
`orthanc-0.2.2/debian/patches/dynamic-jsoncpp' patches file
orthanc-0.2.2/Resources/CMake/JsonCppConfiguration.cmake twice

Technically it would be nice to use DEP3 for patch, but I assume
'upstream' is aware of those ...

Thanks
-- 
Mathieu


--
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/ca+7wusypfdqw1ga5yerqwxu+u_ywuezhibzrmxlrc-mytgp...@mail.gmail.com



Bug#689041: Re: Orthanc

2012-10-05 Thread s . jodogne
Hi Mathieu,

> >> An alternative would be to use "libcurl4-gnutls-dev |
> >> libcurl4-dev".
> >
> > Thank for your reply! I have just uploaded another version of the
> > package
> > with your suggested improvement:
> > http://mentors.debian.net/debian/pool/main/o/orthanc/orthanc_0.2.2-3.dsc
> 
> Why did you choose gnutls ? Your code seems to be using the openssl
> one. And your copyright is explicitely setup to deal with openssl
> exception anyway ?

My code indeed links against OpenSSL to deal with the HTTPS protocol inside the 
embedded Web server (Mongoose).

However, curl is used for the connection among Orthanc peers, in a totally 
separate part of the software. I actually do not care about how curl handles 
HTTPS requests, as I dynamically link against curl. I have always thought that 
the gnutls version of curl is the one that is installed by default in Debian, 
but I may be wrong. That being said, I can evidently easily switch to the 
openssl version of curl.

Could you tell me what is the preferred version of curl in Debian?

TIA,
Sébastien-


--
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/130916679.3489241.1349460878733.javamail.r...@serv224.segi.ulg.ac.be



Bug#689041: Re: Orthanc

2012-10-05 Thread Mathieu Malaterre
On Fri, Oct 5, 2012 at 6:02 PM, Sebastien Jodogne
 wrote:
> Dear Gregor,
>
>
 * why libcurl4-gnutls-dev | libcurl4-nss-dev | libcurl4-openssl-dev ?
 shouldn't that be something simplier like libcurl4-dev ? libcurl-dev ?
 libcurl-ssl-dev ?
>>>
>>>
>>> If I use "libcurl4-dev", "libcurl-dev" or "libcurl-ssl-dev", I
>>> obtain the following Lintian warning:
>>>
>>> http://lintian.debian.org/tags/virtual-package-depends-without-real-package-depends.html
>>>
>>> For this reason, I have preferred to keep my explicit enumeration.
>>
>>
>> An alternative would be to use "libcurl4-gnutls-dev | libcurl4-dev".
>
>
> Thank for your reply! I have just uploaded another version of the package
> with your suggested improvement:
> http://mentors.debian.net/debian/pool/main/o/orthanc/orthanc_0.2.2-3.dsc

Why did you choose gnutls ? Your code seems to be using the openssl
one. And your copyright is explicitely setup to deal with openssl
exception anyway ?

thx
-- 
Mathieu


-- 
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/CA+7wUszeoi3MJ76OkAirb0AYNQbyfX=0gpSC5=vh7bhd4eh...@mail.gmail.com



Bug#689041: Re: Orthanc

2012-10-05 Thread Sebastien Jodogne

Dear Gregor,


* why libcurl4-gnutls-dev | libcurl4-nss-dev | libcurl4-openssl-dev ?
shouldn't that be something simplier like libcurl4-dev ? libcurl-dev ?
libcurl-ssl-dev ?


If I use "libcurl4-dev", "libcurl-dev" or "libcurl-ssl-dev", I
obtain the following Lintian warning:
http://lintian.debian.org/tags/virtual-package-depends-without-real-package-depends.html

For this reason, I have preferred to keep my explicit enumeration.


An alternative would be to use "libcurl4-gnutls-dev | libcurl4-dev".


Thank for your reply! I have just uploaded another version of the 
package with your suggested improvement:

http://mentors.debian.net/debian/pool/main/o/orthanc/orthanc_0.2.2-3.dsc

Sébastien-


--
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/506f04ab.9070...@chu.ulg.ac.be



Bug#689041: Re: Orthanc

2012-10-05 Thread gregor herrmann
On Fri, 05 Oct 2012 16:24:14 +0200, Sebastien Jodogne wrote:

> >* why libcurl4-gnutls-dev | libcurl4-nss-dev | libcurl4-openssl-dev ?
> >shouldn't that be something simplier like libcurl4-dev ? libcurl-dev ?
> >libcurl-ssl-dev ?
> 
> If I use "libcurl4-dev", "libcurl-dev" or "libcurl-ssl-dev", I
> obtain the following Lintian warning:
> http://lintian.debian.org/tags/virtual-package-depends-without-real-package-depends.html
> 
> For this reason, I have preferred to keep my explicit enumeration.

An alternative would be to use "libcurl4-gnutls-dev | libcurl4-dev". 

Cheers,
gregor

-- 
 .''`.  Homepage: http://info.comodo.priv.at/ - OpenPGP key 0xBB3A68018649AA06
 : :' : Debian GNU/Linux user, admin, and developer  -  http://www.debian.org/
 `. `'  Member of VIBE!AT & SPI, fellow of the Free Software Foundation Europe
   `-   NP: Andrew Lloyd Webber & Tim Rice


signature.asc
Description: Digital signature


Bug#689041: Re: Orthanc

2012-10-05 Thread Sebastien Jodogne

Dear Mathieu,


Many thanks for your feedback. I have just uploaded a new version of the
Orthanc package:
http://mentors.debian.net/package/orthanc
http://mentors.debian.net/debian/pool/main/o/orthanc/orthanc_0.2.2-1.dsc


Much better indeed ! Some comments below:


I think I have successfully implemented almost all your helpful comments 
in the new version of the package:

http://mentors.debian.net/package/orthanc
http://mentors.debian.net/debian/pool/main/o/orthanc/orthanc_0.2.2-2.dsc

In particular, I now dynamically link against SQLite and JsonCpp, and I 
have enriched the copyright notice. However, the two following 
improvements you have noticed have not been fixed:




* why libcurl4-gnutls-dev | libcurl4-nss-dev | libcurl4-openssl-dev ?
shouldn't that be something simplier like libcurl4-dev ? libcurl-dev ?
libcurl-ssl-dev ?


If I use "libcurl4-dev", "libcurl-dev" or "libcurl-ssl-dev", I obtain 
the following Lintian warning:

http://lintian.debian.org/tags/virtual-package-depends-without-real-package-depends.html

For this reason, I have preferred to keep my explicit enumeration.



* why use oflog and libwrap ? They are not used:
...
dpkg-shlibdeps: warning: package could avoid a useless dependency if
debian/orthanc/usr/bin/orthanc was not linked against libwrap.so.0 (it
uses none of the library's symbols)
dpkg-shlibdeps: warning: package could avoid a useless dependency if
debian/orthanc/usr/bin/orthanc was not linked against liboflog.so.2
(it uses none of the library's symbols)
...


Actually, I think this is a problem in the libdcmtk2-dev package. For 
instance, when I dump the symbols of the "dcmnet.so" file to look for 
the logging facilities, I get the following:


>
jodogne@unstable:~$ nm -D -C /usr/lib/libdcmnet.so | grep log4cplus
 U 
log4cplus::Logger::addAppender(log4cplus::helpers::SharedObjectPtr)

 U log4cplus::Logger::getAppender(OFString const&)
 U 
log4cplus::Logger::removeAppender(log4cplus::helpers::SharedObjectPtr)

 U log4cplus::Logger::removeAppender(OFString const&)
 U log4cplus::Logger::getAllAppenders()
 U log4cplus::Logger::removeAllAppenders()
 U log4cplus::Logger::Logger(log4cplus::Logger const&)
 U log4cplus::Logger::~Logger()
 U log4cplus::Logger::isEnabledFor(int) const
 U log4cplus::Logger::forcedLog(int, OFString const&, 
char const*, int, char const*) const

 U typeinfo for log4cplus::Logger
<

But, the "libdcmnet.so" does not explicitly depend on the "liboflog.so" 
library that implements these symbols:


>
jodogne@unstable:~$ ldd /usr/lib/libdcmnet.so | grep oflog
jodogne@unstable:~$
<

For this reason, I am obliged to link against "oflog", otherwise I get a 
problem at the linking stage. The same problem appears with the "wrap" 
library that is also implicitly required by "libdcmnet.so" :


>
jodogne@unstable:~$ nm -C -D /usr/lib/libdcmnet.so | grep request_init
 U request_init
jodogne@unstable:~$ ldd /usr/lib/libdcmnet.so | grep wrap
jodogne@unstable:~$
<

Note that this problem is also present in the Debian Squeeze and Ubuntu 
11.10 distributions. I don't think I can fix this problem by myself.



Is there any other improvement I can make to the Orthanc package?

Thanks again,
Sébastien-


--
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/506eed8e.6040...@chu.ulg.ac.be



Bug#689041: Re: Orthanc

2012-10-04 Thread Mathieu Malaterre
Sébastien,

On Thu, Oct 4, 2012 at 4:02 PM, Sebastien Jodogne
 wrote:
> Salut Mathieu,
>
> Many thanks for your feedback. I have just uploaded a new version of the
> Orthanc package:
> http://mentors.debian.net/package/orthanc
> http://mentors.debian.net/debian/pool/main/o/orthanc/orthanc_0.2.2-1.dsc

Much better indeed ! Some comments below:

d/rules:
* Why are you override_ing everything in d/rules ? Why not let dh
decide what to do, where to compile ? Also cmake support --parallel
(cf man dh)

d/control:
* why libcurl4-gnutls-dev | libcurl4-nss-dev | libcurl4-openssl-dev ?
shouldn't that be something simplier like libcurl4-dev ? libcurl-dev ?
libcurl-ssl-dev ?
* do not use libpng12-dev, prefer the version less
* no such thing as libdcmtk1-dev (>= 3.5) AFAIK
* I would use debhelper >= 9; to get hardening working in cmake
* Section: misc ? Really ?
* libgtest-dev is listed but configure reveals:
...
-- Could NOT find GTest (missing:  GTEST_LIBRARY GTEST_MAIN_LIBRARY)
...
* why use oflog and libwrap ? They are not used:
...
dpkg-shlibdeps: warning: package could avoid a useless dependency if
debian/orthanc/usr/bin/orthanc was not linked against libwrap.so.0 (it
uses none of the library's symbols)
dpkg-shlibdeps: warning: package could avoid a useless dependency if
debian/orthanc/usr/bin/orthanc was not linked against liboflog.so.2
(it uses none of the library's symbols)
...

d/orthanc.lintian-overrides
* I: orthanc: unused-override spelling-error-in-binary usr/bin/orthanc
negotation negotiation
* what's wrong with sqlite in debian ? Which version exactly do you need ?

ThirdPartyDownloads/jsoncpp-src-0.5.0.tar.gz
* why convenient copy and not debian one: libjsoncpp-dev

d/changelog:
* just keep a single entry, with ref to ITP#. This is fine.

d/copyright:
* where is the copyright/license for jsoncpp, mongoose & sqlite-amalgamation ?
* You need to explicitly state the license you are using for debian/*
files. Something like "Same as above" is fine.

d/compat:
* use 9

d/orthanc.init:
* /var/run is deprecated: http://wiki.debian.org/ReleaseGoals/RunDirectory


Thanks !


--
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/CA+7wUswyMTpV6+rnkUFywe+BokC2tdiBKmfaseKat=ovibq...@mail.gmail.com



Bug#689041: Re: Orthanc

2012-10-04 Thread Sebastien Jodogne

Salut Mathieu,

Many thanks for your feedback. I have just uploaded a new version of the 
Orthanc package:

http://mentors.debian.net/package/orthanc
http://mentors.debian.net/debian/pool/main/o/orthanc/orthanc_0.2.2-1.dsc

The binaries now dynamically link against the versions of DCMTK and 
Boost that are shipped with Debian. Furthermore, no package is 
downloaded from Internet anymore.


Regards,
Sébastien-


--
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/506d96ed.7060...@chu.ulg.ac.be