You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by tr...@apache.org on 2009/10/06 01:58:34 UTC

svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Author: trawick
Date: Mon Oct  5 23:58:34 2009
New Revision: 822094

URL: http://svn.apache.org/viewvc?rev=822094&view=rev
Log:
consolidate/improve reporting of bogus files in the configuration

Modified:
    httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Modified: httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c
URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c?rev=822094&r1=822093&r2=822094&view=diff
==============================================================================
--- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c (original)
+++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c Mon Oct  5 23:58:34 2009
@@ -590,6 +590,16 @@
     return config->pass_headers;
 }
 
+static const char *missing_file_msg(apr_pool_t *p, const char *filetype, const char *filename,
+                                    apr_status_t rv)
+{
+    char errbuf[120];
+
+    apr_strerror(rv, errbuf, sizeof errbuf);
+    return apr_psprintf(p, "%s %s cannot be accessed: (%d)%s",
+                        filetype, filename, rv, errbuf);
+}
+
 const char *set_authenticator_info(cmd_parms * cmd, void *config,
                                    const char *authenticator)
 {
@@ -600,9 +610,7 @@
     /* Is the wrapper exist? */
     if ((rv = apr_stat(&finfo, authenticator, APR_FINFO_NORM,
                        cmd->temp_pool)) != APR_SUCCESS) {
-        return apr_psprintf(cmd->pool,
-                            "can't get authenticator file info: %s, errno: %d",
-                            authenticator, apr_get_os_error());
+        return missing_file_msg(cmd->pool, "Authenticator", authenticator, rv);
     }
 
     /* Create the wrapper node */
@@ -650,9 +658,7 @@
     /* Is the wrapper exist? */
     if ((rv = apr_stat(&finfo, authorizer, APR_FINFO_NORM,
                        cmd->temp_pool)) != APR_SUCCESS) {
-        return apr_psprintf(cmd->pool,
-                            "can't get authorizer file info: %s, errno: %d",
-                            authorizer, apr_get_os_error());
+        return missing_file_msg(cmd->pool, "Authorizer", authorizer, rv);
     }
 
     /* Create the wrapper node */
@@ -700,9 +706,7 @@
     /* Is the wrapper exist? */
     if ((rv = apr_stat(&finfo, access, APR_FINFO_NORM,
                        cmd->temp_pool)) != APR_SUCCESS) {
-        return apr_psprintf(cmd->pool,
-                            "can't get access checker file info: %s, errno: %d",
-                            access, apr_get_os_error());
+        return missing_file_msg(cmd->pool, "Access checker", access, rv);
     }
 
     /* Create the wrapper node */
@@ -816,9 +820,7 @@
     /* Does the wrapper exist? */
     if ((rv = apr_stat(&finfo, path, APR_FINFO_NORM,
                        cmd->temp_pool)) != APR_SUCCESS) {
-        return apr_psprintf(cmd->pool,
-                            "can't get FastCGI file info: '%s' (%s), errno: %d",
-                            wrapperpath, path, apr_get_os_error());
+        return missing_file_msg(cmd->pool, "Wrapper", path, rv);
     }
 
     apr_cpystrn(wrapper->args, wrapperpath, _POSIX_PATH_MAX);
@@ -930,9 +932,7 @@
 
     rv = apr_stat(&finfo, cmdname, APR_FINFO_NORM, cmd->temp_pool);
     if (rv != APR_SUCCESS) {
-        return apr_psprintf(cmd->pool,
-                            "File %s does not exist or cannot be accessed (error %d)",
-                            cmdname, rv);
+        return missing_file_msg(cmd->pool, "Command", cmdname, rv);
     }
 
     if (!*args) {



Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Tue, Oct 6, 2009 at 9:40 PM, William A. Rowe, Jr.
<wr...@rowe-clan.net> wrote:
> Roy T. Fielding wrote:
>> On Oct 6, 2009, at 1:00 PM, Jeff Trawick wrote:
>>
>>> On Tue, Oct 6, 2009 at 3:40 PM, Chris Darroch <ch...@pearsoncmg.com>
>>> wrote:
>>>>
>>>> Jeff Trawick wrote:
>>>>
>>>>> Beyond beta, I think we have something that is clearly better than
>>>>> the 2007 mod_fcgid 2.2 release and should get out the door soon as a
>>>>> GA (as long as testing doesn't show any regression).  I just made
>>>>> what I hope are uncontroversial changes to the directive names.
>>>>> I'll try to make peace with the rest.  It would be great if others
>>>>> would decide in the short term what they can't live with.
>>>>
>>>>  The directive name changes look great to me -- thanks very much!
>>>> Are there any you remain concerned about?
>>>
>>> Earlier I posted suggested changes to just about everything (thread
>>> "[mod_fcgid] Cleaning up configuration directive names").  I've gotten
>>> over that ;)  Here are a few from the original list that aren't so
>>> important to change but still might be considered an improvement by
>>> others:
>>>
>>> FCGIDOutputBufferSize -> FCGIDResponseBufferSize
>>>
>>> FCGIDBusyTimeout -> FCGIDRequestTimeout
>>> FCGIDBusyScanInterval -> FCGIDRequestTimeoutScanInterval (unfortunate
>>> name for unfortunate concept)
>>
>> Can I make a last-minute plea for readability?  Those names suck.
>> If we just lowercase the cgid, it would be more readable.
>>
>>  FcgidResponseBufferSize
>>  FcgidRequestTimeout
>>  FcgidRequestTimeoutScanInterval
>>
>> perl -pi -e 's/FCGID/Fcgid/g;' files...
>
> +1.  I'm still working on the final solution to directives.html.xx so if someone wants to
> commit this change in the next hour or two, it will be in the coming release.

on it

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Roy T. Fielding wrote:
> On Oct 6, 2009, at 1:00 PM, Jeff Trawick wrote:
> 
>> On Tue, Oct 6, 2009 at 3:40 PM, Chris Darroch <ch...@pearsoncmg.com>
>> wrote:
>>>
>>> Jeff Trawick wrote:
>>>
>>>> Beyond beta, I think we have something that is clearly better than
>>>> the 2007 mod_fcgid 2.2 release and should get out the door soon as a
>>>> GA (as long as testing doesn't show any regression).  I just made
>>>> what I hope are uncontroversial changes to the directive names. 
>>>> I'll try to make peace with the rest.  It would be great if others
>>>> would decide in the short term what they can't live with.
>>>
>>>  The directive name changes look great to me -- thanks very much!
>>> Are there any you remain concerned about?
>>
>> Earlier I posted suggested changes to just about everything (thread
>> "[mod_fcgid] Cleaning up configuration directive names").  I've gotten
>> over that ;)  Here are a few from the original list that aren't so
>> important to change but still might be considered an improvement by
>> others:
>>
>> FCGIDOutputBufferSize -> FCGIDResponseBufferSize
>>
>> FCGIDBusyTimeout -> FCGIDRequestTimeout
>> FCGIDBusyScanInterval -> FCGIDRequestTimeoutScanInterval (unfortunate
>> name for unfortunate concept)
> 
> Can I make a last-minute plea for readability?  Those names suck.
> If we just lowercase the cgid, it would be more readable.
> 
>  FcgidResponseBufferSize
>  FcgidRequestTimeout
>  FcgidRequestTimeoutScanInterval
> 
> perl -pi -e 's/FCGID/Fcgid/g;' files...

+1.  I'm still working on the final solution to directives.html.xx so if someone wants to
commit this change in the next hour or two, it will be in the coming release.

(The .c sources don't matter, directives are case insensitive)

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Oct 6, 2009, at 1:00 PM, Jeff Trawick wrote:

> On Tue, Oct 6, 2009 at 3:40 PM, Chris Darroch  
> <ch...@pearsoncmg.com> wrote:
>>
>> Jeff Trawick wrote:
>>
>>> Beyond beta, I think we have something that is clearly better  
>>> than the 2007 mod_fcgid 2.2 release and should get out the door  
>>> soon as a GA (as long as testing doesn't show any regression).  I  
>>> just made what I hope are uncontroversial changes to the  
>>> directive names.  I'll try to make peace with the rest.  It would  
>>> be great if others would decide in the short term what they can't  
>>> live with.
>>
>>  The directive name changes look great to me -- thanks very much!
>> Are there any you remain concerned about?
>
> Earlier I posted suggested changes to just about everything (thread
> "[mod_fcgid] Cleaning up configuration directive names").  I've gotten
> over that ;)  Here are a few from the original list that aren't so
> important to change but still might be considered an improvement by
> others:
>
> FCGIDOutputBufferSize -> FCGIDResponseBufferSize
>
> FCGIDBusyTimeout -> FCGIDRequestTimeout
> FCGIDBusyScanInterval -> FCGIDRequestTimeoutScanInterval (unfortunate
> name for unfortunate concept)

Can I make a last-minute plea for readability?  Those names suck.
If we just lowercase the cgid, it would be more readable.

  FcgidResponseBufferSize
  FcgidRequestTimeout
  FcgidRequestTimeoutScanInterval

perl -pi -e 's/FCGID/Fcgid/g;' files...

....Roy

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Tue, Oct 6, 2009 at 4:00 PM, Jeff Trawick <tr...@gmail.com> wrote:
> FCGIDBusyTimeout -> FCGIDRequestTimeout
> FCGIDBusyScanInterval -> FCGIDRequestTimeoutScanInterval (unfortunate
> name for unfortunate concept)

Maybe just plain FCGIDRequestScanInterval isn't so bad.

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Oct 7, 2009 at 10:05 AM, Chris Darroch <ch...@pearsoncmg.com> wrote:
> Jeff Trawick wrote:
>
>> FCGIDOutputBufferSize -> FCGIDResponseBufferSize
>>
>> FCGIDBusyTimeout -> FCGIDRequestTimeout
>> FCGIDBusyScanInterval -> FCGIDRequestTimeoutScanInterval (unfortunate
>> name for unfortunate concept)
>
>  Hmm ... can't say I have any opinion on those, really.  I suppose
> "Response" is somewhat more specific than "Output".
>
>  If we're thinking about axing or replacing the "busy scan" at
> some point, maybe we should just leave those two named as-is?

works for me

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by Chris Darroch <ch...@pearsoncmg.com>.
Jeff Trawick wrote:

> FCGIDOutputBufferSize -> FCGIDResponseBufferSize
> 
> FCGIDBusyTimeout -> FCGIDRequestTimeout
> FCGIDBusyScanInterval -> FCGIDRequestTimeoutScanInterval (unfortunate
> name for unfortunate concept)

   Hmm ... can't say I have any opinion on those, really.  I suppose
"Response" is somewhat more specific than "Output".

   If we're thinking about axing or replacing the "busy scan" at
some point, maybe we should just leave those two named as-is?

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B


Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Tue, Oct 6, 2009 at 3:40 PM, Chris Darroch <ch...@pearsoncmg.com> wrote:
>
> Jeff Trawick wrote:
>
>> Beyond beta, I think we have something that is clearly better than the 2007 mod_fcgid 2.2 release and should get out the door soon as a GA (as long as testing doesn't show any regression).  I just made what I hope are uncontroversial changes to the directive names.  I'll try to make peace with the rest.  It would be great if others would decide in the short term what they can't live with.
>
>  The directive name changes look great to me -- thanks very much!
> Are there any you remain concerned about?

Earlier I posted suggested changes to just about everything (thread
"[mod_fcgid] Cleaning up configuration directive names").  I've gotten
over that ;)  Here are a few from the original list that aren't so
important to change but still might be considered an improvement by
others:

FCGIDOutputBufferSize -> FCGIDResponseBufferSize

FCGIDBusyTimeout -> FCGIDRequestTimeout
FCGIDBusyScanInterval -> FCGIDRequestTimeoutScanInterval (unfortunate
name for unfortunate concept)

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by Chris Darroch <ch...@pearsoncmg.com>.
Jeff Trawick wrote:

> Beyond beta, I think we have something that is clearly better than the 
> 2007 mod_fcgid 2.2 release and should get out the door soon as a GA (as 
> long as testing doesn't show any regression).  I just made what I hope 
> are uncontroversial changes to the directive names.  I'll try to make 
> peace with the rest.  It would be great if others would decide in the 
> short term what they can't live with.

   The directive name changes look great to me -- thanks very much!
Are there any you remain concerned about?

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B


Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by Rainer Jung <ra...@kippdata.de>.
On 06.10.2009 22:31, William A. Rowe, Jr. wrote:
> Rainer Jung wrote:
>>
>>>> Another thing is being able to split INSTDIR from APACHE2_HOME. The
>>>> first is where the module should go to, the other one where httpd is. A
>>>> simple tweak to the Makefile allows that. Then there's also an unconditional
>>>>
>>>> INSTDIR=\Apache22
>>>>
>>>> near the beginning of both Makefiles, which seems not right.
>>> That's a default.  If INSTDIR is provided, that value is overridden.
>>
>> Hmmm, it is applied a little down, but only if INSTDIR is not yet set.
>> So setting the default is in the Makefile twice, once unconditionally,
>> and then later down only if INSTDIR is not yet set. The unconditional
>> one should go. Furthermore the test if it is already set should also be
>> applied to APACHE2_HOME around the line, were APACHE2_HOME is set to
>> INSTDIR.
> 
> Understand that if it's provided on the command line, the command line overrides.
> It really is just a default.  But as you point out, bringing it in from the environment
> requires that we protect it in an empty value test.
> 
> Right now we have it to initialize INSTDIR twice, and that's obviously wrong,
> so I've adopted your suggestions, including reporting the APACHE2_HOME value.
> 
> I think all appropriate changes are now made, please review :)

Yup, works.

I think you can remove the INSTDIR and APACHE2_HOME options when
recursively calling nmake. Now that we don't set them undconditionally
the passing of vars works automatically (at least during my test).

Regards,

Rainer

Index: Makefile-fcgid.win
===================================================================
--- Makefile-fcgid.win  (Revision 822505)
+++ Makefile-fcgid.win  (Arbeitskopie)
@@ -82,22 +82,18 @@

 _buildr:
        @$(MAKE) $(MAKEOPT) -f Makefile-fcgid.win \
-               INSTDIR="$(INSTDIR)" APACHE2_HOME="$(APACHE2_HOME)" \
                SHORT=R LONG=Release _build

 _buildd:
        @$(MAKE) $(MAKEOPT) -f Makefile-fcgid.win \
-               INSTDIR="$(INSTDIR)" APACHE2_HOME="$(APACHE2_HOME)" \
                SHORT=D LONG=Debug   _build

 installr:
        @$(MAKE) $(MAKEOPT) -f Makefile-fcgid.win \
-               INSTDIR="$(INSTDIR)" APACHE2_HOME="$(APACHE2_HOME)" \
                SHORT=R LONG=Release _build _install

 installd:
        @$(MAKE) $(MAKEOPT) -f Makefile-fcgid.win \
-               INSTDIR="$(INSTDIR)" APACHE2_HOME="$(APACHE2_HOME)" \
                SHORT=D LONG=Debug   _build _install

 clean: _cleanr _cleand
@@ -109,12 +105,10 @@

 _cleanr:
        $(MAKE) $(MAKEOPT) -f Makefile-fcgid.win \
-               INSTDIR="$(INSTDIR)" APACHE2_HOME="$(APACHE2_HOME)" \
                SHORT=R LONG=Release CTARGET=CLEAN _build

 _cleand:
        $(MAKE) $(MAKEOPT) -f Makefile-fcgid.win \
-               INSTDIR="$(INSTDIR)" APACHE2_HOME="$(APACHE2_HOME)" \
                SHORT=D LONG=Debug   CTARGET=CLEAN _build

 _build:
@@ -127,12 +121,10 @@

 _cleanr:
        $(MAKE) $(MAKEOPT) -f Makefile-fcgid.win \
-               INSTDIR="$(INSTDIR)" APACHE2_HOME="$(APACHE2_HOME)" \
                SHORT=R LONG=Release CTARGET="/clean" _build

 _cleand:
        $(MAKE) $(MAKEOPT) -f Makefile-fcgid.win \
-               INSTDIR="$(INSTDIR)" APACHE2_HOME="$(APACHE2_HOME)" \
                SHORT=D LONG=Debug   CTARGET="/clean" _build

 _build:
@@ -143,12 +135,10 @@

 _cleanr:
        @$(MAKE) $(MAKEOPT) -f Makefile-fcgid.win \
-               INSTDIR="$(INSTDIR)" APACHE2_HOME="$(APACHE2_HOME)" \
                SHORT=R LONG=Release CTARGET="/CLEAN" _build

 _cleand:
        @$(MAKE) $(MAKEOPT) -f Makefile-fcgid.win \
-               INSTDIR="$(INSTDIR)" APACHE2_HOME="$(APACHE2_HOME)" \
                SHORT=D LONG=Debug   CTARGET="/CLEAN" _build

 _build:

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Rainer Jung wrote:
> 
>>> Another thing is being able to split INSTDIR from APACHE2_HOME. The
>>> first is where the module should go to, the other one where httpd is. A
>>> simple tweak to the Makefile allows that. Then there's also an unconditional
>>>
>>> INSTDIR=\Apache22
>>>
>>> near the beginning of both Makefiles, which seems not right.
>> That's a default.  If INSTDIR is provided, that value is overridden.
> 
> Hmmm, it is applied a little down, but only if INSTDIR is not yet set.
> So setting the default is in the Makefile twice, once unconditionally,
> and then later down only if INSTDIR is not yet set. The unconditional
> one should go. Furthermore the test if it is already set should also be
> applied to APACHE2_HOME around the line, were APACHE2_HOME is set to
> INSTDIR.

Understand that if it's provided on the command line, the command line overrides.
It really is just a default.  But as you point out, bringing it in from the environment
requires that we protect it in an empty value test.

Right now we have it to initialize INSTDIR twice, and that's obviously wrong,
so I've adopted your suggestions, including reporting the APACHE2_HOME value.

I think all appropriate changes are now made, please review :)

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by Rainer Jung <ra...@kippdata.de>.
On 06.10.2009 20:11, William A. Rowe, Jr. wrote:
>> Both Makefiles check against httpd.vcproj in the main directory which is
>> not present when building out of tree.
> 
> 
> Now I see what you mean, both for .vcproj and .mak tests, something like the last
> commit, right?

Yes, I missed the .mak one.

>> I suggest we check against modules/fcgid/mod_fcgid.vcproj resp.
>> modules/ftp/mod_ftp.vcproj?

That works now with your change.

>> Another thing is being able to split INSTDIR from APACHE2_HOME. The
>> first is where the module should go to, the other one where httpd is. A
>> simple tweak to the Makefile allows that. Then there's also an unconditional
>>
>> INSTDIR=\Apache22
>>
>> near the beginning of both Makefiles, which seems not right.
> 
> That's a default.  If INSTDIR is provided, that value is overridden.

Hmmm, it is applied a little down, but only if INSTDIR is not yet set.
So setting the default is in the Makefile twice, once unconditionally,
and then later down only if INSTDIR is not yet set. The unconditional
one should go. Furthermore the test if it is already set should also be
applied to APACHE2_HOME around the line, were APACHE2_HOME is set to
INSTDIR.

>> Finally the install target has a problem if INSTDIR!=APACHE2_HOME. There
>> is a test against "COMPLETED" which is never set. I think we can simply
>> remove the check, otherwise the xcopy for the manual fails due to the
>> missing target directory.
> 
> Looking.

See above.

>> And last but not least, there is no mod_fcgid.h to install. Unless we
>> want to install other headers, we can drop that part.
> 
> Until we have one.  Right now, it's a noop, so don't worry yourself about it :)
> It's called a template ;-)  And given mod_cgi/mod_cgid history, we should likely
> expect one to arrive, one day.

OK, now I understand "COMPLETED". We could remove the mkdir for manual
form COMPLETED, because xcopy does it (at least on my version of windows).

Regards,

Rainer

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Rainer Jung wrote:
> 
> I used trunk and the modules/ftp/*.mak file. The Makefile*-win for fcgid
> and for ftp first did not work for me. It turns out the reason is, that
> I build out of tree.

Then you have to use the .mak file accordingly (current path, don't do something
as absurd as build-clean-build).  IOW you are on your own :)

> Both Makefiles check against httpd.vcproj in the main directory which is
> not present when building out of tree.


Now I see what you mean, both for .vcproj and .mak tests, something like the last
commit, right?

> I suggest we check against modules/fcgid/mod_fcgid.vcproj resp.
> modules/ftp/mod_ftp.vcproj?
> 
> Another thing is being able to split INSTDIR from APACHE2_HOME. The
> first is where the module should go to, the other one where httpd is. A
> simple tweak to the Makefile allows that. Then there's also an unconditional
> 
> INSTDIR=\Apache22
> 
> near the beginning of both Makefiles, which seems not right.

That's a default.  If INSTDIR is provided, that value is overridden.

> Finally the install target has a problem if INSTDIR!=APACHE2_HOME. There
> is a test against "COMPLETED" which is never set. I think we can simply
> remove the check, otherwise the xcopy for the manual fails due to the
> missing target directory.

Looking.

> And last but not least, there is no mod_fcgid.h to install. Unless we
> want to install other headers, we can drop that part.

Until we have one.  Right now, it's a noop, so don't worry yourself about it :)
It's called a template ;-)  And given mod_cgi/mod_cgid history, we should likely
expect one to arrive, one day.

> I prepared a little patch you can find at
> 
> http://people.apache.org/~rjung/patches/Makefiles-fcgid.patch
> 
> What do you think about it?

Compare to the last two commits, and see if they satisfy?



Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by Rainer Jung <ra...@kippdata.de>.
On 06.10.2009 18:26, William A. Rowe, Jr. wrote:
> Rainer Jung wrote:
>> On 06.10.2009 14:56, Jeff Trawick wrote:
>>> (hoping that includes building on Windows to see the more obvious
>>> Jeff-breakage :( )
>>
>> Neither obvious nor non-obvious: I tried building on Windows right now
>> (against 2.2.14). It succeeds without errors or warning :)
>>
>> I tried the devenv method and also the .mak file. Both methods worked,
>> for the .mak method I first had to enter the modules/fcgid directory,
>> otherwise the makefile complains about not finding the .dep file.
> 
> You were building Makefile-fcgid.win I trust?  Oh - you cannot invoke an
> msvcrt exported makefile from another directory, but the top level makefile
> should handle this for you.  There is an issue with passing the variables
> to the lower-level invocation of Makefile-fcgid.win which I'll look for fixes.

I used trunk and the modules/ftp/*.mak file. The Makefile*-win for fcgid
and for ftp first did not work for me. It turns out the reason is, that
I build out of tree.

Both Makefiles check against httpd.vcproj in the main directory which is
not present when building out of tree.

I suggest we check against modules/fcgid/mod_fcgid.vcproj resp.
modules/ftp/mod_ftp.vcproj?

Another thing is being able to split INSTDIR from APACHE2_HOME. The
first is where the module should go to, the other one where httpd is. A
simple tweak to the Makefile allows that. Then there's also an unconditional

INSTDIR=\Apache22

near the beginning of both Makefiles, which seems not right.

Finally the install target has a problem if INSTDIR!=APACHE2_HOME. There
is a test against "COMPLETED" which is never set. I think we can simply
remove the check, otherwise the xcopy for the manual fails due to the
missing target directory.

And last but not least, there is no mod_fcgid.h to install. Unless we
want to install other headers, we can drop that part.

I prepared a little patch you can find at

http://people.apache.org/~rjung/patches/Makefiles-fcgid.patch

What do you think about it?

Regards,

Rainer

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Rainer Jung wrote:
> On 06.10.2009 14:56, Jeff Trawick wrote:
>> On Tue, Oct 6, 2009 at 4:07 AM, William A. Rowe, Jr.
>> <wrowe@rowe-clan.net <ma...@rowe-clan.net>> wrote:
>>
>>     trawick@apache.org <ma...@apache.org> wrote:
>>     > Author: trawick
>>     > Date: Mon Oct  5 23:58:34 2009
>>     > New Revision: 822094
>>     >
>>     > URL: http://svn.apache.org/viewvc?rev=822094&view=rev
>>     <http://svn.apache.org/viewvc?rev=822094&view=rev>
>>     > Log:
>>     > consolidate/improve reporting of bogus files in the configuration
>>
>>     My quick evaluation of the state of the code
>>
>>
>> (hoping that includes building on Windows to see the more obvious
>> Jeff-breakage :( )
> 
> Neither obvious nor non-obvious: I tried building on Windows right now
> (against 2.2.14). It succeeds without errors or warning :)
> 
> I tried the devenv method and also the .mak file. Both methods worked,
> for the .mak method I first had to enter the modules/fcgid directory,
> otherwise the makefile complains about not finding the .dep file.

You were building Makefile-fcgid.win I trust?  Oh - you cannot invoke an
msvcrt exported makefile from another directory, but the top level makefile
should handle this for you.  There is an issue with passing the variables
to the lower-level invocation of Makefile-fcgid.win which I'll look for fixes.

>>     suggests we are ready for another beta
>>     candidate right now?  Unless I hear otherwise, I'll roll this in a
>>     half day and see
>>     if we aren't more successful this round.
>>
>>
>> +1
>>
>> Beyond beta, I think we have something that is clearly better than the
>> 2007 mod_fcgid 2.2 release and should get out the door soon as a GA (as
>> long as testing doesn't show any regression).  I just made what I hope
>> are uncontroversial changes to the directive names.  I'll try to make
>> peace with the rest.  It would be great if others would decide in the
>> short term what they can't live with.
> 
> +1

Agreed, putting one final beta out there for users to review the directives
would go a long ways into pacifying that final list of renames or catching any
that should be changed one last time.

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by Rainer Jung <ra...@kippdata.de>.
On 06.10.2009 14:56, Jeff Trawick wrote:
> On Tue, Oct 6, 2009 at 4:07 AM, William A. Rowe, Jr.
> <wrowe@rowe-clan.net <ma...@rowe-clan.net>> wrote:
> 
>     trawick@apache.org <ma...@apache.org> wrote:
>     > Author: trawick
>     > Date: Mon Oct  5 23:58:34 2009
>     > New Revision: 822094
>     >
>     > URL: http://svn.apache.org/viewvc?rev=822094&view=rev
>     <http://svn.apache.org/viewvc?rev=822094&view=rev>
>     > Log:
>     > consolidate/improve reporting of bogus files in the configuration
> 
>     My quick evaluation of the state of the code
> 
> 
> (hoping that includes building on Windows to see the more obvious
> Jeff-breakage :( )

Neither obvious nor non-obvious: I tried building on Windows right now
(against 2.2.14). It succeeds without errors or warning :)

I tried the devenv method and also the .mak file. Both methods worked,
for the .mak method I first had to enter the modules/fcgid directory,
otherwise the makefile complains about not finding the .dep file.

I quickly loaded the module, but did not do intensive testing.

>     suggests we are ready for another beta
>     candidate right now?  Unless I hear otherwise, I'll roll this in a
>     half day and see
>     if we aren't more successful this round.
> 
> 
> +1
> 
> Beyond beta, I think we have something that is clearly better than the
> 2007 mod_fcgid 2.2 release and should get out the door soon as a GA (as
> long as testing doesn't show any regression).  I just made what I hope
> are uncontroversial changes to the directive names.  I'll try to make
> peace with the rest.  It would be great if others would decide in the
> short term what they can't live with.

+1

Regards,

Rainer

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Tue, Oct 6, 2009 at 4:07 AM, William A. Rowe, Jr. <wr...@rowe-clan.net>wrote:

> trawick@apache.org wrote:
> > Author: trawick
> > Date: Mon Oct  5 23:58:34 2009
> > New Revision: 822094
> >
> > URL: http://svn.apache.org/viewvc?rev=822094&view=rev
> > Log:
> > consolidate/improve reporting of bogus files in the configuration
>
> My quick evaluation of the state of the code


(hoping that includes building on Windows to see the more obvious
Jeff-breakage :( )


> suggests we are ready for another beta
> candidate right now?  Unless I hear otherwise, I'll roll this in a half day
> and see
> if we aren't more successful this round.
>

+1

Beyond beta, I think we have something that is clearly better than the 2007
mod_fcgid 2.2 release and should get out the door soon as a GA (as long as
testing doesn't show any regression).  I just made what I hope are
uncontroversial changes to the directive names.  I'll try to make peace with
the rest.  It would be great if others would decide in the short term what
they can't live with.

Re: svn commit: r822094 - /httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
trawick@apache.org wrote:
> Author: trawick
> Date: Mon Oct  5 23:58:34 2009
> New Revision: 822094
> 
> URL: http://svn.apache.org/viewvc?rev=822094&view=rev
> Log:
> consolidate/improve reporting of bogus files in the configuration

My quick evaluation of the state of the code suggests we are ready for another beta
candidate right now?  Unless I hear otherwise, I'll roll this in a half day and see
if we aren't more successful this round.