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

Re: Bug#689041: RFS: orthanc/0.2.1-3 [ITP] -- Lightweight, RESTful DICOM server for healthcare and medical research



Sébastien

On Mon, Oct 8, 2012 at 6:19 PM, Sebastien Jodogne
<s.jodogne@chu.ulg.ac.be> wrote:
>> Well, I wasn't really happy about the following (but I uploaded anyway):
>>
>> $ cat d/rules
>> ...
>> override_dh_auto_install:
>>         cp $(CURDIR)/Build/Orthanc
>> $(CURDIR)/debian/orthanc/usr/bin/orthanc
>>         gzip -c -9 $(CURDIR)/NEWS >
>> $(CURDIR)/debian/orthanc/usr/share/doc/orthanc/changelog.gz
>>
>> What were you trying to do ?
>
>
> The "cp" command is used to convert the name of the binary to lowercase. The
> camel case naming in the upstream package is a convention I use for Windows
> builds.

debian only enforce lower case for package name. This is unneeded
IMHO. Some users might even be confused that suddenly debian package
uses orthanc, while Orthanc is used on *NIX boxes...

> The "gzip" command is used to install the upstream changelog.
>
> There are certainly better ways of doing so, unfortunately this is my first
> Debian packaging attempt :)

See Andreas comment.

>
>> Also I'll remove -DDEBIAN_HARDENING=ON on the next upload, I just do
>> not understand why it was used in the case of an explicit debian
>> packaging...
>
>
> Indeed, this flag is not required on Debian sid. I have implemented this
> flag to allow backporting to squeeze, whose debhelper version is apparently
> not able to natively cope with hardening.

Technically there is nothing wrong, some duplicate work that is all.
What I fear is rather for advanced user:
http://www.debian.org/doc/debian-policy/ch-source.html#s-debianrules-options
Technically you should be able to control such compilation flags
outside of the cmake build system, eg if the user wants something
different from the default hardening.

>> Not running the test suite is not a good idea.
>
>
> I have been obliged to do so, since it seems that the "libgtest-dev" package
> does not come with the required shared library. I could however easily
> modify the CMake files to statically link against gtest (only for the test
> suite of course).
>

When all else fails, read the instructions:
$ cat /usr/share/doc/libgtest-dev/README.Debian


>> Finally, I would not use -DCMAKE_BUILD_TYPE=Debug, maybe something
>> like MinSizeRel would be slightly better (-O2)
>
>
> I have indeed forgotten to replace "Debug" with "Release".

Release is fine too.

> I will submit the last three modifications tomorrow morning. I will warn you
> when I will have done so.
>
> I will also try and close the ITP and the RFS bugs tomorrow.
>
> ...And thank for uploading the package!

Thanks for your contribution to the DICOM community !

As a side note, where is the implementation coming from, is this JSON
binding describe anywhere in the DICOM standard ?

Thanks.
-- 
Mathieu


Reply to: