Add some improvements to init script. #964

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@marcelaraujo

Added restart support.

@moreaki

This comment has been minimized.

Show comment Hide comment
@moreaki

moreaki Aug 20, 2013

While your additions are well-intended, I believe that in the current state of affairs they will not be propagated to the final boot control scripts upon installation. Check the utils/install_server.sh script in the sources, which basically compiles the final init script. I think a more intended patch could look like follows:

--- a/utils/install_server.sh
+++ b/utils/install_server.sh
@@ -136,6 +136,7 @@ rm -f $TMP_FILE
 REDIS_INIT_HEADER=\
 "#/bin/sh\n
 #Configurations injected by install_server below....\n\n
+. /etc/rc.d/init.d/function\n\n
 EXEC=$REDIS_EXECUTABLE\n
 CLIEXEC=$CLI_EXEC\n
 PIDFILE=$PIDFILE\n

While your additions are well-intended, I believe that in the current state of affairs they will not be propagated to the final boot control scripts upon installation. Check the utils/install_server.sh script in the sources, which basically compiles the final init script. I think a more intended patch could look like follows:

--- a/utils/install_server.sh
+++ b/utils/install_server.sh
@@ -136,6 +136,7 @@ rm -f $TMP_FILE
 REDIS_INIT_HEADER=\
 "#/bin/sh\n
 #Configurations injected by install_server below....\n\n
+. /etc/rc.d/init.d/function\n\n
 EXEC=$REDIS_EXECUTABLE\n
 CLIEXEC=$CLI_EXEC\n
 PIDFILE=$PIDFILE\n

This comment has been minimized.

Show comment Hide comment
@marcelaraujo

marcelaraujo Aug 20, 2013

Owner

Who is responsable for applying patches?

I gonna change it because on Red Hat/Fedora/CentOS systems, the init script doesn't works and it can be added to boot initialization without init.d functions.

Owner

marcelaraujo replied Aug 20, 2013

Who is responsable for applying patches?

I gonna change it because on Red Hat/Fedora/CentOS systems, the init script doesn't works and it can be added to boot initialization without init.d functions.

This comment has been minimized.

Show comment Hide comment
@moreaki

moreaki Aug 20, 2013

https://github.com/antirez (@antirez) is the main responsible person for applying changes, however he's a busy chap and init script changes most probably are not very high up his todo list. I just add some comments here and there to facilitate his decision making in either applying the patch or dropping it all together and closing the issue; sort of a backlog reduction facilitator.

On a sidenote: as you can see, your patch has been superseed by more complete cleanup, which I have referenced somewhere.

https://github.com/antirez (@antirez) is the main responsible person for applying changes, however he's a busy chap and init script changes most probably are not very high up his todo list. I just add some comments here and there to facilitate his decision making in either applying the patch or dropping it all together and closing the issue; sort of a backlog reduction facilitator.

On a sidenote: as you can see, your patch has been superseed by more complete cleanup, which I have referenced somewhere.

@moreaki

This comment has been minimized.

Show comment Hide comment
@moreaki

moreaki Aug 20, 2013

This looks just fine, although I wonder if the sleep time should be a settable parameter?

This looks just fine, although I wonder if the sleep time should be a settable parameter?

moreaki referenced this pull request in melvyn-sopacua/redis Aug 20, 2013

Melvyn Sopacua
Add restart command
While in here, make sure the provides: line is correct.
@moreaki

This comment has been minimized.

Show comment Hide comment
@moreaki

moreaki Aug 20, 2013

Hmm, the set of cleanups proposed in might be favourable to this pull request: #1066. The restart has been implemented here: melvyn-sopacua/redis@ac3c44a

moreaki commented Aug 20, 2013

Hmm, the set of cleanups proposed in might be favourable to this pull request: #1066. The restart has been implemented here: melvyn-sopacua/redis@ac3c44a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment