Mailing-List: contact new-httpd-help@apache.org; run by ezmlm
Precedence: bulk
Reply-To: new-httpd@apache.org
Date: Sat, 27 May 2000 15:04:06 -0700 (PDT)
From: Greg Stein <gstein@lyra.org>
To: new-httpd@apache.org
Subject: RE: cvs commit: apache-2.0/src/modules/standard
 mod_access.cmod_alias.c
 mod_asis.c mod_auth.c mod_auth_anon.c mod_auth_digest.cmod_autoindex.c
 mod_cern_meta.c mod_cgi.c mod_cgid.c mod_dir.c mod_echo.cmod_env.c
 mod_expires.c
 mod_file_cache.c mod_headers.c
In-Reply-To: <000501bfc825$70a4fff0$345985d0@corecomm.net>
Message-ID: <Pine.LNX.4.10.10005271457060.32359-100000@nebula.lyra.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII

I'm going to preface this with a big simplifying factor. One that should
have been done INSTEAD of all this AP_EXPORT vs API_EXPORT linkage crap.

CHANGE THE .DSP!!!

We've already said that ap/ "should not exist and should be part of
main/". Great. So make it that way!

On Sat, 27 May 2000, William A. Rowe, Jr. wrote:
> > On Fri, 26 May 2000 rbb@covalent.net wrote:
> > >...
> > > On 27 May 2000 wrowe@locus.apache.org wrote:
> > >...
> > > >      /* Hooks */
> > > >   -AP_DECLARE_HOOK(int,header_parser,(request_rec *))
> > > >   -AP_DECLARE_HOOK(void,post_config,
> > > >   +AP_DECLARE_HOOK(API_EXPORT,int,header_parser,(request_rec *))
> > > >   +AP_DECLARE_HOOK(API_EXPORT,void,post_config,
> > > >    	     (ap_pool_t *pconf,ap_pool_t 
> > *plog,ap_pool_t *ptemp,server_rec *s))
> > > >   -AP_DECLARE_HOOK(void,open_logs,
> > > >   +AP_DECLARE_HOOK(API_EXPORT,void,open_logs,
> > > >    	     (ap_pool_t *pconf,ap_pool_t 
> > *plog,ap_pool_t *ptemp,server_rec *s))
> > > >   -AP_DECLARE_HOOK(void,child_init,(ap_pool_t *pchild, 
> > server_rec *s))
> > > >   +AP_DECLARE_HOOK(API_EXPORT,void,child_init,(ap_pool_t 
> > *pchild, server_rec *s))
> > > 
> > > Every single hook is being declared API_EXPORT.  This seems silly to
> > > me.  Can't we just use API_EXPORT in the macro definition?
> > 
> > That's what I said, too. Consider me a "-1" on this part of 
> > the commit.
> > Until we have a *need* for a different linkage, this 
> > parameter should not
> > be included. *When* that need arises, then we can add a second set of
> > macros (but leave these alone!).
> 
> No... AP is a library that is not dependent on the apache core server.

YOU made the AP library. You don't have to go and screw with the rest of
the server to resolve that problem.

> API_EXPORT_bleh are defined in the core server, but the library exists
> before the core.  I've had more problems than I care for trying to take
> these eggs away from the chicken.  Since *today* it's a library function,
> it shouldn'd be defined in terms of the server.

"today" is because of the .dsp. Not because of anything else.

> If ap_hooks.c is moved to main and is compiled into the core, then I 
> don't care, it becomes an argument of 'when' there is a need, and since
> I have enough nays, I'll reverse those args out.

Those extra args should be reversed out. There are a couple "-1" votes
against that change already.

You can move the ap_hooks.c simply be updating the .dsp. Or you could add
ap_hooks.c into src/main/ and delete it from src/ap/.

> > > >   -#ifndef MODULE_VAR_EXPORT
> > > >   -#define MODULE_VAR_EXPORT
> > > >   +#ifndef MODULE_EXPORT_VAR
> > > >   +#define MODULE_EXPORT_VAR
> > > >    #endif
> > > >   -#ifndef API_VAR_EXPORT
> > > >   -#define API_VAR_EXPORT
> > > >   +#ifndef API_EXPORT_VAR
> > > >   +#define API_EXPORT_VAR
> > > >    #endif
> > > 
> > > These are name changes for the sake of name changes, blech.
> > 
> > Agreed.
> 
> As for EXPORT_VAR vs. VAR_EXPORT, it is in the schema of module_EXPORT, 
> and happens to be data instead of a function.  Easier to GREP.  But if
> I get the nays I'll flip *_EXPORT_VAR back to *_VAR_EXPORT under all
> of APR, AP, and API.  I don't care enough to argue this point.

There have been a couple "nays" already, stating that this was a
gratuitous change (done without warning).

I can see why you did it, but if the change is going to occur, then it
should have at least introduced some namespace protection.

> > If we're going to change the names, then it really should be 
> > to something
> > like:
> > 
> >   AP_MODULE_EXPORT_VAR
> >   AP_EXPART_VAR
> > 
> > i.e. namespace protection
> 
> API_ was 'because it's there', and was the path of least resistance
> (fewest changes).  APR_ because that was intuitive.  AP_ because 
> that's intuitive as well, in that it is the ap library.  What prefix 
> would you propose for:
> 
>   The APR library package ________

APR_

>   The AP library package  ________

Never heard of that library :-)

>   The server core binary  ________

AP_

>...
> > > I am basically -1 for this whole commit unless somebody can 
> > > explain why the name change was necessary.
> > 
> > I certainly can't explain...
> 
> Which name change?  VAR_EXPORT <-> EXPORT_VAR?  AP_ vs API_ vs APR_ ?  

The VAR_EXPORT to EXPORT_VAR thing.

You explained why, but I think it should have been done with an AP_
prefix.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/