Do not update systemd script if file exists

node-red / linux-installers

Node-RED install scripts for various flavours of Linux

Apache License 2.0

94 stars 63 forks source link

Do not update systemd script if file exists #26

Closed ralphwetzel closed 2 years ago

ralphwetzel commented 2 years ago

Currently nodered.service is always recreated from the GitHub master template as part of the update process (at least on RPi). Any modification (e.g. definition of a custom --userDir) in this file thus will be overwritten - without notice or warning.

This PR adds the following logic:

If the file does not exist, create it - as usual.
If the file exists and the definition of User:Group and WorkingDirectory matches the current installation: Keep the existing file & additionally create an alternative one from the master template. This allows to compare the both files in case of doubt.
If the file exists and the definition of User:Group and WorkingDirectory does not match the current installation: Backup the existing file & create a new one from the master template. This ensures that the modifications (at least) are preserved...

Closes #25.

dceejay commented 2 years ago

Hi - Thanks for the PR - but I think this may be overthinking the issue. I'm happy to not recreate the file if it already exists. If it already does then yes we should say so - but trying to spot relevant differences is beyond the scope of what we need to do. So I think it should just tell the user we have or have not created the file. I don't think we should create extra dummy files that pollute the users system. If the user wants us to recreate it then they can delete/rename the file and re-run the install.

ralphwetzel commented 2 years ago

Hi! Thank you for your feedback; fair comments.

Proposal for the adaptation:

If the file does not exist, create it - as usual.
If the file exists: run the file creation process & compare the results:
- If the 2 files are equal: Mark as successful, no further comment.
- If there's a difference: State sth like "Modified script found, check log for details" & explain there "If the user wants us to recreate it then they can delete/rename the file and re-run the install."
- in both cases: Do not change the existing file.

This should ensure that most users will experience the usual feedback ... and only those who really modified nodered.service get a comment. There's an edge case though: If there's a modification of the master template, this will trigger the 'mod script found' message as well.

dceejay commented 2 years ago

Sounds cleaner thanks. And yes in the rare chance we do update the master we could also update the message to say what had changed - or indeed that it was our change and suggest what to do etc..

ralphwetzel commented 2 years ago

Hello again! I've simplified the logic according to our discussion:

Ready for review again...

dceejay commented 2 years ago

Great - much cleaner. Thanks