Re: [PATCH v2] img/app: add dejavu_fonts pkg and fontconfig file

["Dom (shymega) Rodriguez" <shymega@shymega.org.uk>]

On 11.02.2025 18:27, Alyssa Ross wrote:
>"Dom (shymega) Rodriguez" <shymega@shymega.org.uk> writes:
>
>> On 08.02.2025 14:36, Samy Lahfa wrote:
>>>Signed-off-by: Samy Lahfa <samy+spectrum@lahfa.xyz>
>>>---
>>>This fixes the foot terminal app-vm by adding dejavu_fonts package to the
>>>nix store as well as adding the fontconfig file, so it knows which
>>>default font to use.
>>>
>>> img/app/Makefile             |  2 ++
>>> img/app/default.nix          |  3 ++-
>>> img/app/etc/fonts/fonts.conf | 21 +++++++++++++++++++++
>>> 3 files changed, 25 insertions(+), 1 deletion(-)
>>> create mode 100644 img/app/etc/fonts/fonts.conf
>>>
>>>diff --git a/img/app/Makefile b/img/app/Makefile
>>>index 7887aa8..eeb17fa 100644
>>>--- a/img/app/Makefile
>>>+++ b/img/app/Makefile
>>>@@ -30,6 +30,7 @@ $(imgdir)/appvm/blk/root.img: ../../scripts/make-gpt.sh ../../scripts/sfdisk-fie
>>>
>>> VM_FILES = \
>>> 	etc/dbus-1/session.conf \
>>>+	etc/fonts/fonts.conf \
>>> 	etc/fstab \
>>> 	etc/init \
>>> 	etc/mdev.conf \
>>>@@ -52,6 +53,7 @@ VM_FILES = \
>>> 	etc/s6-linux-init/scripts/rc.shutdown \
>>> 	etc/s6-linux-init/scripts/rc.shutdown.final \
>>> 	etc/xdg/xdg-desktop-portal/portals.conf
>>>+
>>> VM_DIRS = dev run proc sys tmp \
>>> 	etc/s6-linux-init/run-image/service
>>> VM_FIFOS = etc/s6-linux-init/run-image/service/s6-linux-init-shutdownd/fifo
>>>diff --git a/img/app/default.nix b/img/app/default.nix
>>>index 6537fb2..5b58a5f 100644
>>>--- a/img/app/default.nix
>>>+++ b/img/app/default.nix
>>>@@ -8,7 +8,7 @@ pkgsStatic.callPackage (
>>> { lib, stdenvNoCC, runCommand, writeClosure
>>> , erofs-utils, jq, s6-rc, util-linux
>>> , busybox, cacert, execline, kmod, linux_latest, mdevd, s6, s6-linux-init
>>>-, xdg-desktop-portal-spectrum
>>>+, xdg-desktop-portal-spectrum, dejavu_fonts,
>>> }:
>>>
>>> let
>>>@@ -37,6 +37,7 @@ let
>>>       s6-rc
>>>       terminfo
>>>       xdg-desktop-portal-spectrum
>>>+      dejavu_fonts
>>>
>>>       # Some packages can't (currently?) be built statically.
>>
>> Alyssa, are we aiming for a alphabetically-sorted list of packages in
>> this file, or is it good as-is?
>
>Basically any time a list is alphabetically sorted, it should be kept
>alphabetically sorted.

Understood. Do we have this in a 'contributing guidelines' document?

>
>>>
>>>diff --git a/img/app/etc/fonts/fonts.conf b/img/app/etc/fonts/fonts.conf
>>>new file mode 100644
>>>index 0000000..0dcde54
>>>--- /dev/null
>>>+++ b/img/app/etc/fonts/fonts.conf
>>>@@ -0,0 +1,21 @@
>>>+<?xml version="1.0" encoding="UTF-8"?>
>>>+<!-- SPDX-License-Identifier: CC0-1.0 -->
>>>+<!-- SPDX-FileCopyrightText: 2021 Alyssa Ross <hi@alyssa.is> -->
>>
>> Alyssa - is CC0-1.0 OK here, or would you prefer something else?
>
>Yeah — this is actually a copy of an existing file in the tree,
>host/rootfs/etc/fonts/fonts.conf, so keeping the copyright and license
>information the same was the right thing to do.

ack.

>
>>>+<!DOCTYPE fontconfig SYSTEM "urn:fontconfig:fonts.dtd">
>>>+<fontconfig>
>>>+  <alias binding="same">
>>>+    <family>monospace</family>
>>>+    <prefer>
>>>+      <family>DejaVu Sans Mono</family>
>>>+    </prefer>
>>>+  </alias>
>>>+
>>>+  <alias binding="same">
>>>+    <family>sans-serif</family>
>>>+    <prefer>
>>>+      <family>DejaVu Sans</family>
>>>+    </prefer>
>>>+  </alias>
>>>+
>>>+  <dir>/usr/share/fonts</dir>
>>>+</fontconfig>
>>>--
>>>2.47.2
>>>
>>
>> The only thing I'd like to see would be the description in the commit
>> message, not in the email annotation - this would need to be a v3 patch,
>> but when I apply this patch, I can't see the description in the tree, so
>> it's hard to understand the reason for the commit.
>>
>> Other than that, thank you for your contribution - just need to fix the
>> commit, and then check with Alyssa about list sorting and the license.
>
>Yeah, to clarify this: when "git am" applies a patch, everything after
>the "---" is discarded, so stuff that's intended for the commit message
>should go before that.  It's sometimes useful to put extra information
>that doesn't need to make it into git after the "---", but most of the
>explanation of the change should go before.

Sorry, should have clarified on my part. I was in 'technical mode'.

>Samy, both of the remaining issues here (the list sorting, and the patch
>description) are pretty trivial — you can send a v3 if you'd like, but
>I'm also just happy to just apply this patch and fix it up as I do so.
>What would you prefer?

No objection from me on fixing up the patch on your side, Alyssa. I
certainly don't want to make the barrier to entry for contributing
higher.

Best wishes,
--
Dom Rodriguez