[bug#30801] [PATCH 0/1] Add opencv

[ludo@gnu.org (Ludovic Courtès)]

Hi Björn,

Björn Höfling <bjoern.hoefling@bjoernhoefling.de> skribis:

> The test suite consists of an extra package, weighting 465MB
> compressed. It runs very well. I think the size is worth it. It
> consists of proprietary things (i.e. lena.jpg). As far as I understand,
> that is OK, if it doesn't get in the final src/bin store output. Right?

As a rule of thumb, there should not be non-free stuff in the derivation
graph.

If there’s non-free software, that’s not OK, even if it doesn’t show up
in the output.

If it’s “just” data (pictures) that are non-free, that’s OK per the
FSDG:
<https://www.gnu.org/distros/free-system-distribution-guidelines.html#non-functional-data>.
If we could replace it with a free variant, I think we should clearly
encourage it, but lena.jpg is hardly replaceable in this context (I’d
hope it weren’t around for what it tells about CS, but that’s another
story…).

> CPU-optimization: I hope I have done everything right. Reading the
> article from Guix HPC a second time helped a lot. So now it should be
> compiled with SSE2/NEON being the minimum required instruction set, and
> dispatches to other ISAs where available.

Great.

> Size: Currently I load a bunch of dependencies in and have one
> big  package as output. guix size is 1.1 GiB. I slightly have the
> feeling someone could ask to split it in several outputs. Though having
> one big output was the easiest thing first and I don't know how one
> would handle inter-dependencies between the different outputs.

1.1G is a bit too much IMO, indeed.  :-)

How much is OpenCV itself?  If OpenCV itself is big, it would be nice to
look at what’s taking up space in there with ‘du’.  For instance, if
there are .a files, we might want to not build them and keep only shared
objects.

Splitting in separate outputs may or may not make sense.  If there are
examples or large pieces of HTML doc, introducing a “doc” and/or an
“examples” output may help.  Likewise, if there are binaries that depend
on more stuff that the library itself, introducing a “lib” output might
be a good idea.

Could you take a look?

Nitpick:

> +    (synopsis "Computer Vision Library")

No need to capitalize.

> +    (description "OpenCV (Open Source Computer Vision) is a library aimed at

I’d remove the parenthetic part.

> +real-time computer vision, including several hundred computer
> +vision algorithms.")

Bonus points if you can expound a little bit, without just listing those
algorithms though.  ;-)

Apart from that the patch looks very polished and that’s a pleasure to
review!

Thanks,
Ludo’.