Opened 10 years ago
Closed 9 years ago
#875 closed defect (fixed)
properties files get unduly modified by installation procedure
Reported by: | Peter Baumann | Owned by: | Bang Pham Huu |
---|---|---|---|
Priority: | minor | Milestone: | 9.1.x |
Component: | build system | Version: | development |
Keywords: | Cc: | Dimitar Misev | |
Complexity: | Medium |
Description
the "make install" script for handling the petascope.properties and log4j.properties file can create a new file. While it attempts to copy over old settings some stuff, like comments, may get lost. The preocedure is meant for getting in new keys/values, which per se is good. A better approach, however, is to keep the old file and just append any newly introduced variables.
PS: in the script, this text is unclear:
"service existing service file, will not modify it"
Attachments (2)
Change History (14)
comment:1 by , 9 years ago
Component: | undecided → autotools |
---|---|
Milestone: | Future → 9.1.x |
Owner: | changed from | to
Status: | new → assigned |
comment:2 by , 9 years ago
Dimitar: agree (I can see you make backup petascope.properties before 9 in make install), but I don't know what is called "new" variable in here (new with one but old with others) (so the best way is compare with user's file, if they don't have property then I will add). I hope understand correctly (in fact, I also did it with Jetty properties (start_embedded_petascope and java_server)).
comment:3 by , 9 years ago
As discussed today, it's probably best to take that logic for updating the configuration file out of the Makefile.am, and put it in a script which will give us more power (e.g. python or bash).
comment:4 by , 9 years ago
ok, I got it, user will configured everything in petascope.properties (log4j.properties) in Rasdaman source code first and my script will get old configuration from user and modify the new petascope.properties last time before overwrite (with backup) to $RMANHOME/etc.
comment:5 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
The patch has been submitted as it has been validated carefully and accepted both by Dimitar, I will close this ticket here.
comment:6 by , 9 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
There is a slight bug in the update_properties.sh script: it creates petascope.properties backups on every make install. So I end up with a bunch of petascope.properties*.bak in $RMANHOME/etc, even though the petascope.properties file has not changed at all.
No backup should be left in etc/ if nothing has been updated.
Also the permissions of the petascope.properties are not right, they should be same like the other config files (-rw-r—r—):
-rw-r--r-- 1 dimitar dimitar 711 Nov 29 09:18 log-server.conf -rwxr-xr-x 1 dimitar dimitar 7441 Dec 2 17:37 petascope.properties -rwxr-xr-x 1 dimitar dimitar 7441 Dec 2 17:37 petascope.properties.12-02-2015_17-37-14.bak
comment:7 by , 9 years ago
Ok, Dimitar, actually a lot of things has added from the original purpose so I did not take care to check both file are identical first. A patch has been submitted and also fix the permission to 644 as you want. When the patch is accepted I will close this again.
comment:8 by , 9 years ago
Thanks for the quick fix! Please submit for review first next time, I had to go to the patchmanager to see the code.
+#2.3 Check if NEW and OLD file are the same (no need to create a backup and do anything) +logn "Checking the difference between Old and New configuration files..." +diff=$(diff "$NEW" "$OLD") +if [[ $diff == "" ]]; then + echo "Your old configuration is already up to date." + ok +fi +echo "Done."
$diff
should be in quotes in the if statement; "Checking the difference bet…" is not needed log output. It's better like this:
+#2.3 Check if NEW and OLD file are the same (no need to create a backup and do anything) +cmp --quiet "$NEW" "$OLD" +if [ $? -eq 0 ]; then + log "The existing configuration is already up to date." + ok +fi
by , 9 years ago
Attachment: | 0001-ticket-875-properties-files-get-unduly-modified-by-i.patch added |
---|
comment:9 by , 9 years ago
Sorry for inconvenience as I thought this is too small so don't push to "codereview", so I upload the patch manager to here (please check, if it is passed then I will upload to patch manager tomorrow - so you can delete the earlier patch before. Thanks).
comment:10 by , 9 years ago
Ok Bang, please test your fix first, it doesn't look like you did that at all. And remove the
logn "Checking the difference between Old and New configuration files..."
No rush, it doesn't need to be done right now in the night of course.
by , 9 years ago
Attachment: | 0001-ticket-875-properties-files-get-unduly-modified-by.patch added |
---|
New update
comment:11 by , 9 years ago
Hi Dimitar,
it should not be rush of course, but I also run update_petascope.properties few times before submitting a patch (and work in good behavior). I could not get your idea about it did not work as you want (it just compare Old and New configuration and if it is the same then exit NO_ERROR - do you want to add something for more detail?).
I submit a new patch without the line you don't want to see and of course run with "make install" inside petascope directory few times alo, please download and recheck.
comment:12 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
I will close as Dimitar has accepted and allow me to submit a patch. Thanks.
Bang, please make sure the existing petascope.properties file is backed up before it's modified.