Monitorix icon indicating copy to clipboard operation
Monitorix copied to clipboard

BUG monitorix.conf overwrite

Open Saentist opened this issue 2 years ago • 23 comments

When used make install-systemd-all (posibly in other manual install types) monitorix.conf is overwriten, without any question. With dpkg -i monitorix.deb it ask do we want to overwrite config file.

Sugestion: adding simple logic to Makefile IF monitorix.conf exist in /etc/monitorix THEN copy to monitorix.conf.default ELSE just copy it ;)

No config lost.

 make install-systemd-all
Installing script and modules...
install -p -d "/usr/bin"
install -p -m755 monitorix "/usr/bin/monitorix"
install -p -d "/var/lib/monitorix"
install -p -d "/var/lib/monitorix"
install -p -d "/var/lib/monitorix/www"
install -p -d "/var/lib/monitorix/www/cgi"
install -p -dm755 "/var/lib/monitorix/www/imgs"
install -p -m755 monitorix.cgi "/var/lib/monitorix/www/cgi/monitorix.cgi"
install -p -m644 logo_bot.png "/var/lib/monitorix/www/logo_bot.png"
install -p -m644 logo_top.png "/var/lib/monitorix/www/logo_top.png"
install -p -m644 monitorixico.png "/var/lib/monitorix/www/monitorixico.png"
install -p -d "/var/lib/monitorix/www/css"
install -p -m644 css/black.css "/var/lib/monitorix/www/css/black.css"
install -p -m644 css/white.css "/var/lib/monitorix/www/css/white.css"
install -p -d "/etc/monitorix"
install -p -m644 monitorix.conf "/etc/monitorix/monitorix.conf"
...

Saentist avatar Mar 16 '22 21:03 Saentist

I'll see what can I do. Meanwhile read this FAQ to configure Monitorix properly.

mikaku avatar Mar 17 '22 06:03 mikaku

@Saentist, please, check this patch and let me know if it works.

mikaku avatar Mar 23 '22 08:03 mikaku

@mikaku not work

/etc/monitorix# ll
drwxr-xr-x   3 root root  4096 мар 23 13:13 ./
drwxr-xr-x 164 root root 12288 мар 23 02:59 ../
drwxr-xr-x   2 root root  4096 мар 16 16:14 conf.d/
-rw-r--r--   1 root root 34812 мар 16 22:22 monitorix.conf
-rw-r--r--   1 root root 28996 мар 17 00:02 monitorix.conf~

work in opposite

ifneq ("$(wildcard $(PATH_TO_FILE))","")
FILE_EXISTS = 1
else
FILE_EXISTS = 0
endif

https://fedingo.com/how-to-check-if-file-exists-in-shell-script/

Saentist avatar Mar 23 '22 11:03 Saentist

its not bad to add somewhere on systemd section

sudo systemctl daemon-reload
sudo systemctl enable monitorix.service
sudo systemctl reload-or-restart monitorix

Saentist avatar Mar 23 '22 11:03 Saentist

work in opposite

Please elaborate.

mikaku avatar Mar 23 '22 14:03 mikaku

script backup (somewhere) current config and place default config from repo in config folder

again we go to this old situation https://github.com/mikaku/Monitorix/issues/295

Saentist avatar Mar 23 '22 14:03 Saentist

script backup (somewhere) current config and place default config from repo in config folder

Sorry, I don't know if I understand you correctly.

You mean that the new monitorix.conf is renamed as monitorix.conf~ and your current configuration file is not touched?

mikaku avatar Mar 23 '22 15:03 mikaku

no monitorix.conf in /etc/monitorix is rewritten from repo where is backuped one i don't know monitorix.conf~ is backup from nano editor, ignore this file

file='/etc/monitorix/monitorix.conf'
if [ -f $file ]; then
    #code to be run if file exists
else
    #code to be run if file does not exist
fi

If i read correctly in current Makefile $(INSTALL_DATA) $(BACKUP) $(PN).conf "$(DESTDIR)$(CONFDIR)/$(PN)/$(PN).conf'' is install -p -m644 -b monitorix.conf "$(DESTDIR)/etc/monitorix/monitorix/conf"

there is no clear what is variable =''$(DESTDIR)'', guess is file system root ''/''

Saentist avatar Mar 23 '22 16:03 Saentist

monitorix.conf~ is backup from nano editor, ignore this file

No, the monitorix.conf~ is created by the command install when executing Makefile. It is the backup of the previous configuration file before installing the new one.

there is no clear what is variable =''$(DESTDIR)'', guess is file system root ''/''

$(DESTDIR) has no value, and since all pathname are absolute this puts this variable as a user decision if you have installed Monitorix on a different place.

If you set DESTDIR=/home/peter/ the file Makefile will install Monitorix inside such directory. Of course, you'll have to setup the configuration file accordingly.

mikaku avatar Mar 23 '22 17:03 mikaku

No, the monitorix.conf~ is created by the command install when executing Makefile. It is the backup of the previous configuration file before installing the new one.

So work in opposite if file monitorix.conf exist renamed as monitorix.conf~ and monitorix.conf copyed I cant see logic in this, user to go manually and rename files, to see his working config. And most worst is if open it with standard nano editor monitorix.conf backup is gone

if /etc/monitorix/monitorix.conf file exist, then script must copy repo file as monitorix.conf.default to /etc/monitorix else just copy to newer empty folder /etc/monitorix

This is most simple logic, config is there and work, and there is also a default new config with eventually newer features.

Saentist avatar Mar 23 '22 17:03 Saentist

And most worst is if open it with standard nano editor monitorix.conf backup is gone

I can change the suffix of the backed up file with --suffix=.bak, the rest of behavior is how install works.

mikaku avatar Mar 23 '22 17:03 mikaku

This is most simple logic, config is there and work, and there is also a default new config with eventually newer features.

I agree, it means then we cannot use this functionality of install. I'll see what can I do.

mikaku avatar Mar 23 '22 17:03 mikaku

But why you want to backup working config, and replace it with default not working one?

Saentist avatar Mar 23 '22 17:03 Saentist

But why you want to backup working config, and replace it with default not working one?

Because you shouldn't use the monitorix.conf to configure your Monitorix. Instead, you should create your own configuration file in conf.d/.

This way your Monitorix is always up-to-date.

mikaku avatar Mar 23 '22 17:03 mikaku

Apart from that, nice thing with RPM (and DEB) packaging:

%config(noreplace) %{_sysconfdir}/monitorix/monitorix.conf

If the user has modified the file, the installer (rpm/dpkg) will ask the user what to do then: install the new, keep the old, view the diff, edit.

But much better if you as a user put your own changes into separate files in conf.d/. You only need to put the sections you modified there, not the complete config. And you can have multiple files for easier maintenance. One example is already there with the .deb

%config(noreplace) %{_sysconfdir}/monitorix/conf.d/00-debian.conf

Whatever you place there in separate files has no risk to be overwritten. This was introduced to Monitorix some years ago for exactly this purpose.

IzzySoft avatar Mar 23 '22 21:03 IzzySoft

As @Saentist observed, the parameter -b (backup) in the command install when using Makefile to install Monitorix, works in the opposite direction than the common package managers (rpm / dpkg) do. That is, those common package managers will keep your configuration intact and will place the new configuration file apart (e.g. will be suffixed as .rpmnew in the case of rpm).

Since almost no one is using the Makefile instead the its own package manager to install Monitorix, I'm not sure if it worth the work to force Makefile to behave like the common package managers.

Thoughts?

mikaku avatar Mar 24 '22 10:03 mikaku

Since almost no one is using the Makefile instead the its own package manager to install Monitorix, I'm not sure if it worth the work to force Makefile to behave like the common package managers.

Except people with use git version with all new stuff. Why not try to implement some of examples above?

Saentist avatar Mar 24 '22 16:03 Saentist

Why not try to implement some of examples above?

The syntax of the examples above is for a .spec file only, and they are already implemented.

mikaku avatar Mar 24 '22 16:03 mikaku

instead the its own package manager

Good point: we package managers use that to build the packages :rofl: But as in that case the install always happens to an empty dir (for a clean package), we can ignore that :wink: And apologies if I caused confusion with the .spec examples.

That apart, as Jordi wrote: never modify the original config, always use conf.d for your own stuff – and all is well. I fully agree it's not worth reworking the Makefile logic. If you stick to the rules, there's no need for that – and if you don't: well… I'd say then it's your own responsibility. No warranties on overclocked CPUs :wink:

IzzySoft avatar Mar 24 '22 20:03 IzzySoft

Why not try to implement some of examples above?

The syntax of the examples above is for a .spec file only, and they are already implemented.

What .spec? Example is for makefile

ifneq ("$(wildcard $(PATH_TO_FILE))","")
FILE_EXISTS = 1
else
FILE_EXISTS = 0
endif

there is lot of resources https://makefiletutorial.com

Saentist avatar Mar 25 '22 06:03 Saentist

What is a spec file?

Please read first post. Problem is not in packages Problem reported is when is used make install-systemd-all Makefile is afected by this issue.

If problem was in packages, i'll ask why in your repo there is no "Nightly builds".

Saentist avatar Mar 25 '22 08:03 Saentist

there is lot of resources https://makefiletutorial.com/

I'll take a look, thanks for the URL.

mikaku avatar Mar 25 '22 08:03 mikaku

This commit will save as .bak your current config file.

I don't know enough the Makefile syntax to provide a better solution, sorry. If you want to improve it then provide yourself a modified version of Makefile.

mikaku avatar Nov 10 '22 11:11 mikaku