openhab / openhab-linuxpkg

Repo for Linux packages
Eclipse Public License 2.0
18 stars 33 forks source link

Modify PID detection #46

Closed ralle12345 closed 7 years ago

ralle12345 commented 7 years ago

The changes in the pull request change the PID detection of the running openhab2 instance to match only on the 'ps aux' line of the openhab2 instance, but not on the openhab1 instance running on the same system.

ThomDietrich commented 7 years ago

Okay I guess. @BClark09: ralle also changed file permissions with this PR. Looks okay to me, just wanted to point it out.

ralle12345 commented 7 years ago

Funny, I just did a checkout of the repo, edited with TextMate on OSX and committed...

BClark09 commented 7 years ago

Fine by me!

(Thanks @ThomDietrich, actually I would probably have missed that, as it's very subtle on Github. ;) )

BClark09 commented 7 years ago

@ralle12345, before I merge could you sign off your commit please?

ralle12345 commented 7 years ago

Yesterday I found two other problems with the init.d script.

  1. the init.d script ignores the settings regarding OPENHAB_HTTP(S)_PORT made in /etc/default/openhab2
  2. the init.d script is not marked as conffile in the deb package leading to a modified init script being overwritten on package update.

I have solved (1) and I could push the commit to the pull request. Should I do that?

Haven't been able to solve (2), though. I have searched around here in build.gradle, but adding "fileType CONFIG | NOREPLACE" to the init file didn't work out the way I expected :(.

BClark09 commented 7 years ago

Hi @ralle12345, thanks for that.

  1. Absolutely, both of these problems relate to the sysVinit problems we want to fix so I'd more than welcome a change in this PR, provided that you sign it off. As a note, none of these problems occur in later versions of Ubuntu/Debian because these tend towards systemd.

  2. Files for a .deb package need to be placed inside of a configurationFile() call, fileType is used for the RPM packages. However, I don't think the init.d script should be a conffile for the following reasons:

    • We need to be absolutely sure that any changes to the startup are reflected upon updates.
    • The startup script should call files like /etc/default/openhab2 in order to change user configurable parameters, these files won't change on update.
ralle12345 commented 7 years ago

I will add the commit then and sign off. Is it sufficient to sign off the latest commit?

I understand your point, but in the current way a modified init.d script is overwritten without a question. Other debian packages ask wether to overwrite the current file or keep the modified one.

BTW: I know that more recent versions of Ubuntu/Debian use systemd. My openhab system is the last one with SysV-init.

BClark09 commented 7 years ago

Thanks @ralle12345, I managed to test this and it works as intended.

Other debian packages ask whether to overwrite the current file

I thought about this, actually in Debian's policy manual, it suggests making init.d scripts conffiles. What would be the typical use case for changing the startup script?

ralle12345 commented 7 years ago

My use case was to have an init script that had what I submitted in the PR :). For now the use case is done with. But there may be other changes that one wants to incorporate into the init script.

I agree that "joe Average" should just be supplied the most recent init script that openHAB delivers, but someone who has special needs should be able to get what he needs (and keep his changes over package upgrades). By marking the init script as a conffile dpkg should ask the user before overwriting the changed file and give a chance to review what has changed.

To me it was a real PITA to have the vanilla version of the init script kill my openHAB1 instance each time I upgraded openHAB2.

BClark09 commented 7 years ago

Fair enough, you've convinced me ;) I probably will set it as a conffile later on. I have a couple of edits I want to make to the script at first before I do though.

For cases like the PR, I'd strongly encourage everyone to either make an issue or a PR as you did, as these are the issues that would effect everyone. Your fix will be appreciated by many I believe.