[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCHES] Add Kodi
From: |
Ludovic Courtès |
Subject: |
Re: [PATCHES] Add Kodi |
Date: |
Wed, 25 Nov 2015 14:38:42 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
David Thompson <address@hidden> skribis:
> This one was a real hairball, but here is a patch set that adds the Kodi
> media center! I hope that I've explained all the craziness relatively
> well in the comments.
Wow, quite an achievement!
> From d708d0c36e202bbad7255c3b8a55ca0afdd18cb3 Mon Sep 17 00:00:00 2001
> From: David Thompson <address@hidden>
> Date: Tue, 24 Nov 2015 13:35:44 -0500
> Subject: [PATCH 1/3] gnu: Add tinyxml.
>
> * gnu/packages/xml.scm (tinyxml): New variable.
> * gnu/packages/patches/tinyxml-use-stl.patch: New file.
> * gnu-system.am (dist_patch_DATA): Add it.
[...]
> +From a53b6ee4519a7657164610ac14a82c57b1273bf6 Mon Sep 17 00:00:00 2001
> +From: David Thompson <address@hidden>
> +Date: Mon, 23 Nov 2015 06:54:36 -0500
> +Subject: [PATCH] Use STL.
Maybe just say why this is needed.
> + ;; Generate and install pkg-config file.
> + (mkdir-p pkgconfig)
> + (call-with-output-file (string-append pkgconfig "/tinyxml.pc")
It’s OK to do that, but only if there are users that expect it (usually
because a major distro has been doing it for some time.) Maybe just add
a something like “because Kodi expects it”, or “similar to what Debian
does”.
Otherwise LGTM.
> From 11b1b337dbd3b245c314b1d22eaeb1331a34e729 Mon Sep 17 00:00:00 2001
> From: David Thompson <address@hidden>
> Date: Tue, 24 Nov 2015 13:37:34 -0500
> Subject: [PATCH 2/3] gnu: Add jasper.
>
> * gnu/packages/image.scm (jasper): New variable.
LGTM.
> From feb1a97e9d3c0e28ee265861bb34c90aa3e06265 Mon Sep 17 00:00:00 2001
> From: David Thompson <address@hidden>
> Date: Mon, 16 Nov 2015 22:31:26 -0500
> Subject: [PATCH 3/3] gnu: Add kodi.
+ commit log and copyright header.
> + '(#:configure-flags '("--with-ffmpeg=shared") ; don't use bundled ffmpeg
Would it be possible to delete the bundled ffmpeg in a snippet?
> + (let ((cwd (getcwd)))
> + (dynamic-wind
> + (const #t)
> + (lambda ()
> + (chdir "tools/depends/native/JsonSchemaBuilder/src")
> + (zero? (system* "sh" "autogen.sh")))
> + (lambda ()
> + (chdir cwd))))))
Use ‘with-directory-excursion’ instead.
> + (native-inputs
> + `(("autoconf" ,autoconf)
> + ("automake" ,automake)
> + ("cmake" ,cmake)
> + ("doxygen" ,doxygen)
> + ("gawk" ,gawk)
> + ("gettext" ,gnu-gettext)
We could remove autoconf/automake/libtool/gettext if they used ‘make
dist’, but I guess they don’t. Bah.
We should start a “make dist” campaign; makedist.org appears to be
available… ;-)
> + ("icedtea7" ,icedtea7)
Is it a build-only dependency (it’s in ‘native-inputs’)? Can it be
avoided?
Otherwise LGTM.
Thank you!
Ludo’.