From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] addon rsnapshot Date: Wed, 22 Mar 2023 10:01:10 +0000 Message-ID: <44451D9E-0845-4403-A32C-03FD38664E87@ipfire.org> In-Reply-To: <20230319125027.1488265-1-gerd@hoerst.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6672114805188481025==" List-Id: --===============6672114805188481025== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hello Gerd, Thank you for your submission. I would once again like to highlight that we have collected a large number of= DOs and DONTs here: https://wiki.ipfire.org/devel/submit-patches That process is needed because there are technical requirements for most step= s, and secondly it explains that to work together with other developers. In this instance, this is a patch that does not at all say what this is suppo= sed to do, what the benefits are, why we should ship this at all, etc. There = is no commit message and not even the subject includes any purpose. Just the = name of the package. It is very important to justify *any* patch as explained here: https://wiki.ipfire.org/devel/git/commit-messages We need to know the =E2=80=9Cwhy=E2=80=9D. Arguably this could be rather shor= t when the patch fixes a typo, but adding a new package which will create mai= ntenance work for the whole team should be justified. > On 19 Mar 2023, at 12:50, Gerd Hoerst wrote: >=20 > Signed-off-by: Gerd Hoerst > --- > config/rootfiles/packages/rsnapshot | 6 ++ > lfs/rsnapshot | 86 +++++++++++++++++++++++++++++ > make.sh | 1 + > src/paks/rsnapshot/install.sh | 27 +++++++++ > src/paks/rsnapshot/uninstall.sh | 26 +++++++++ > src/paks/rsnapshot/update.sh | 26 +++++++++ > 6 files changed, 172 insertions(+) > create mode 100644 config/rootfiles/packages/rsnapshot > create mode 100644 lfs/rsnapshot > create mode 100644 src/paks/rsnapshot/install.sh > create mode 100644 src/paks/rsnapshot/uninstall.sh > create mode 100644 src/paks/rsnapshot/update.sh >=20 > diff --git a/config/rootfiles/packages/rsnapshot b/config/rootfiles/package= s/rsnapshot > new file mode 100644 > index 000000000..023498e2c > --- /dev/null > +++ b/config/rootfiles/packages/rsnapshot > @@ -0,0 +1,6 @@ > +usr/bin/rsnapshot > +usr/bin/rsnapshot-diff > +etc/rsnapshot.conf > +#etc/rsnapshot.conf.default > +#usr/share/man/man1/rsnapshot-diff.1 > +#usr/share/man/man1/rsnapshot.1 The rootfile is not in alphabetical order. That suggests that this was not ge= nerated from a clean build. Could you please verify that it includes all file= s that have been created? > diff --git a/lfs/rsnapshot b/lfs/rsnapshot > new file mode 100644 > index 000000000..0445471be > --- /dev/null > +++ b/lfs/rsnapshot > @@ -0,0 +1,86 @@ > +##########################################################################= ##### > +# = # > +# IPFire.org - A linux based firewall = # > +# Copyright (C) 2007-2020 IPFire Team = # > +# = # > +# This program is free software: you can redistribute it and/or modify = # > +# it under the terms of the GNU General Public License as published by = # > +# the Free Software Foundation, either version 3 of the License, or = # > +# (at your option) any later version. = # > +# = # > +# This program is distributed in the hope that it will be useful, = # > +# but WITHOUT ANY WARRANTY; without even the implied warranty of = # > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the = # > +# GNU General Public License for more details. = # > +# = # > +# You should have received a copy of the GNU General Public License = # > +# along with this program. If not, see . = # > +# = # > +##########################################################################= ##### > + > +##########################################################################= ##### > +# Definitions > +##########################################################################= ##### > + > +include Config > + > +VER =3D 1.4.5 > + > +THISAPP =3D rsnapshot-$(VER) > +DL_FILE =3D $(THISAPP).tar.gz > +DL_FROM =3D https://github.com/rsnapshot/rsnapshot/releases/download/1.= 4.5 > +DIR_APP =3D $(DIR_SRC)/$(THISAPP) > +TARGET =3D $(DIR_INFO)/$(THISAPP) > +PROG =3D rsnapshot > +PAK_VER =3D 1 > +DEPS =3D "rsync" DEPS should not be quoted. > +##########################################################################= ##### > +# Top-level Rules > +##########################################################################= ##### > + > +objects =3D $(DL_FILE) > + > +$(DL_FILE) =3D $(DL_FROM)/$(DL_FILE) > + > +$(DL_FILE)_BLAKE2 =3D 2a668aa16991b2b4e611c6204cdcd0e8c9593e5f0af5ea89e787= a578e73b6f5987514cd7d0252bb78aea1b157ef85aea947686111ca9e3befdb2a8cef0aa9ecd > + > +install : $(TARGET) > + > +check : $(patsubst %,$(DIR_CHK)/%,$(objects)) > + > +download :$(patsubst %,$(DIR_DL)/%,$(objects)) > +b2 : $(subst %,%_BLAKE2,$(objects)) > + > +dist:=20 > + @$(PAK) > +##########################################################################= ##### > +# Downloading, checking, md5sum > +##########################################################################= ##### > + > +$(patsubst %,$(DIR_CHK)/%,$(objects)) : > + @$(CHECK) > + > +$(patsubst %,$(DIR_DL)/%,$(objects)) : > + @$(LOAD) > + > +$(subst %,%_MD5,$(objects)) : > + @$(MD5) > + > +##########################################################################= ##### > +# Installation Details > +##########################################################################= ##### > + > +$(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) > + @$(PREBUILD) > + @rm -rf $(DIR_APP) && cd $(DIR_SRC) && tar zxf $(DIR_DL)/$(DL_FILE) > + cd $(DIR_APP) && \ > + ./configure \ > + --prefix=3D/usr \ > + --sysconfdir=3D/etc \ > + --bindir=3D/usr/bin \ > + > + cd $(DIR_APP) && make $(MAKETUNING) > + cd $(DIR_APP) && make install > + cp -avfr $(DIR_CONF)/$(PROG)/* / > + @rm -rf $(DIR_APP) > + @$(POSTBUILD) > diff --git a/make.sh b/make.sh > index 3b7f9850c..59d2971da 100755 > --- a/make.sh > +++ b/make.sh > @@ -1478,6 +1478,7 @@ buildipfire() { > lfsmake2 libmpeg2 > lfsmake2 gnump3d > lfsmake2 rsync > + lfsmake2 rsnapshot > lfsmake2 rpcbind > lfsmake2 keyutils > lfsmake2 nfs > diff --git a/src/paks/rsnapshot/install.sh b/src/paks/rsnapshot/install.sh > new file mode 100644 > index 000000000..9aafb0d56 > --- /dev/null > +++ b/src/paks/rsnapshot/install.sh > @@ -0,0 +1,27 @@ > +#!/bin/bash > +##########################################################################= ## > +# = # > +# This file is part of the IPFire Firewall. = # > +# = # > +# IPFire is free software; you can redistribute it and/or modify = # > +# it under the terms of the GNU General Public License as published by = # > +# the Free Software Foundation; either version 2 of the License, or = # > +# (at your option) any later version. = # > +# = # > +# IPFire is distributed in the hope that it will be useful, = # > +# but WITHOUT ANY WARRANTY; without even the implied warranty of = # > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the = # > +# GNU General Public License for more details. = # > +# = # > +# You should have received a copy of the GNU General Public License = # > +# along with IPFire; if not, write to the Free Software = # > +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA= # > +# = # > +# Copyright (C) 2007 IPFire-Team . = # > +# = # > +##########################################################################= ## > +# > +. /opt/pakfire/lib/functions.sh=20 > + > +extract_files=20 > +restore_backup ${NAME} > diff --git a/src/paks/rsnapshot/uninstall.sh b/src/paks/rsnapshot/uninstall= .sh > new file mode 100644 > index 000000000..66f4344eb > --- /dev/null > +++ b/src/paks/rsnapshot/uninstall.sh > @@ -0,0 +1,26 @@ > +#!/bin/bash > +##########################################################################= ## > +# = # > +# This file is part of the IPFire Firewall. = # > +# = # > +# IPFire is free software; you can redistribute it and/or modify = # > +# it under the terms of the GNU General Public License as published by = # > +# the Free Software Foundation; either version 2 of the License, or = # > +# (at your option) any later version. = # > +# = # > +# IPFire is distributed in the hope that it will be useful, = # > +# but WITHOUT ANY WARRANTY; without even the implied warranty of = # > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the = # > +# GNU General Public License for more details. = # > +# = # > +# You should have received a copy of the GNU General Public License = # > +# along with IPFire; if not, write to the Free Software = # > +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA= # > +# = # > +# Copyright (C) 2007 IPFire-Team . = # > +# = # > +##########################################################################= ## > +# > +. /opt/pakfire/lib/functions.sh > +make_backup ${NAME} > +remove_files > diff --git a/src/paks/rsnapshot/update.sh b/src/paks/rsnapshot/update.sh > new file mode 100644 > index 000000000..89c40d0d7 > --- /dev/null > +++ b/src/paks/rsnapshot/update.sh > @@ -0,0 +1,26 @@ > +#!/bin/bash > +##########################################################################= ## > +# = # > +# This file is part of the IPFire Firewall. = # > +# = # > +# IPFire is free software; you can redistribute it and/or modify = # > +# it under the terms of the GNU General Public License as published by = # > +# the Free Software Foundation; either version 2 of the License, or = # > +# (at your option) any later version. = # > +# = # > +# IPFire is distributed in the hope that it will be useful, = # > +# but WITHOUT ANY WARRANTY; without even the implied warranty of = # > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the = # > +# GNU General Public License for more details. = # > +# = # > +# You should have received a copy of the GNU General Public License = # > +# along with IPFire; if not, write to the Free Software = # > +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA= # > +# = # > +# Copyright (C) 2007 IPFire-Team . = # > +# = # > +##########################################################################= ## > +# > +. /opt/pakfire/lib/functions.sh > +./uninstall.sh > +./install.sh There is no need to have any custom install/uninstall/update scripts when the= y are not doing anything the default set isn=E2=80=99t doing. You are calling backup restore functions but there is no backup include file = for the configuration file. So these can be removed even if you were to add a= backup include file. Best, -Michael > --=20 > 2.25.1 >=20 --===============6672114805188481025==--