Re: [PATCH] glib-or-gtk-build-system: new build-system

[Ludovic Courtès]

Federico Beffa <address@hidden> 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 gtk+.

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’s OK here.  Thoughts?

Some mostly cosmetic comments:

> From dcb7dc451eac21e24e6972fa4c1a4524a7658af0 Mon Sep 17 00:00:00 2001
> From: Federico Beffa <address@hidden>
> 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-system'.

Just “New files.”  Also, make sure to add them to gnu-system.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 ‘store’.

This will have to be adjusted to the new “bags” interface.  Namely, the
part that adds inputs must be moved to a new procedure (called ‘lower’
in the other build systems), and ‘glib-or-gtk-build’ will be 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 “file names” or “directory names” here,
whereas “path” is used for search paths.)

> +(define (gtk-module-dirs inputs)
> +  "Check for the existence of \"libdir/gtk-v.0\" in INPUTS.  Return a list
> +with all found pathes.  If none is found return an empty list."

s/dirs/directories/ and s/pathes/directories/

No need for “If none is found...”, because it’s just 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 ‘schemas-directories’.

> +(define (schemas-dirs inputs)
> +  "Check for the existence of \"datadir/glib-2.0/schemas\" in INPUTS.  Return
> +a list with all found pathes.  If none is found return an empty list."

Rather ‘schemas-directories’, and also “directories” instead of “pathes”.

> +(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 “Build Systems”, ideally
with references (@uref) to the GSettings and GTK+ module documentation?

Could you post an updated patch?

Thanks for all this!

Ludo’.