Re: bug#26109: [PATCH 3/7] gnu: Add dcmtk.

Re: bug#26109: [PATCH 3/7] gnu: Add dcmtk.

[John Darrington]

[CC guix-devel@gnu.org]

So we have to make a choice:

1. Package a released program with a known vulnerability; or
2. Package an unreleased git snapshot.

Which is the lesser evil?

J'

On Sat, Mar 18, 2017 at 12:21:40PM -0400, Kei Kebreau wrote:
> John Darrington <john@darrington.wattle.id.au> writes:
> 
> > On Fri, Mar 17, 2017 at 04:42:59PM -0400, Kei Kebreau wrote:
> >      
> >      Judging from the description of the software, it seems like this could
> >      fit in gnu/packages/image.scm.
> >      Also, the linter says that this package vulnerable to
> >      CVE-2015-8979. Supposedly this* upstream patch fixes it. Could you see
> >      if that fix works for this package?
> >      
> >      * https://github.com/commontk/DCMTK/commit/1b6bb76
> >      
> >
> > Unfortunately this patch doesn't go in.  It seems that as well as fixing this
> > vulnerability it also makes some unrelated changes.  Furthermore, it depends
> > on a whole lot of other patches which are not in this release.
> >
> > Do we have a procedure on what to do in cases like this?
> >
> > J'
> 
> I don't know if we have an official procedure, though we could try using
> a later git snapshot with the security patch already integrated.
> Hopefully that provides functionality compatible to that of the stable
> release, though it's at least a five year difference between release times.
> 
> http://git.cmtk.org/?p=dcmtk.git,a=tags

Re: bug#26109: [PATCH 3/7] gnu: Add dcmtk.

[Kei Kebreau]

[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]

John Darrington <jmd@gnu.org> writes:

> [CC guix-devel@gnu.org]
>
> So we have to make a choice:
>
> 1. Package a released program with a known vulnerability; or
> 2. Package an unreleased git snapshot.
>
> Which is the lesser evil?

I choose option two. I'm quite uncomfortable with packaging software
that is known to be vulnerable. To me it seems almost malicious if it
can be avoided.

Other opinions?

>
> J'
>
> On Sat, Mar 18, 2017 at 12:21:40PM -0400, Kei Kebreau wrote:
>> John Darrington <john@darrington.wattle.id.au> writes:
>> 
>> > On Fri, Mar 17, 2017 at 04:42:59PM -0400, Kei Kebreau wrote:
>> >      
>> >      Judging from the description of the software, it seems like this could
>> >      fit in gnu/packages/image.scm.
>> >      Also, the linter says that this package vulnerable to
>> >      CVE-2015-8979. Supposedly this* upstream patch fixes it. Could you see
>> >      if that fix works for this package?
>> >      
>> >      * https://github.com/commontk/DCMTK/commit/1b6bb76
>> >      
>> >
>> > Unfortunately this patch doesn't go in.  It seems that as well as fixing this
>> > vulnerability it also makes some unrelated changes.  Furthermore, it depends
>> > on a whole lot of other patches which are not in this release.
>> >
>> > Do we have a procedure on what to do in cases like this?
>> >
>> > J'
>> 
>> I don't know if we have an official procedure, though we could try using
>> a later git snapshot with the security patch already integrated.
>> Hopefully that provides functionality compatible to that of the stable
>> release, though it's at least a five year difference between release times.
>> 
>> http://git.cmtk.org/?p=dcmtk.git,a=tags

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: bug#26109: [PATCH 3/7] gnu: Add dcmtk.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

On Sat, Mar 18, 2017 at 01:36:31PM -0400, John Darrington wrote:
> [CC guix-devel@gnu.org]
> 
> So we have to make a choice:
> 
> 1. Package a released program with a known vulnerability; or

Although all non-trivial software contains bugs, many of which can be
exploited, we should not add new packages with known exploitable
vulnerabilities.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#26109: [PATCH 3/7] gnu: Add dcmtk.

[Ricardo Wurmus]

Leo Famulari <leo@famulari.name> writes:

>>      Or, we could try building from an arbitrary Git commit.
>> 
>> Yes. That is the other option -  I think it might be a what we'll have to do.
>
> Okay, let us know how it goes.

I tried extracting a patch but it was virtually impossible to make it
apply without introducing more security problems in the attempt.

So I updated the package to use the latest commit from git. 

-- 
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

bug#26108: [PATCH 6/7] gnu: Add mia.

[Ricardo Wurmus]

John Darrington <jmd@gnu.org> writes:

> * gnu/packages/image-processing.scm (mia): New variable.
[…]

> +
> +(define-public mia
> +  (package
> +    (name "mia")
> +    (version "2.4.3")
> +    (source (origin
> +              ;; There are too many issues with the released version which
> +              ;; prevent a clean build and test under Guix.  So until the next
> +              ;; release I suggest we use this git checkout.

That’s okay.  Could you please wrap the “(package …)“ expression in a
let expression where ‘commit’ is bound to
"d91cade30a10f179bba8c8e48b84983a296d2c33" and ‘revision’ to "1".  Then
the version field should be changed to

    (string-append "2.4.3-" revision "." (string-take commit 7))

because this is not exactly version "2.4.3”.

> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://git.code.sf.net/p/mia/mia2")
> +                    (commit
> "d91cade30a10f179bba8c8e48b84983a296d2c33")))

After wrapping the thing in a let binding you can use “(commit commit)”
here.

> +              (sha256
> +               (base32
> +                "0y8ihqbliqy21fph3dm5h6k2nvjbajx4y0mn351x990r9y0767vz"))
> +              (file-name (string-append name "-" version))))

Usually, we append “-checkout” to git clones.

> +    (build-system cmake-build-system)
> +    (arguments `(#:configure-flags `("-DMIA_CREATE_NIPYPE_INTERFACES=0"
> +                                     ,(string-append "-DCMAKE_INSTALL_LIBDIR="
> +                                                     (assoc-ref %outputs "out")
> +                                                     "/lib")
> +                                     ;; According to upstream this is necessary
> +                                     ;; with g++-4.9.x because the std::regex
> +                                     ;; library doesn't work reliably.
> +                                     "-DMIA_USE_BOOST_REGEX=ON"
> +
> "-DCMAKE_CXX_FLAGS=-fpermissive")))

I have a slight preference to adding a line break after “arguments”.

> +    (inputs `(("boost" ,boost)
> +              ("dcmtk" ,dcmtk)
> +              ("doxygen" ,doxygen)

I think this should be a native input.

Please also add a line break after “inputs”.

> +              ("eigen" ,eigen)
> +              ("fftw" ,fftw)
> +              ("fftwf" ,fftwf)
> +              ("gsl" ,gsl)
> +              ("gts" ,gts)
> +              ("hdf5" ,hdf5)
> +              ("itpp" ,itpp)
> +              ("libjpeg" ,libjpeg)
> +              ("libpng" ,libpng)
> +              ("libtiff" ,libtiff)
> +              ("libxml" ,libxml2)
> +              ("libxml++" ,libxml++)
> +              ("maxflow" ,maxflow)
> +              ("niftilib" ,niftilib)
> +              ("nlopt" ,nlopt)
> +              ("openexr" ,openexr)
> +              ("python-lxml" ,python2-lxml)
> +              ("vtk" ,vtk@6.3)))
> +    (native-inputs `(("pkg-config"   ,pkg-config)
> +                     ("python" ,python-2)))

Please add a line break after “native-inputs” (for consistency).  Please
also collapse the spaces after "pkg-config".

> +    (synopsis "Toolkit for gray scale medical image analysis")
> +    (description "MIA provides a combination of command line tools, plug-ins,
> +and libraries that make it possible run image processing tasks interactively
> +in a command shell and to prototype using the shell's scripting
> +language.  It is built around a plug-in structure that makes it easy to add
> +functionality without compromising the original code base and it makes use of a
> +wide variety of external libraries that provide additional functionality.")
> +    (home-page "http://mia.sourceforge.net")
> +    (license license:gpl3+)))

The rest looks good to me (I did not check the license).  Would you like
to push an updated version or would you like me to make the changes
myself?

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

bug#26108: [PATCH 6/7] gnu: Add mia.

[John Darrington]

On Thu, Mar 30, 2017 at 03:15:51PM +0200, Ricardo Wurmus wrote:
> 
> > +    (synopsis "Toolkit for gray scale medical image analysis")
> > +    (description "MIA provides a combination of command line tools, plug-ins,
> > +and libraries that make it possible run image processing tasks interactively
> > +in a command shell and to prototype using the shell's scripting
> > +language.  It is built around a plug-in structure that makes it easy to add
> > +functionality without compromising the original code base and it makes use of a
> > +wide variety of external libraries that provide additional functionality.")
> > +    (home-page "http://mia.sourceforge.net")
> > +    (license license:gpl3+)))
> 
> The rest looks good to me (I did not check the license).  Would you like
> to push an updated version or would you like me to make the changes
> myself?

Feel free to make any changes you think necessary and commit it if appropriate.

J'

bug#26108: [PATCH 6/7] gnu: Add mia.

[Ricardo Wurmus]

John Darrington <jmd@gnu.org> writes:

> On Thu, Mar 30, 2017 at 03:15:51PM +0200, Ricardo Wurmus wrote:
>>
>> > +    (synopsis "Toolkit for gray scale medical image analysis")
>> > +    (description "MIA provides a combination of command line tools, plug-ins,
>> > +and libraries that make it possible run image processing tasks interactively
>> > +in a command shell and to prototype using the shell's scripting
>> > +language.  It is built around a plug-in structure that makes it easy to add
>> > +functionality without compromising the original code base and it makes use of a
>> > +wide variety of external libraries that provide additional functionality.")
>> > +    (home-page "http://mia.sourceforge.net")
>> > +    (license license:gpl3+)))
>>
>> The rest looks good to me (I did not check the license).  Would you like
>> to push an updated version or would you like me to make the changes
>> myself?
>
> Feel free to make any changes you think necessary and commit it if appropriate.

I made some changes, updated to the latest version, and pushed it to
master.

Thank you, John, for the patch, and my apologies for letting it sit here
for so long!  (It was impossible to build it on my laptop, so I had to
arrange for a different machine.)

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

bug#26111: [PATCH 5/7] gnu: Add vtk version 6.3.0

[Ricardo Wurmus]

John Darrington <jmd@gnu.org> writes:

> * gnu/packages/vtk.scm (vtk@6.3): New variable.
> ---
[…]
> +;; Mia 2.4.3 doesn't work against vtk7 so we package vtk 6.3 for it:

Meanwhile Mia 2.4.4 has been released which is compatible with vtk7, so
I’m closing this bug.

I’m currently working on this patch set and have updated dcmtk and mia.
I’ll push them once I’ve confirmed they are working correctly.

Thanks!

-- 
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

bug#26110: [PATCH 7/7] gnu: Move vtk to image-processing.scm

[Ludovic Courtès]

John Darrington <jmd@gnu.org> skribis:

> * gnu/packages/image-processing.scm (vtk, vtk@6.3): New variables.
> * gnu/packages/vtk.scm: Delete file.
> * gnu/local.mk (GNU_SYSTEM_MODULES)[gnu/packages/vtk.scm]: Remove.

Applied, thanks John!

Ludo'.