guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#26941: New font-build-system


From: Ludovic Courtès
Subject: bug#26941: New font-build-system
Date: Tue, 16 May 2017 22:17:00 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)

Hi Arun,

Arun Isaac <address@hidden> skribis:

> Here is a WIP patchset creating a 'font-build-system', and packaging a
> few font packages with it. I just copied the emacs-build-system and made
> necessary changes. Please review and provide feedback.

That’s a great initiative!

> Currently, the font-build-system only installs ttf and otf files to
> /share/fonts/truetype and /share/fonts/opentype respectively. It does
> not install any documentation. But, do we really need to install README
> files, a copy of the license, etc.?

It would be nice to install README, COPYING, and LICENSE if they exist.
It’s okay to not do that as a first step though.

> From 3bf34a68e110d4821b40c43c6388c132d71cb443 Mon Sep 17 00:00:00 2001
> From: Arun Isaac <address@hidden>
> Date: Mon, 15 May 2017 20:08:57 +0530
> Subject: [PATCH 1/5] build: Add 'font-build-system'.

Nitpick: the “build:” prefix in subject line is meant to refer to the
configure/Makefile machinery of Guix itself.  I would use

  build-system: Add 'font-build-system'.

> * Makefile.am (MODULES): Add 'guix/build-system/font.scm' and
>   'guix/build/font-build-system.scm'.
> * guix/build-system/font.scm: New file.
> * guix/build/font-build-system.scm: New file.

[...]

> +(define* (lower name
> +                #:key source inputs native-inputs outputs system target
> +                #:allow-other-keys
> +                #:rest arguments)
> +  "Return a bag for NAME."
> +  (define private-keywords
> +    '(#:target #:inputs #:native-inputs))
> +
> +  (and (not target)                               ;XXX: no cross-compilation
> +       (bag
> +         (name name)
> +         (system system)
> +         (host-inputs `(,@(if source
> +                              `(("source" ,source))
> +                              '())
> +                        ,@inputs
> +
> +                        ;; Keep the standard inputs of 'gnu-build-system'.
> +                        ,@(standard-packages)))
> +         (build-inputs native-inputs)
> +         (outputs outputs)
> +         (build font-build)
> +         (arguments (strip-keyword-arguments private-keywords arguments)))))

I would remove (and (not target) …).  After all, we know that the result
is architecture-independent data, so we can build it natively regardless
of whether TARGET is true.

Also, (standard-packages) is way more than needed (it includes the whole
toolchain, etc.; see build-system/gnu.scm).  Here all we need is tar,
gzip, bzip2, and xz.

Could you updated it accordingly?

> +(define* (install #:key outputs #:allow-other-keys)
> +  "Install the package contents."
> +  (let* ((out (assoc-ref outputs "out"))
> +         (src-dir (getcwd))
> +         (fonts-dir (string-append out "/share/fonts")))

I’d avoid abbreviations in identifiers.  So “source” or
“source-directory”, etc.

One last thing: could you add an entry for ‘font-build-system’ in
guix.texi under “Build Systems”?

Otherwise LGTM!

> From d5a745fddf7b6512e4b5c317c19dc8cd3f9a8efe Mon Sep 17 00:00:00 2001
> From: Arun Isaac <address@hidden>
> Date: Mon, 15 May 2017 20:16:04 +0530
> Subject: [PATCH 2/5] gnu: font-inconsolata: Use 'font-build-system'.
>
> * gnu/packages/fonts.scm (font-inconsolata): Switch to 'font-build-system'.

[...]

> From e0af4ce17f0e99f72926f93b5d5ed99df57cf06a Mon Sep 17 00:00:00 2001
> From: Arun Isaac <address@hidden>
> Date: Mon, 15 May 2017 20:18:08 +0530
> Subject: [PATCH 3/5] gnu: font-ubuntu: Use 'font-build-system'.
>
> * gnu/packages/fonts.scm (font-ubuntu): Switch to 'font-build-system'.

[...]

> From c557da3c5167de0cb5714c5f545b07ac8cd55dae Mon Sep 17 00:00:00 2001
> From: Arun Isaac <address@hidden>
> Date: Mon, 15 May 2017 20:19:33 +0530
> Subject: [PATCH 4/5] gnu: font-dejavu: Use 'font-build-system'.
>
> * gnu/packages/fonts.scm (font-dejavu): Switch to 'font-build-system'.

[...]

> From f302764eb627726548a89438b8c3b6a8ff5988d4 Mon Sep 17 00:00:00 2001
> From: Arun Isaac <address@hidden>
> Date: Mon, 15 May 2017 20:20:26 +0530
> Subject: [PATCH 5/5] gnu: font-bitstream-vera: Use 'font-build-system'.
>
> * gnu/packages/fonts.scm (font-bitstream-vera): Switch to 'font-build-system'.
> ---
>  gnu/packages/fonts.scm | 33 +--------------------------------
>  1 file changed, 1 insertion(+), 32 deletions(-)

Really pleasant to see all these deletions.  :-)

Thank you!

Ludo’.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]