"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.

>>
>>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.

>>+<!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.

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?

Thanks!