From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Thompson, David" Subject: Re: [PATCHES] Add Kodi Date: Wed, 25 Nov 2015 11:50:15 -0500 Message-ID: References: <878u5m531a.fsf@izanagi.i-did-not-set--mail-host-address--so-tickle-me> <87h9kagpwd.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:35631) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1dGr-0003Wb-Gw for guix-devel@gnu.org; Wed, 25 Nov 2015 11:50:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1dGq-0007Aq-8X for guix-devel@gnu.org; Wed, 25 Nov 2015 11:50:17 -0500 Received: from mail-yk0-x22e.google.com ([2607:f8b0:4002:c07::22e]:33161) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1dGq-0007Ae-0V for guix-devel@gnu.org; Wed, 25 Nov 2015 11:50:16 -0500 Received: by ykdv3 with SMTP id v3so62114431ykd.0 for ; Wed, 25 Nov 2015 08:50:15 -0800 (PST) In-Reply-To: <87h9kagpwd.fsf@gnu.org> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: =?UTF-8?Q?Ludovic_Court=C3=A8s?= Cc: guix-devel On Wed, Nov 25, 2015 at 8:38 AM, Ludovic Court=C3=A8s wrote: > David Thompson 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 >> 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 >> +Date: Mon, 23 Nov 2015 06:54:36 -0500 >> +Subject: [PATCH] Use STL. > > Maybe just say why this is needed. Sure. I added an explanation. Kodi, and presumably others, assume that TinyXML was built with STL support on. >> + ;; Generate and install pkg-config file. >> + (mkdir-p pkgconfig) >> + (call-with-output-file (string-append pkgconfig "/tinyxm= l.pc") > > It=E2=80=99s OK to do that, but only if there are users that expect it (u= sually > because a major distro has been doing it for some time.) Maybe just add > a something like =E2=80=9Cbecause Kodi expects it=E2=80=9D, or =E2=80=9Cs= imilar to what Debian > does=E2=80=9D. Added a note along those lines. >> From feb1a97e9d3c0e28ee265861bb34c90aa3e06265 Mon Sep 17 00:00:00 2001 >> From: David Thompson >> 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=3Dshared") ; don't use bundle= d ffmpeg > > Would it be possible to delete the bundled ffmpeg in a snippet? Sure, I removed it. I also added a TODO to delete more bundled things that are safe to delete in the future. >> + (let ((cwd (getcwd))) >> + (dynamic-wind >> + (const #t) >> + (lambda () >> + (chdir "tools/depends/native/JsonSchemaBuilder/src") >> + (zero? (system* "sh" "autogen.sh"))) >> + (lambda () >> + (chdir cwd)))))) > > Use =E2=80=98with-directory-excursion=E2=80=99 instead. I should have known that this was already implemented. :) >> + (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 =E2=80=98m= ake > dist=E2=80=99, but I guess they don=E2=80=99t. Bah. > > We should start a =E2=80=9Cmake dist=E2=80=9D campaign; makedist.org appe= ars to be > available=E2=80=A6 ;-) I'm on board. I spent 99% of packaging time trying to get the bootstrapping process to work. I don't think any of the devs realize that they conflate bootstrapping with configuration, or maybe they know and don't see a problem. >> + ("icedtea7" ,icedtea7) > > Is it a build-only dependency (it=E2=80=99s in =E2=80=98native-inputs=E2= =80=99)? Can it be > avoided? Yes, it is a build-only dependency. Unfortunately, it is mandatory to build some part of the application. I added a comment for clarity. Pushed with the suggested changes. Thanks! - Dave