From f971a46f694106d0d7dd9e89a34d74c7d5923e65 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Wed, 28 Aug 2024 12:39:42 +0200 Subject: [PATCH] cmd: rename SNAP_MOUNT_DIR to STATIC_SNAP_MOUNT_DIR Given that most tools now detect the snap mount directory dynamically, picking either /var/lib/snapd/snap or /snap, depending on what is actually being used, it is the right time to rename the remaining option to the more appropriate name of static snap mount directory. The name differs because the value is baked into the files shipped with the distribution-specific packaging. This includes systemd units, environment generators and the management script. We still need this value and it is not going away anytime soon, but it is not the only value and so I think that separating SNAP_MOUNT_DIR from STATIC_SNAP_MOUNT_DIR is well, apt. Signed-off-by: Zygmunt Krynicki --- cmd/Makefile.am | 4 +-- cmd/configure.ac | 13 +++++---- cmd/snap-mgmt/snap-mgmt-selinux.sh.in | 10 +++---- cmd/snap-mgmt/snap-mgmt.sh.in | 40 +++++++++++++-------------- cmd/snap/cmd_snap_op.go | 2 +- cmd/snapd-env-generator/main.c | 2 +- cmd/snapd-generator/main.c | 4 +-- 7 files changed, 38 insertions(+), 37 deletions(-) diff --git a/cmd/Makefile.am b/cmd/Makefile.am index b316a8733ca..afd39957738 100644 --- a/cmd/Makefile.am +++ b/cmd/Makefile.am @@ -418,7 +418,7 @@ snap-mgmt/$(am__dirstamp): touch $@ snap-mgmt/snap-mgmt: snap-mgmt/snap-mgmt.sh.in Makefile snap-mgmt/$(am__dirstamp) - sed -e 's,[@]SNAP_MOUNT_DIR[@],$(SNAP_MOUNT_DIR),' <$< >$@ + sed -e 's,[@]STATIC_SNAP_MOUNT_DIR[@],$(STATIC_SNAP_MOUNT_DIR),' <$< >$@ if SELINUX ## @@ -429,7 +429,7 @@ libexec_SCRIPTS += snap-mgmt/snap-mgmt-selinux CLEANFILES += snap-mgmt/$(am__dirstamp) snap-mgmt/snap-mgmt-selinux snap-mgmt/snap-mgmt-selinux: snap-mgmt/snap-mgmt-selinux.sh.in Makefile snap-mgmt/$(am__dirstamp) - sed -e 's,[@]SNAP_MOUNT_DIR[@],$(SNAP_MOUNT_DIR),' <$< >$@ + sed -e 's,[@]STATIC_SNAP_MOUNT_DIR[@],$(STATIC_SNAP_MOUNT_DIR),' <$< >$@ endif ## diff --git a/cmd/configure.ac b/cmd/configure.ac index 97efc19c840..f70d9c8d5fb 100644 --- a/cmd/configure.ac +++ b/cmd/configure.ac @@ -153,14 +153,15 @@ AS_IF([test "x$enable_merged_usr" = "xyes"], [ AC_DEFINE([MERGED_USR], [1], [Support for merged /usr directory])]) -SNAP_MOUNT_DIR="/snap" +# Keep the --with-snap-mount-dir= option name the same for ease of transition. +STATIC_SNAP_MOUNT_DIR="/snap" AC_ARG_WITH([snap-mount-dir], - AS_HELP_STRING([--with-snap-mount-dir=DIR], [Use an alternate snap mount directory]), - [SNAP_MOUNT_DIR="$withval"]) -AC_SUBST(SNAP_MOUNT_DIR) -AC_DEFINE_UNQUOTED([SNAP_MOUNT_DIR], "${SNAP_MOUNT_DIR}", [Location of the snap mount points]) + AS_HELP_STRING([--with-snap-mount-dir=DIR], [Use an alternate static snap mount directory]), + [STATIC_SNAP_MOUNT_DIR="$withval"]) +AC_SUBST(STATIC_SNAP_MOUNT_DIR) +AC_DEFINE_UNQUOTED([STATIC_SNAP_MOUNT_DIR], "${STATIC_SNAP_MOUNT_DIR}", [Static location of the snap mount points]) -SNAP_MOUNT_DIR_SYSTEMD_UNIT="$(systemd-escape -p "$SNAP_MOUNT_DIR")" +SNAP_MOUNT_DIR_SYSTEMD_UNIT="$(systemd-escape -p "$STATIC_SNAP_MOUNT_DIR")" AC_SUBST([SNAP_MOUNT_DIR_SYSTEMD_UNIT]) AC_DEFINE_UNQUOTED([SNAP_MOUNT_DIR_SYSTEMD_UNIT], "${SNAP_MOUNT_DIR_SYSTEMD_UNIT}", [Systemd unit name for snap mount points location]) diff --git a/cmd/snap-mgmt/snap-mgmt-selinux.sh.in b/cmd/snap-mgmt/snap-mgmt-selinux.sh.in index f276a7d3c75..03e352ed44f 100644 --- a/cmd/snap-mgmt/snap-mgmt-selinux.sh.in +++ b/cmd/snap-mgmt/snap-mgmt-selinux.sh.in @@ -3,7 +3,7 @@ set -e set +x -SNAP_MOUNT_DIR="@SNAP_MOUNT_DIR@" +STATIC_SNAP_MOUNT_DIR="@STATIC_SNAP_MOUNT_DIR@" show_help() { exec cat <<'EOF' @@ -12,13 +12,13 @@ Usage: snap-mgmt-selinux.sh [OPTIONS] A helper script to manage SELinux contexts used by snapd Arguments: - --snap-mount-dir= Provide a path to be used as $SNAP_MOUNT_DIR + --snap-mount-dir= Provide a path to be used as $STATIC_SNAP_MOUNT_DIR --patch-selinux-mount-context= Add SELinux context to mount units --remove-selinux-mount-context= Remove SELinux context from mount units EOF } -SNAP_UNIT_PREFIX="$(systemd-escape -p ${SNAP_MOUNT_DIR})" +SNAP_UNIT_PREFIX="$(systemd-escape -p ${STATIC_SNAP_MOUNT_DIR})" patch_selinux_mount_context() { if ! command -v selinuxenabled > /dev/null; then @@ -92,8 +92,8 @@ while [ -n "$1" ]; do exit ;; --snap-mount-dir=*) - SNAP_MOUNT_DIR=${1#*=} - SNAP_UNIT_PREFIX=$(systemd-escape -p "$SNAP_MOUNT_DIR") + STATIC_SNAP_MOUNT_DIR=${1#*=} + SNAP_UNIT_PREFIX=$(systemd-escape -p "$STATIC_SNAP_MOUNT_DIR") shift ;; --patch-selinux-mount-context=*) diff --git a/cmd/snap-mgmt/snap-mgmt.sh.in b/cmd/snap-mgmt/snap-mgmt.sh.in index 3467670b181..482e0a22343 100755 --- a/cmd/snap-mgmt/snap-mgmt.sh.in +++ b/cmd/snap-mgmt/snap-mgmt.sh.in @@ -8,7 +8,7 @@ set -e set +x -SNAP_MOUNT_DIR="@SNAP_MOUNT_DIR@" +STATIC_SNAP_MOUNT_DIR="@STATIC_SNAP_MOUNT_DIR@" show_help() { exec cat <<'EOF' @@ -18,12 +18,12 @@ A simple script to cleanup snap installations. optional arguments: --help Show this help message and exit - --snap-mount-dir= Provide a path to be used as $SNAP_MOUNT_DIR - --purge Purge all data from $SNAP_MOUNT_DIR + --snap-mount-dir= Provide a path to be used as $STATIC_SNAP_MOUNT_DIR + --purge Purge all data from $STATIC_SNAP_MOUNT_DIR EOF } -SNAP_UNIT_PREFIX="$(systemd-escape -p ${SNAP_MOUNT_DIR})" +SNAP_UNIT_PREFIX="$(systemd-escape -p ${STATIC_SNAP_MOUNT_DIR})" systemctl_stop() { unit="$1" @@ -68,36 +68,36 @@ purge() { systemctl_stop "$unit" if echo "$unit" | grep -q '.*\.mount' ; then - # Transform ${SNAP_MOUNT_DIR}/core/3440 -> core/3440 removing any + # Transform ${STATIC_SNAP_MOUNT_DIR}/core/3440 -> core/3440 removing any # extra / preceding snap name, eg: # /var/lib/snapd/snap/core/3440 -> core/3440 # /snap/core/3440 -> core/3440 # /snap/core//3440 -> core/3440 # NOTE: we could have used `systemctl show $unit -p Where --value` # but systemd 204 shipped with Ubuntu 14.04 does not support this - snap_rev=$(systemctl show "$unit" -p Where | sed -e 's#Where=##' -e "s#$SNAP_MOUNT_DIR##" -e 's#^/*##') + snap_rev=$(systemctl show "$unit" -p Where | sed -e 's#Where=##' -e "s#$STATIC_SNAP_MOUNT_DIR##" -e 's#^/*##') snap=$(echo "$snap_rev" |cut -f1 -d/) rev=$(echo "$snap_rev" |cut -f2 -d/) if [ -n "$snap" ]; then echo "Removing snap $snap" # aliases - if [ -d "${SNAP_MOUNT_DIR}/bin" ]; then - find "${SNAP_MOUNT_DIR}/bin" -maxdepth 1 -lname "$snap" -delete - find "${SNAP_MOUNT_DIR}/bin" -maxdepth 1 -lname "$snap.*" -delete + if [ -d "${STATIC_SNAP_MOUNT_DIR}/bin" ]; then + find "${STATIC_SNAP_MOUNT_DIR}/bin" -maxdepth 1 -lname "$snap" -delete + find "${STATIC_SNAP_MOUNT_DIR}/bin" -maxdepth 1 -lname "$snap.*" -delete fi # generated binaries - rm -f "${SNAP_MOUNT_DIR}/bin/$snap" - rm -f "${SNAP_MOUNT_DIR}/bin/$snap".* + rm -f "${STATIC_SNAP_MOUNT_DIR}/bin/$snap" + rm -f "${STATIC_SNAP_MOUNT_DIR}/bin/$snap".* # snap mount dir - umount -l "${SNAP_MOUNT_DIR}/$snap/$rev" 2> /dev/null || true - rm -rf "${SNAP_MOUNT_DIR:?}/$snap/$rev" - rm -f "${SNAP_MOUNT_DIR}/$snap/current" + umount -l "${STATIC_SNAP_MOUNT_DIR}/$snap/$rev" 2> /dev/null || true + rm -rf "${STATIC_SNAP_MOUNT_DIR:?}/$snap/$rev" + rm -f "${STATIC_SNAP_MOUNT_DIR}/$snap/current" # snap data dir rm -rf "/var/snap/$snap/$rev" rm -rf "/var/snap/$snap/common" rm -f "/var/snap/$snap/current" # opportunistic remove (may fail if there are still revisions left) - for d in "${SNAP_MOUNT_DIR}/$snap" "/var/snap/$snap"; do + for d in "${STATIC_SNAP_MOUNT_DIR}/$snap" "/var/snap/$snap"; do if [ -d "$d" ]; then rmdir --ignore-fail-on-non-empty "$d" fi @@ -135,9 +135,9 @@ purge() { # Units may have been removed do a reload systemctl -q daemon-reload || true - # Undo any bind mounts to ${SNAP_MOUNT_DIR} or /var/snap done by parallel + # Undo any bind mounts to ${STATIC_SNAP_MOUNT_DIR} or /var/snap done by parallel # installs or LP:#1668659 - for mp in "$SNAP_MOUNT_DIR" /var/snap; do + for mp in "$STATIC_SNAP_MOUNT_DIR" /var/snap; do # btrfs bind mounts actually include subvolume in the filesystem-path # https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg51810.html if grep -q " $mp $mp " /proc/self/mountinfo || @@ -181,7 +181,7 @@ purge() { rm -rf /var/lib/snapd/features echo "Final directory cleanup" - rm -rf "${SNAP_MOUNT_DIR}" + rm -rf "${STATIC_SNAP_MOUNT_DIR}" rm -rf /var/snap echo "Removing leftover snap shared state data" @@ -220,8 +220,8 @@ while [ -n "$1" ]; do exit ;; --snap-mount-dir=*) - SNAP_MOUNT_DIR=${1#*=} - SNAP_UNIT_PREFIX=$(systemd-escape -p "$SNAP_MOUNT_DIR") + STATIC_SNAP_MOUNT_DIR=${1#*=} + SNAP_UNIT_PREFIX=$(systemd-escape -p "$STATIC_SNAP_MOUNT_DIR") shift ;; --purge) diff --git a/cmd/snap/cmd_snap_op.go b/cmd/snap/cmd_snap_op.go index 4e4aa1f74b7..5a391ac480c 100644 --- a/cmd/snap/cmd_snap_op.go +++ b/cmd/snap/cmd_snap_op.go @@ -439,7 +439,7 @@ func maybeWithSudoSecurePath() bool { return false } // Known distros setting secure_path that does not include - // $SNAP_MOUNT_DIR/bin: + // $STATIC_SNAP_MOUNT_DIR/bin: return release.DistroLike("fedora", "opensuse", "debian") } diff --git a/cmd/snapd-env-generator/main.c b/cmd/snapd-env-generator/main.c index 84efe057a33..d3ed7648a52 100644 --- a/cmd/snapd-env-generator/main.c +++ b/cmd/snapd-env-generator/main.c @@ -28,7 +28,7 @@ // in Ubuntu 17.10+ int main(int argc, char **argv) { - const char *snap_bin_dir = SNAP_MOUNT_DIR "/bin"; + const char *snap_bin_dir = STATIC_SNAP_MOUNT_DIR "/bin"; char *path = getenv("PATH"); if (path == NULL || sc_streq(path, "")) { diff --git a/cmd/snapd-generator/main.c b/cmd/snapd-generator/main.c index 0862db0adf8..a9b2d68b881 100644 --- a/cmd/snapd-generator/main.c +++ b/cmd/snapd-generator/main.c @@ -260,8 +260,8 @@ static int ensure_root_fs_shared(const char *normal_dir) "Description=Ensure that the snap directory " "shares mount events.\n"); fprintf(f, "[Mount]\n"); - fprintf(f, "What=" SNAP_MOUNT_DIR "\n"); - fprintf(f, "Where=" SNAP_MOUNT_DIR "\n"); + fprintf(f, "What=" STATIC_SNAP_MOUNT_DIR "\n"); + fprintf(f, "Where=" STATIC_SNAP_MOUNT_DIR "\n"); fprintf(f, "Type=none\n"); fprintf(f, "Options=bind,shared\n");