Re: [PATCH] Win32: install service in rewrite args rather than post_config?
Bill,
How's this look?
svn diff
Index: mpm_winnt.c
===
--- mpm_winnt.c (revision 153937)
+++ mpm_winnt.c (working copy)
@@ -1071,7 +1071,8 @@
/* Handle the following SCM aspects in this phase:
*
* -k runservice [transition for WinNT, nothing for Win9x]
- * -k (!)install [error out if name is not installed]
+ * -k install
+ * -k config
* -k uninstall
* -k stop
* -k shutdown (same as -k stop). Maintained for backward compatability.
@@ -1280,6 +1281,15 @@
"%s: Service is already installed.", service_name);
exit(APEXIT_INIT);
}
+else
+{
+/* Install the service */
+rv = mpm_service_install(process->pool, inst_argc, inst_argv, 0);
+if (rv != APR_SUCCESS) {
+exit(rv);
+}
+/* Proceed to post_config in order to test the installed
configuration */
+}
}
else if (running_as_service)
{
@@ -1325,6 +1335,15 @@
"No installed service named \"%s\".", service_name);
exit(APEXIT_INIT);
}
+else if (!strcasecmp(signal_arg, "config"))
+{
+/* Reconfigure the service */
+rv = mpm_service_install(process->pool, inst_argc, inst_argv, 1);
+if (rv != APR_SUCCESS) {
+exit(rv);
+}
+/* Proceed to post_config in order to test the installed configuration
*/
+}
/* Track the args actually entered by the user.
* These will be used for the -k install parameters, as well as
@@ -1387,8 +1406,8 @@
/* Handle the following SCM aspects in this phase:
*
- * -k install
- * -k config
+ * -k install (catch and exit as install was handled in rewrite_args)
+ * -k config (catch and exit as config was handled in rewrite_args)
* -k start
* -k restart
* -k runservice [Win95, only once - after we parsed the config]
@@ -1399,18 +1418,23 @@
* We reached this phase by avoiding errors that would cause
* these options to fail unexpectedly in another process.
*/
-
if (!strcasecmp(signal_arg, "install")) {
-rv = mpm_service_install(ptemp, inst_argc, inst_argv, 0);
+/* Service install happens in the rewrite_args hooks. If we
+ * made it this far, the server configuration is clean and the
+ * service will successfully start.
+ */
apr_pool_destroy(s->process->pool);
apr_terminate();
-exit(rv);
+exit(0);
}
if (!strcasecmp(signal_arg, "config")) {
-rv = mpm_service_install(ptemp, inst_argc, inst_argv, 1);
+/* Service reconfiguration happens in the rewrite_args hooks. If we
+ * made it this far, the server configuration is clean and the
+ * service will successfully start.
+ */
apr_pool_destroy(s->process->pool);
apr_terminate();
-exit(rv);
+exit(0);
}
if (!strcasecmp(signal_arg, "start")) {
Re: [PATCH] Win32: install service in rewrite args rather than post_config?
At 09:24 AM 2/15/2005, Bill Stoddard wrote: >William A. Rowe, Jr. wrote: >>At 06:00 PM 2/12/2005, Bill Stoddard wrote: >> >>>If the last thing your installer does is install the service, the service >>>install will fail if there is a problem with httpd.conf. The most likely >>>problem is that some other service is using port 80 (or port 443, or >>>whatever port you want httpd to listen on). >> >>That and other issues. I have a couple theories on this... >>* Solve the port 80 (443...) problem simply by testing those >> ports in real_features.dll, a stub which our .msi invokes >> to test other real-world issues, today. Add the choice of >> ports (default: 80) and run the user in a circle till they >> choose a valid port. >>* If this is possible; Pipe the service install output to a log file. >>Present the user the results of that log file >> upon exit. Hopefully, this provides some feedback of why >> it did not install. [This is not as trivial in .msi as >> it might sound.] > >Sounds like complex solutions to a simple problem... Why not do this: >move the service install to pre_config (or rewrite_args). Then allow the >service install step to proceed through post_config as it does today. If >post_config fails, the service will still be installed but you will get a >clear error message saying your service will not start until you resolve the >issues with httpd.conf. I'll work up a patch. Now -that- is a solution I can buy :) Clever approach and it addresses both concerns. I'd even agree that -k config should unconditionally perform the same service reconfig, and drop out the same way as -k install. Service install must still fail if the service is already installed, so not every error case can result in installing the service. But for config errors, I agree. Bill
Re: [PATCH] Win32: install service in rewrite args rather than post_config?
William A. Rowe, Jr. wrote: At 06:00 PM 2/12/2005, Bill Stoddard wrote: Bill R correctly identified the reason I'd like to see the service install occur before post-config. If the last thing your installer does is install the service, the service install will fail if there is a problem with httpd.conf. The most likely problem is that some other service is using port 80 (or port 443, or whatever port you want httpd to listen on). That and other issues. I have a couple theories on this... * Solve the port 80 (443...) problem simply by testing those ports in real_features.dll, a stub which our .msi invokes to test other real-world issues, today. Add the choice of ports (default: 80) and run the user in a circle till they choose a valid port. * If this is possible; Pipe the service install output to a log file. Present the user the results of that log file upon exit. Hopefully, this provides some feedback of why it did not install. [This is not as trivial in .msi as it might sound.] Sounds like complex solutions to a simple problem... Why not do this: move the service install to pre_config (or rewrite_args). Then allow the service install step to proceed through post_config as it does today. If post_config fails, the service will still be installed but you will get a clear error message saying your service will not start until you resolve the issues with httpd.conf. I'll work up a patch. Bill
Re: [PATCH] Win32: install service in rewrite args rather than post_config?
At 06:00 PM 2/12/2005, Bill Stoddard wrote: > >Bill R correctly identified the reason I'd like to see >the service install occur before post-config. If the last >thing your installer does is install the service, the service >install will fail if there is a problem with httpd.conf. The >most likely problem is that some other service is using port 80 >(or port 443, or whatever port you want httpd to listen on). That and other issues. I have a couple theories on this... * Solve the port 80 (443...) problem simply by testing those ports in real_features.dll, a stub which our .msi invokes to test other real-world issues, today. Add the choice of ports (default: 80) and run the user in a circle till they choose a valid port. * If this is possible; Pipe the service install output to a log file. Present the user the results of that log file upon exit. Hopefully, this provides some feedback of why it did not install. [This is not as trivial in .msi as it might sound.] There are a few other changes I'm working on to the installer which will create other failure cases, such as running the service as another user. We can't safely move around a p/w for that account without the possibility of it being snooped. So if run-as-user is selected, we won't be starting the installed service, but taking them to the service control panel after creating that service for them to enter the p/w. We also need to axe (force to shutdown) the ApacheMonitor.exe taskbar applet on uninstall/upgrade/etc. Working on that. Finally the ApacheMonitor needs an extra option to dredge out the Application Event MMC view, just like it can bring up the Services view, or it's own expanded view. Not to mention the green light/red light indicator is broken in the taskbar. My theory is, if any service set to start 'Automatic' is not started, the light should never be green. Oh, and finally, we really shouldn't invert the icon color next to the service name in the quick view when it has mouse focus. The service name can be inverted, but the inversion changes red<>green lol. All this and more as I have cycles, help is always welcome. Bill
Re: [PATCH] Win32: install service in rewrite args rather than post_config?
From: "Bill Stoddard" Bill R correctly identified the reason I'd like to see the service install occur before post-config. And if there is an error here do what? Also what if one has never "read" the source code, "before post-config" means nothing to one who may know the answer to a simple question. If the last thing your installer does is install the service, the service install will fail if there is a problem with httpd.conf. Yes and you want to do what here? The most likely problem is that some other service is using port 80 (or port 443, or whatever port you want httpd to listen on). Yes and you want to do what here? You still have not said A This is happening to me (users) Port already used B And if A I (users) want to do this Tell Users - never install Tell Users - after Install Fix the problem, try again? C All A and B but for a developer Tell users/developer what and when, and then do what? So once again, what is the "real" problem (user or developer) and what to you want or "wish" to do about it? Jeff
Re: [PATCH] Win32: install service in rewrite args rather than post_config?
Jeff White wrote: From: "Bill Stoddard" It would, but is it worth the extra code to enable switching between both behaviours? Got some code to review? Bill What's the "real problem" Bill (Stoddard) wants to over come or what does he "really" want to happen? That's almost haiku :) Bill R correctly identified the reason I'd like to see the service install occur before post-config. If the last thing your installer does is install the service, the service install will fail if there is a problem with httpd.conf. The most likely problem is that some other service is using port 80 (or port 443, or whatever port you want httpd to listen on). Bill
Re: [PATCH] Win32: install service in rewrite args rather than post_config?
From: "Bill Stoddard" It would, but is it worth the extra code to enable switching between both behaviours? Got some code to review? Bill What's the "real problem" Bill (Stoddard) wants to over come or what does he "really" want to happen? Got some example ideas? Jeff
Re: [PATCH] Win32: install service in rewrite args rather than post_config?
At 02:49 PM 2/11/2005, Bill Stoddard wrote: >William A. Rowe, Jr. wrote: >>For manually installed services, we -should- keep the behavior. >>What good is it to have a user install a reference to an Apache >>service which won't work? > >Installing a service != starting a service. Users will figure out httpd.conf >has problems as soon as they attempt to start the service they just installed. >I guess I don't fully appreciate the value of the current behaviour :-) If you follow the users list - most users become confused because starting a service shoots early failures at the Windows event log. These are hard for more novice users to track down, since they only get a simple message such as 'The Apache2 service failed to start'. If that same failure happened during a service install, the error is given to them at the console. Bill
Re: [PATCH] Win32: install service in rewrite args rather than post_config?
William A. Rowe, Jr. wrote: For manually installed services, we -should- keep the behavior. What good is it to have a user install a reference to an Apache service which won't work? Installing a service != starting a service. Users will figure out httpd.conf has problems as soon as they attempt to start the service they just installed. I guess I don't fully appreciate the value of the current behaviour :-) For automated installs, e.g. the package installer, I see the benefit of offering more than one behavior. E.g. the win32 installer, or someone's external deployment scripts, should be able to override the tests. Would that satisfy your requirements? It would, but is it worth the extra code to enable switching between both behaviours? Got some code to review? Bill
Re: [PATCH] Win32: install service in rewrite args rather than post_config?
William A. Rowe, Jr. wrote: For manually installed services, we -should- keep the behavior. What good is it to have a user install a reference to an Apache service which won't work? For automated installs, e.g. the package installer, I see the benefit of offering more than one behavior. E.g. the win32 installer, or someone's external deployment scripts, should be able to override the tests. Would that satisfy your requirements? Indeed it would! Bill
Re: [PATCH] Win32: install service in rewrite args rather than post_config?
For manually installed services, we -should- keep the behavior.
What good is it to have a user install a reference to an Apache
service which won't work?
For automated installs, e.g. the package installer, I see the
benefit of offering more than one behavior. E.g. the win32
installer, or someone's external deployment scripts, should
be able to override the tests.
Would that satisfy your requirements?
Bill
At 11:57 AM 2/11/2005, Bill Stoddard wrote:
>If I try apache -k install -n foo, the install for service foo will fail if
>there are any problems in httpd.conf (invalid directives, syntax errors, or if
>the listener port is already active). While I see some value in failing the
>install of the service based on httpd.conf, overall I think's its more of a
>pain than a benefit. This patch moves the install of the service out of the
>post_config hook and into the rewrite_args hook (same hook where the -k
>uninstall code lives, Arguably service install/uninstall handling should be
>moved to the pre_config hook ). Comments?
>
>
>C:\home\apache\20\httpd-2.0\server\mpm\winnt>svn diff mpm_winnt.c
>svn diff mpm_winnt.c
>Index: mpm_winnt.c
>===
>--- mpm_winnt.c (revision 149412)
>+++ mpm_winnt.c (working copy)
>@@ -1070,7 +1070,7 @@
> /* Handle the following SCM aspects in this phase:
> *
> * -k runservice [transition for WinNT, nothing for Win9x]
>- * -k (!)install [error out if name is not installed]
>+ * -k install
> * -k uninstall
> * -k stop
> * -k shutdown (same as -k stop). Maintained for backward compatability.
>@@ -1279,6 +1279,12 @@
> "%s: Service is already installed.", service_name);
> exit(APEXIT_INIT);
> }
>+else
>+{
>+/* Install the service */
>+rv = mpm_service_install(process->pool, inst_argc, inst_argv, 0);
>+exit(rv);
>+}
> }
> else if (running_as_service)
> {
>@@ -1386,7 +1392,6 @@
>
> /* Handle the following SCM aspects in this phase:
> *
>- * -k install
> * -k config
> * -k start
> * -k restart
>@@ -1399,12 +1404,6 @@
> * these options to fail unexpectedly in another process.
> */
>
>-if (!strcasecmp(signal_arg, "install")) {
>-rv = mpm_service_install(ptemp, inst_argc, inst_argv, 0);
>-apr_pool_destroy(s->process->pool);
>-apr_terminate();
>-exit(rv);
>-}
> if (!strcasecmp(signal_arg, "config")) {
> rv = mpm_service_install(ptemp, inst_argc, inst_argv, 1);
> apr_pool_destroy(s->process->pool);
>
[PATCH] Win32: install service in rewrite args rather than post_config?
If I try apache -k install -n foo, the install for service foo will fail if there are any problems in
httpd.conf (invalid directives, syntax errors, or if the listener port is already active). While I see some
value in failing the install of the service based on httpd.conf, overall I think's its more of a pain than a
benefit. This patch moves the install of the service out of the post_config hook and into the rewrite_args
hook (same hook where the -k uninstall code lives, Arguably service install/uninstall handling should be moved
to the pre_config hook ). Comments?
C:\home\apache\20\httpd-2.0\server\mpm\winnt>svn diff mpm_winnt.c
svn diff mpm_winnt.c
Index: mpm_winnt.c
===
--- mpm_winnt.c (revision 149412)
+++ mpm_winnt.c (working copy)
@@ -1070,7 +1070,7 @@
/* Handle the following SCM aspects in this phase:
*
* -k runservice [transition for WinNT, nothing for Win9x]
- * -k (!)install [error out if name is not installed]
+ * -k install
* -k uninstall
* -k stop
* -k shutdown (same as -k stop). Maintained for backward compatability.
@@ -1279,6 +1279,12 @@
"%s: Service is already installed.", service_name);
exit(APEXIT_INIT);
}
+else
+{
+/* Install the service */
+rv = mpm_service_install(process->pool, inst_argc, inst_argv, 0);
+exit(rv);
+}
}
else if (running_as_service)
{
@@ -1386,7 +1392,6 @@
/* Handle the following SCM aspects in this phase:
*
- * -k install
* -k config
* -k start
* -k restart
@@ -1399,12 +1404,6 @@
* these options to fail unexpectedly in another process.
*/
-if (!strcasecmp(signal_arg, "install")) {
-rv = mpm_service_install(ptemp, inst_argc, inst_argv, 0);
-apr_pool_destroy(s->process->pool);
-apr_terminate();
-exit(rv);
-}
if (!strcasecmp(signal_arg, "config")) {
rv = mpm_service_install(ptemp, inst_argc, inst_argv, 1);
apr_pool_destroy(s->process->pool);
