Re: [PATCH 08/20] Standardize directories and symlinks in images

Alyssa Ross
2025-09-08T08:59:31Z

[-- Attachment #1: Type: text/plain, Size: 15305 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> There are a few directories and symbolic links that a Linux system
> should always have. Even if Spectrum OS itself does not use them,
> third-party dependencies and/or applications might rely on them.
> Create these in scripts/make-erofs.sh rather than separately in
> each VM's build scripts. The creation of /run/lock assumes that
> s6-linux-init is being used, but that assumption is easy to fix later.
> This also enforces that the symlinks and directories were *not* created
> in other places. The app VM build violated this rule, so fix it.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>

This really seems like it's making things substantially more
complicated, especially with the need to remove links so they can later
be recreated again by make-erofs.sh. If we really want to make sure we
don't forget certain directories, we could do that in a much simpler way
by just checking for existence once we've assembled the directory that
will become the image.

> ---
> host/rootfs/Makefile | 15 ++------
> host/rootfs/bin | 1 -
> host/rootfs/lib | 1 -
> host/rootfs/sbin | 1 -
> img/app/Makefile | 8 ++--
> img/app/bin | 1 -
> img/app/default.nix | 101 +++++++++++++++++++++++++++++--------------------
> img/app/sbin | 1 -
> scripts/make-erofs.sh | 34 +++++++++++++++++
> vm/sys/net/Makefile | 8 +---
> vm/sys/net/bin | 1 -
> vm/sys/net/default.nix | 2 +
> vm/sys/net/lib | 1 -
> vm/sys/net/sbin | 1 -
> vm/sys/net/var/run | 1 -
> 15 files changed, 106 insertions(+), 71 deletions(-)
>
> diff --git a/host/rootfs/Makefile b/host/rootfs/Makefile
> index dce78e60bc1a8c18f5f448aaa9aeed2c8a7da04e..6cdbac201257faedb70344bcfd5cf9d4fd25b507 100644
> --- a/host/rootfs/Makefile
> +++ b/host/rootfs/Makefile
> @@ -54,7 +54,6 @@ FILES = \
> etc/s6-linux-init/scripts/rc.init \
> etc/xdg/weston/autolaunch \
> etc/xdg/weston/weston.ini \
> - usr/share/dbus-1/services/org.freedesktop.portal.Documents.service \
> usr/bin/assign-devices \
> usr/bin/create-vm-dependencies \
> usr/bin/run-appimage \
> @@ -63,10 +62,10 @@ FILES = \
> usr/bin/vm-import \
> usr/bin/vm-start \
> usr/bin/vm-stop \
> - usr/bin/xdg-open
> + usr/bin/xdg-open \
> + usr/share/dbus-1/services/org.freedesktop.portal.Documents.service

Would nice for this sort of trivial fix to be a separate patch that
could be immediately applied.

>
> DIRS = \
> - dev \
> etc/s6-linux-init/env \
> etc/s6-linux-init/run-image/configs \
> etc/s6-linux-init/run-image/service/dbus/instance \
> @@ -90,14 +89,11 @@ DIRS = \
> etc/s6-linux-init/run-image/service/xdg-desktop-portal-spectrum-host/instances \
> etc/s6-linux-init/run-image/service/xdg-desktop-portal-spectrum-host/template/data \
> etc/s6-linux-init/run-image/service/xdg-desktop-portal-spectrum-host/template/env \
> - etc/s6-linux-init/run-image/user \
> etc/s6-linux-init/run-image/vm/by-id \
> etc/s6-linux-init/run-image/vm/by-name \
> etc/s6-linux-init/run-image/wait \
> ext \
> - run \
> - proc \
> - sys \
> + root \

I'm not sure what we'd want /root for? Root's home directory is /.

> var
>
> FIFOS = etc/s6-linux-init/run-image/service/s6-svscan-log/fifo
> @@ -105,11 +101,8 @@ FIFOS = etc/s6-linux-init/run-image/service/s6-svscan-log/fifo
> # These are separate because they need to be included, but putting
> # them as make dependencies would confuse make.
> LINKS = \
> - bin \
> etc/s6-linux-init/run-image/opengl-driver \
> - etc/s6-linux-init/run-image/service/vmm/template/run \
> - lib \
> - sbin
> + etc/s6-linux-init/run-image/service/vmm/template/run
>
> BUILD_FILES = build/etc/s6-rc
>
> diff --git a/host/rootfs/bin b/host/rootfs/bin
> deleted file mode 120000
> index 1e881eda3a544eaa86b6019cbe7067ffc58bfafc..0000000000000000000000000000000000000000
> --- a/host/rootfs/bin
> +++ /dev/null
> @@ -1 +0,0 @@
> -usr/bin
> \ No newline at end of file
> diff --git a/host/rootfs/lib b/host/rootfs/lib
> deleted file mode 120000
> index 0d5487ba8608d4d1a7328cf8a4e0242d1988c491..0000000000000000000000000000000000000000
> --- a/host/rootfs/lib
> +++ /dev/null
> @@ -1 +0,0 @@
> -usr/lib
> \ No newline at end of file
> diff --git a/host/rootfs/sbin b/host/rootfs/sbin
> deleted file mode 120000
> index 1e881eda3a544eaa86b6019cbe7067ffc58bfafc..0000000000000000000000000000000000000000
> --- a/host/rootfs/sbin
> +++ /dev/null
> @@ -1 +0,0 @@
> -usr/bin
> \ No newline at end of file
> diff --git a/img/app/Makefile b/img/app/Makefile
> index c6b9a23ce8796582d6e2f5121c30c2269975aa2d..062082e35ba352a8f0520b28379690f5a2ba2ed3 100644
> --- a/img/app/Makefile
> +++ b/img/app/Makefile
> @@ -57,15 +57,15 @@ VM_FILES = \
> etc/wireplumber/wireplumber.conf.d/99_spectrum.conf \
> etc/xdg/xdg-desktop-portal/portals.conf
>
> -VM_DIRS = dev run proc sys tmp var \
> +VM_DIRS = \
> etc/s6-linux-init/run-image/service \
> - etc/s6-linux-init/run-image/user \
> - etc/s6-linux-init/run-image/wait
> + etc/s6-linux-init/run-image/wait \
> + var
> VM_FIFOS = etc/s6-linux-init/run-image/service/s6-linux-init-shutdownd/fifo
>
> # These are separate because they need to be included, but putting
> # them as make dependencies would confuse make.
> -VM_LINKS = bin etc/ssl/certs/ca-certificates.crt sbin
> +VM_LINKS = etc/ssl/certs/ca-certificates.crt
>
> VM_BUILD_FILES = build/etc/s6-rc
>
> diff --git a/img/app/bin b/img/app/bin
> deleted file mode 120000
> index 1e881eda3a544eaa86b6019cbe7067ffc58bfafc..0000000000000000000000000000000000000000
> --- a/img/app/bin
> +++ /dev/null
> @@ -1 +0,0 @@
> -usr/bin
> \ No newline at end of file
> diff --git a/img/app/default.nix b/img/app/default.nix
> index d3eed1f0accdc8968d1ba5bdec74ab597789082f..4daee260afd41de14de06a006b00c2c6db0f5e2a 100644
> --- a/img/app/default.nix
> +++ b/img/app/default.nix
> @@ -12,6 +12,42 @@ pkgsStatic.callPackage (
> }:
>
> let
> + kernelTarget =
> + if stdenvNoCC.hostPlatform.isx86 then
> + # vmlinux.bin is the stripped version of vmlinux.
> + # Confusingly, compressed/vmlinux.bin is the stripped version of
> + # the top-level vmlinux target, while the top-level vmlinux.bin
> + # is the stripped version of compressed/vmlinux. So we use
> + # compressed/vmlinux.bin, since we want a stripped version of
> + # the kernel that *hasn't* been built to be compressed. Weird!
> + "compressed/vmlinux.bin"
> + else
> + stdenvNoCC.hostPlatform.linux-kernel.target;
> +
> + kernel = (linux_latest.override {
> + structuredExtraConfig = with lib.kernel; {
> + DRM_FBDEV_EMULATION = lib.mkForce no;
> + EROFS_FS = yes;
> + FONTS = lib.mkForce unset;
> + FONT_8x8 = lib.mkForce unset;
> + FONT_TER16x32 = lib.mkForce unset;
> + FRAMEBUFFER_CONSOLE = lib.mkForce unset;
> + FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER = lib.mkForce unset;
> + FRAMEBUFFER_CONSOLE_DETECT_PRIMARY = lib.mkForce unset;
> + FRAMEBUFFER_CONSOLE_ROTATION = lib.mkForce unset;
> + RC_CORE = lib.mkForce unset;
> + VIRTIO = yes;
> + VIRTIO_BLK = yes;
> + VIRTIO_CONSOLE = yes;
> + VIRTIO_PCI = yes;
> + VT = no;
> + };
> + }).overrideAttrs ({ installFlags ? [], ... }: {
> + installFlags = installFlags ++ [
> + "KBUILD_IMAGE=$(boot)/${kernelTarget}"
> + ];
> + });
> +
> appimageFhsenv = (buildFHSEnv (appimageTools.defaultFhsEnvArgs // {
> name = "vm-fhs-env";
> targetPkgs = pkgs: appimageTools.defaultFhsEnvArgs.targetPkgs pkgs ++ [
> @@ -53,50 +89,33 @@ let
> pkgs.wireplumber
> ];
> })).fhsenv;
> -in
>
> -let

Another cleanup that would be really nice to have separately, so I don't
have to try to review two things at once.

> packagesSysroot = runCommand "packages-sysroot" {} ''
> - mkdir -p $out/etc/ssl/certs
> - ln -s ${appimageFhsenv}/{lib64,usr} ${kernel}/lib $out
> - ln -s ${cacert}/etc/ssl/certs/* $out/etc/ssl/certs
> + set -eu
> + mkdir -p -- "$out/etc/ssl/certs" "$out/usr/bin"
> + # ../../scripts/make-erofs.sh will re-create these
> + rm -f -- "$out/usr/lib64" "$out/usr/lib"
> + source_dir=${lib.escapeShellArg appimageFhsenv}/usr
> + for i in "$source_dir"/*; do
> + subdir=''${i##*/}
> + case $subdir in
> + (bin|include|lib|lib64|libexec|sbin|share) :;;
> + (*) printf 'Bad subdirectory %s\n' "$subdir" >&2; exit 1;;
> + esac
> + done
> + if ! [ -h "$source_dir/lib" ]; then echo "FHSenv didn't make lib a symlink" >&2; exit 1; fi
> + ln -s -- "$source_dir/include" "$source_dir/libexec" "$source_dir/share" "$out/usr"
> + cp -RT -- "$source_dir/lib64" "$out/usr/lib"
> + # Do this first so that the subsequent call to cp (without -T)
> + # will create new entries in the existing bin directory.
> + cp -RT -- "$source_dir/sbin" "$out/usr/bin"
> + # with -T cp tries to delete the whole target directory first
> + cp -R -- "$source_dir/bin" "$out/usr"
> + # so that ln can make the symlink
> + chmod -- 0755 "$out/usr/lib"
> + ln -s -- ${lib.escapeShellArg kernel}/lib/modules "$out/usr/lib/"
> + ln -s -- ${lib.escapeShellArg cacert}/etc/ssl/certs/* "$out/etc/ssl/certs"
> '';
> -
> - kernelTarget =
> - if stdenvNoCC.hostPlatform.isx86 then
> - # vmlinux.bin is the stripped version of vmlinux.
> - # Confusingly, compressed/vmlinux.bin is the stripped version of
> - # the top-level vmlinux target, while the top-level vmlinux.bin
> - # is the stripped version of compressed/vmlinux. So we use
> - # compressed/vmlinux.bin, since we want a stripped version of
> - # the kernel that *hasn't* been built to be compressed. Weird!
> - "compressed/vmlinux.bin"
> - else
> - stdenvNoCC.hostPlatform.linux-kernel.target;
> -
> - kernel = (linux_latest.override {
> - structuredExtraConfig = with lib.kernel; {
> - DRM_FBDEV_EMULATION = lib.mkForce no;
> - EROFS_FS = yes;
> - FONTS = lib.mkForce unset;
> - FONT_8x8 = lib.mkForce unset;
> - FONT_TER16x32 = lib.mkForce unset;
> - FRAMEBUFFER_CONSOLE = lib.mkForce unset;
> - FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER = lib.mkForce unset;
> - FRAMEBUFFER_CONSOLE_DETECT_PRIMARY = lib.mkForce unset;
> - FRAMEBUFFER_CONSOLE_ROTATION = lib.mkForce unset;
> - RC_CORE = lib.mkForce unset;
> - VIRTIO = yes;
> - VIRTIO_BLK = yes;
> - VIRTIO_CONSOLE = yes;
> - VIRTIO_PCI = yes;
> - VT = no;
> - };
> - }).overrideAttrs ({ installFlags ? [], ... }: {
> - installFlags = installFlags ++ [
> - "KBUILD_IMAGE=$(boot)/${kernelTarget}"
> - ];
> - });
> in
>
> stdenvNoCC.mkDerivation {
> diff --git a/img/app/sbin b/img/app/sbin
> deleted file mode 120000
> index 1e881eda3a544eaa86b6019cbe7067ffc58bfafc..0000000000000000000000000000000000000000
> --- a/img/app/sbin
> +++ /dev/null
> @@ -1 +0,0 @@
> -usr/bin
> \ No newline at end of file
> diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
> index d566a4ac7b30f55338fe9b8b6a94702686f6ddd1..5196394d405310971659b0dbc0c91cfcaaaf9118 100755
> --- a/scripts/make-erofs.sh
> +++ b/scripts/make-erofs.sh
> @@ -115,5 +115,39 @@ find "$root" \
> find "$root/etc" "$root/var" ! -type l -execdir chmod u+w,go-w,ugo+rX -- '{}' +
> chmod 0755 "$root"
>
> +# Fix permissions on / so that the subsequent commands work
> +chmod 0755 "$root"
> +
> +# Create the basic mount points for pseudo-filesystems and tmpfs filesystems.
> +# These should always be mounted over, so use 0400 permissions for them.
> +# 0000 would be better, but it breaks mkfs.erofs as it tries to open the
> +# directories for reading.
> +mkdir -m 0400 "$root/dev" "$root/proc" "$root/run" "$root/sys" "$root/tmp"
> +
> +# Cause s6-linux-init to create /run/lock and /run/user
> +# with the correct mode (0755) and create /home,
> +# /var/cache, /var/log, and /var/spool directly.
> +mkdir -m 0755 \
> + "$root/etc/s6-linux-init/run-image/lock" \
> + "$root/etc/s6-linux-init/run-image/user" \
> + "$root/home" \
> + "$root/var/cache" \
> + "$root/var/log" \
> + "$root/var/spool"
> +
> +# Create symbolic links that are always expected to exist.
> +chmod 0755 "$root/usr"
> +ln -s ../proc/self/mounts "$root/etc/mtab"
> +ln -s ../run "$root/var/run"
> +ln -s ../run/lock "$root/var/lock"
> +ln -s ../tmp "$root/var/tmp"
> +ln -s bin "$root/usr/sbin"
> +ln -s lib "$root/usr/lib64"

This doesn't seem right as a generic thing. Nix-built binaries won't
ever need this. It's only in img/app for AppImage etc. compatibility.
Not relevant to other images.

> +ln -s usr/bin "$root/bin"
> +ln -s usr/bin "$root/sbin"
> +ln -s usr/lib "$root/lib"
> +ln -s usr/lib "$root/lib64"
> +chmod 0555 "$root/usr"
> +
> # Make the erofs image.
> mkfs.erofs -x-1 -b4096 --all-root "$@" "$root"
> diff --git a/vm/sys/net/Makefile b/vm/sys/net/Makefile
> index e6819400b2079e3eaa9d24737b2fc4b816a592c8..a8ad03862165a69f3f7dd3e49f668cfa887d817f 100644
> --- a/vm/sys/net/Makefile
> +++ b/vm/sys/net/Makefile
> @@ -39,11 +39,7 @@ VM_FILES = \
> etc/s6-linux-init/run-image/service/getty-hvc0/run \
> etc/s6-linux-init/scripts/rc.init \
> etc/sysctl.conf
> -VM_DIRS = dev etc/s6-linux-init/env run proc sys var/lib/connman
> -
> -# These are separate because they need to be included, but putting
> -# them as make dependencies would confuse make.
> -VM_LINKS = bin lib sbin var/run
> +VM_DIRS = etc/s6-linux-init/env var/lib/connman
>
> VM_BUILD_FILES = build/etc/s6-rc
>
> @@ -53,7 +49,7 @@ build/empty:
> build/rootfs.erofs: ../../../scripts/make-erofs.sh $(PACKAGES_FILE) $(VM_FILES) $(VM_BUILD_FILES) build/empty
> ( \
> cat $(PACKAGES_FILE) ;\
> - for file in $(VM_FILES) $(VM_LINKS); do printf '%s\n%s\n' $$file $$file; done ;\
> + for file in $(VM_FILES); do printf '%s\n%s\n' $$file $$file; done ;\
> for file in $(VM_BUILD_FILES); do printf '%s\n%s\n' $$file $${file#build/}; done ;\
> printf 'build/empty\n%s\n' $(VM_DIRS) ;\
> ) | ../../../scripts/make-erofs.sh $@
> diff --git a/vm/sys/net/bin b/vm/sys/net/bin
> deleted file mode 120000
> index 1e881eda3a544eaa86b6019cbe7067ffc58bfafc..0000000000000000000000000000000000000000
> --- a/vm/sys/net/bin
> +++ /dev/null
> @@ -1 +0,0 @@
> -usr/bin
> \ No newline at end of file
> diff --git a/vm/sys/net/default.nix b/vm/sys/net/default.nix
> index b5873ebe1e80dd88c1ba997f7ebd3ee7369bb40f..a2c635e8ff09ab2b0ae4694344f3810c1b9739a5 100644
> --- a/vm/sys/net/default.nix
> +++ b/vm/sys/net/default.nix
> @@ -51,6 +51,8 @@ let
> for pkg in ${lib.escapeShellArgs usrPackages}; do
> lndir -ignorelinks -silent "$pkg" "$out/usr"
> done
> + [ -h "$out/usr/sbin" ]
> + rm -f -- "$out/usr/sbin"
> '';
>
> nixosAllHardware = nixos ({ modulesPath, ... }: {

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]