From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] glib-or-gtk-build-system: new build-system Date: Mon, 06 Oct 2014 23:32:41 +0200 Message-ID: <87y4ssubnq.fsf@gnu.org> References: 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]:33851) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbFtZ-0007Bp-J4 for guix-devel@gnu.org; Mon, 06 Oct 2014 17:32:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XbFtV-000879-4J for guix-devel@gnu.org; Mon, 06 Oct 2014 17:32:41 -0400 Received: from hera.aquilenet.fr ([2a01:474::1]:49157) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbFtU-000873-MH for guix-devel@gnu.org; Mon, 06 Oct 2014 17:32:37 -0400 In-Reply-To: (Federico Beffa's message of "Mon, 6 Oct 2014 18:43:04 +0200") 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: Federico Beffa Cc: guix-devel@gnu.org Federico Beffa skribis: > Please find attached a new build system for applications making use of > glib or gtk+. > It uses wrappers as previously discussed on this list. > > I've successfully tested it with three packages: emacs, libcanberra and g= tk+. Excellent! Would be nice to check Evince and EOG, which were known to have this kind of problem. > Currently it is implemented as a separate build-system inheriting from > the gnu-build-system. Essentially it adds 2 phases to the latter. > IMO, it would make sense to make those 2 phases part of the > gnu-build-system and enable them with the help of flags. Great. The strategy looks good to me. Mark had concerns about the wrapper approach in general, but IMO it=E2=80=99s OK here. Thoughts? Some mostly cosmetic comments: > From dcb7dc451eac21e24e6972fa4c1a4524a7658af0 Mon Sep 17 00:00:00 2001 > From: Federico Beffa > Date: Mon, 6 Oct 2014 15:49:29 +0200 > Subject: [PATCH] glib-or-gtk-build-system: new build-system > > * guix/build-system/glib-or-gtk.scm, guix/build/glib-or-gtk-build-system.= scm: > Add initial version of a new build-system called 'glib-or-gtk-build-sys= tem'. Just =E2=80=9CNew files.=E2=80=9D Also, make sure to add them to gnu-syste= m.am and to mention it in the log. > +(define* (glib-or-gtk-build store name source inputs > + #:key (guile #f) > + (outputs '("out")) > + (search-paths '()) > + (configure-flags ''()) > + (make-flags ''()) > + (glib (default-glib)) Please align parameters with =E2=80=98store=E2=80=99. This will have to be adjusted to the new =E2=80=9Cbags=E2=80=9D interface. = Namely, the part that adds inputs must be moved to a new procedure (called =E2=80=98low= er=E2=80=99 in the other build systems), and =E2=80=98glib-or-gtk-build=E2=80=99 will b= e simplified accordingly. It should be simple to see what needs to be changed by looking at the other build systems, but let me know if you have any questions. > +(define (rel-path-exists? path rel-path) > + (directory-exists? (string-append path rel-path))) Rather (subdirectory-exists? parent sub-directory). (By convention GNU software uses =E2=80=9Cfile names=E2=80=9D or =E2=80=9Cd= irectory names=E2=80=9D here, whereas =E2=80=9Cpath=E2=80=9D is used for search paths.) > +(define (gtk-module-dirs inputs) > + "Check for the existence of \"libdir/gtk-v.0\" in INPUTS. Return a li= st > +with all found pathes. If none is found return an empty list." s/dirs/directories/ and s/pathes/directories/ No need for =E2=80=9CIf none is found...=E2=80=9D, because it=E2=80=99s jus= t a special case. > +(define (glib-schemas input prev) > + (let* ((in (match input > + ((_ . dir) dir) > + (_ ""))) > + (datadir (string-append in "/share"))) > + (if (rel-path-exists? datadir "/glib-2.0/schemas") > + (cons datadir prev) > + prev))) I think this should be moved into =E2=80=98schemas-directories=E2=80=99. > +(define (schemas-dirs inputs) > + "Check for the existence of \"datadir/glib-2.0/schemas\" in INPUTS. R= eturn > +a list with all found pathes. If none is found return an empty list." Rather =E2=80=98schemas-directories=E2=80=99, and also =E2=80=9Cdirectories= =E2=80=9D instead of =E2=80=9Cpathes=E2=80=9D. > +(define* (glib-or-gtk-build #:key inputs (phases %standard-phases) > + #:allow-other-keys #:rest args) Please align #:allow-other-keys with #:key. Lastly, could add an entry in guix.texi under =E2=80=9CBuild Systems=E2=80= =9D, ideally with references (@uref) to the GSettings and GTK+ module documentation? Could you post an updated patch? Thanks for all this! Ludo=E2=80=99.