hirak99 / yabsnap

Btrfs Scheduled Snapshot Manager for Arch
Apache License 2.0
62 stars 2 forks source link

Perhaps rely on ‘/etc/lsb-release’? #1

Closed kseistrup closed 1 year ago

kseistrup commented 1 year ago

I can see that the install-to-dest.sh script is grepping for Arch Linux in the file /etc/issue. I think that's a very fragile way of doing it since the system administrator may modify that file at any time (see issue(5) for details).

Instead, make yabsnap depend on the lsb-release package and make sure (1) that /etc/lsb-release exists, and (2) that $DISTRIB_ID equals Arch. Perhaps something along these lines:

ME="${0##*/}"
ETC_LSB_RELEASE='/etc/lsb-release'

test -r "$ETC_LSB_RELEASE" || {
  printf '%s: cannot read %s\n' "$ME" "$ETC_LSB_RELEASE" >&2
  exit 1
}

. "$ETC_LSB_RELEASE"

test "$DISTRIB_ID" = 'Arch' || {
  printf '%s: Not an Arch based distro, not proceeding.\n' "$ME" >&2
  exit 1
}
kseistrup commented 1 year ago

PS: https://github.com/hirak99/yabsnap/blob/master/scripts/install-to-dest.sh#L25

hirak99 commented 1 year ago

Thank you for the suggestion. Commit https://github.com/hirak99/yabsnap/commit/8043bac7b0d0964d65a76cfbaf02bb75e86e9a90 should resolve this.

Also 'lsb-release' was added to makedepends() in PKGBUILD.

hirak99 commented 1 year ago

Ahh unfortunately this wouldn't work for Arch derived distros, such as Manjaro or EndeavourOS.

Perhaps checking for disrtro is misguided; all we want is pacman to be present. I'll change this to just look for /usr/bin/pacman.

kseistrup commented 1 year ago

Ahh unfortunately this wouldn't work for Arch derived distros, such as Manjaro or EndeavourOS.

What do those distros have in /etc/lsb-release? Wouldn't something like

case "$(lsb_realease -si)" in
  Arch | Manjaro | EndeavourOS )
    :  # we're good, just drop through the case statement
  ;;
  * )
    echo 'Not an ArchLinux based distro' >&2
    exit 1
  ;;
esac

work?

hirak99 commented 1 year ago

Yes that should also work.

But I realized what I really wanted to check - via the distribution - was whether the distro supports the libalpm hook. Checking for existence of pacman feels like a more direct way to do that.