You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4cxx-dev@logging.apache.org by Christophe de VIENNE <cd...@alphacent.com> on 2004/08/12 19:51:20 UTC
Re: [PATCH] Fix global namespace pollution (config_auto.h)
Cesar Eduardo Barros wrote:
>On Thu, Jul 08, 2004 at 05:32:37PM -0300, Cesar Eduardo Barros wrote:
>
>
>>log4cxx is including a copy of the autoconf-generated config_auto.h even
>>when compiling another program which also uses autoconf, causing
>>conflicts. However, only LOG4CXX_EXPORT is really needed. I made this
>>quick patch to fix it.
>>
>>This patch reuses the LOG4CXX define used for almost the same purpose on
>>config_msvc.h (to determine if we're compiling log4cxx itself or a
>>program which uses log4cxx).
>>
>>
>
>I didn't notice some headers were using the defines from config_auto.h
>(HAVE_*, MUST_* and UNICODE). So, the simple solution above didn't work
>when I tried to use the DOM configurator.
>
>
I was trying to find a nice solution to this problem, it seems I don't
have to anymore :-) Thanks !
>This patch renames these defines on all headers and adds a sed-generated
>copy of config_auto.h prepending LOG4CXX_ to all these defines and
>removing the bogus PACKAGE_*, VERSION and STDC_HEADERS ones when not
>being included within the log4cxx sources.
>
>
This patch solves efficiently the macro problem. The idea to use sed is
very good : nice job :-)
>Tested compiling in the source directory, but should work with a
>separate object directory too.
>
>
No, but I fixed it (it was a previous issue).
I also had to changed macros names in src/threadspecificdata.cpp.
I'm reviewing src/*.cpp to check is there is no forgotten macro renaming.
Apart from this, I think the patch can be commited. Does everybody agree ?
Christophe
Re: [PATCH] Fix global namespace pollution (config_auto.h)
Posted by Curt Arnold <ca...@houston.rr.com>.
On Aug 12, 2004, at 3:53 PM, c wrote:
> I agree with you, Curt. But I think it's a good short term solution to
> help developers who are blocked with the current implementation.
> The solution you refer to is surely the best one, but much work will
> be needed to achieve it. Consequently, we could deliver it later.
>
> Regards,
>
I'll vote a +0.
I did more investigation that I can justify at the moment. The hardest
problem seemed to be the dependency on helper/CriticalSection.h on
config.h to define HAVE_PTHREAD. I don't see a cheap way to get that
out of the header file.
I was a bit troubled by CriticalSection having its mutex and
owningThread members public. Since there are accessors for them, I
assume it was just an oversight to not add a "private:" before the
#ifdef HAVE_PTHREAD. If the patch touches, criticalsection.h could you
consider changing the visibility of those members.
Also, Christophe is not listed on the
http://logging.apache.org/log4cxx/team.html. I assume that he should
be.
The AUTHORS file lists Michaël, but with a users.sourceforge.net
domain. Should those addresses be updated? Should Christophe and I be
added?
Re: [PATCH] Fix global namespace pollution (config_auto.h)
Posted by Christophe de Vienne <cd...@alphacent.com>.
Michaël CATANZARITI wrote:
> I agree with you, Curt. But I think it's a good short term solution to
> help developers who are blocked with the current implementation.
I'll commit it tomorrow (my working copy is at my office).
Regards,
Christophe
Re: [PATCH] Fix global namespace pollution (config_auto.h)
Posted by Michaël CATANZARITI <mc...@apache.org>.
I agree with you, Curt. But I think it's a good short term solution to help developers who are blocked with the current implementation.
The solution you refer to is surely the best one, but much work will be needed to achieve it. Consequently, we could deliver it later.
Regards,
Curt Arnold wrote:
> I'm uncomfortable with that solution, but I'm on deadline on something
> else and can't research it now. It is a problem that needs to be fixed,
> but I'd like to fix it by eliminating implementation details (like the
> config_auto.h) from the expansions of the API include files like logger.h.
>
>
> On Aug 12, 2004, at 12:51 PM, Christophe de VIENNE wrote:
>
>> Cesar Eduardo Barros wrote:
>>
>>> On Thu, Jul 08, 2004 at 05:32:37PM -0300, Cesar Eduardo Barros wrote:
>>>
>>>> log4cxx is including a copy of the autoconf-generated config_auto.h
>>>> even
>>>> when compiling another program which also uses autoconf, causing
>>>> conflicts. However, only LOG4CXX_EXPORT is really needed. I made this
>>>> quick patch to fix it.
>>>>
>>>> This patch reuses the LOG4CXX define used for almost the same
>>>> purpose on
>>>> config_msvc.h (to determine if we're compiling log4cxx itself or a
>>>> program which uses log4cxx).
>>>>
>>>
>>> I didn't notice some headers were using the defines from config_auto.h
>>> (HAVE_*, MUST_* and UNICODE). So, the simple solution above didn't work
>>> when I tried to use the DOM configurator.
>>>
>>
>> I was trying to find a nice solution to this problem, it seems I don't
>> have to anymore :-) Thanks !
>>
>>> This patch renames these defines on all headers and adds a sed-generated
>>> copy of config_auto.h prepending LOG4CXX_ to all these defines and
>>> removing the bogus PACKAGE_*, VERSION and STDC_HEADERS ones when not
>>> being included within the log4cxx sources.
>>>
>>
>> This patch solves efficiently the macro problem. The idea to use sed
>> is very good : nice job :-)
>>
>>
>>> Tested compiling in the source directory, but should work with a
>>> separate object directory too.
>>>
>>
>> No, but I fixed it (it was a previous issue).
>> I also had to changed macros names in src/threadspecificdata.cpp.
>> I'm reviewing src/*.cpp to check is there is no forgotten macro renaming.
>>
>> Apart from this, I think the patch can be commited. Does everybody
>> agree ?
>>
>>
>> Christophe
>>
>
>
>
--
Michaël CATANZARITI
log4cxx project manager
log4cxx user mailing list:
mailto:log4cxx-user-subscribe@logging.apache.org
log4cxx developer mailing list:
mailto:log4cxx-dev-subscribe@logging.apache.org
Re: [PATCH] Fix global namespace pollution (config_auto.h)
Posted by Christophe de Vienne <cd...@alphacent.com>.
Curt Arnold wrote:
> I'm uncomfortable with that solution, but I'm on deadline on something
> else and can't research it now. It is a problem that needs to be
> fixed, but I'd like to fix it by eliminating implementation details
> (like the config_auto.h) from the expansions of the API include files
> like logger.h.
>
This is indeed the final and correct solution, and I think we should
still work on it.
However that solution is perfect as a temporary one, which does not
disturb the actual way of doing and avoid to have macros like PACKAGE or
VERSION polluting the global namespace.
Christophe
Re: [PATCH] Fix global namespace pollution (config_auto.h)
Posted by Curt Arnold <ca...@houston.rr.com>.
I'm uncomfortable with that solution, but I'm on deadline on something
else and can't research it now. It is a problem that needs to be
fixed, but I'd like to fix it by eliminating implementation details
(like the config_auto.h) from the expansions of the API include files
like logger.h.
On Aug 12, 2004, at 12:51 PM, Christophe de VIENNE wrote:
> Cesar Eduardo Barros wrote:
>
>> On Thu, Jul 08, 2004 at 05:32:37PM -0300, Cesar Eduardo Barros wrote:
>>
>>> log4cxx is including a copy of the autoconf-generated config_auto.h
>>> even
>>> when compiling another program which also uses autoconf, causing
>>> conflicts. However, only LOG4CXX_EXPORT is really needed. I made this
>>> quick patch to fix it.
>>>
>>> This patch reuses the LOG4CXX define used for almost the same
>>> purpose on
>>> config_msvc.h (to determine if we're compiling log4cxx itself or a
>>> program which uses log4cxx).
>>>
>>
>> I didn't notice some headers were using the defines from config_auto.h
>> (HAVE_*, MUST_* and UNICODE). So, the simple solution above didn't
>> work
>> when I tried to use the DOM configurator.
>>
>
> I was trying to find a nice solution to this problem, it seems I don't
> have to anymore :-) Thanks !
>
>> This patch renames these defines on all headers and adds a
>> sed-generated
>> copy of config_auto.h prepending LOG4CXX_ to all these defines and
>> removing the bogus PACKAGE_*, VERSION and STDC_HEADERS ones when not
>> being included within the log4cxx sources.
>>
>
> This patch solves efficiently the macro problem. The idea to use sed
> is very good : nice job :-)
>
>
>> Tested compiling in the source directory, but should work with a
>> separate object directory too.
>>
>
> No, but I fixed it (it was a previous issue).
> I also had to changed macros names in src/threadspecificdata.cpp.
> I'm reviewing src/*.cpp to check is there is no forgotten macro
> renaming.
>
> Apart from this, I think the patch can be commited. Does everybody
> agree ?
>
>
> Christophe
>
Re: [PATCH] Fix global namespace pollution (config_auto.h)
Posted by Christophe de VIENNE <cd...@alphacent.com>.
Christophe de VIENNE wrote:
> Cesar Eduardo Barros wrote:
>
>> Tested compiling in the source directory, but should work with a
>> separate object directory too.
>
> No, but I fixed it (it was a previous issue).
I meant an existing issue, and not because of the patch.
> I also had to changed macros names in src/threadspecificdata.cpp.
> I'm reviewing src/*.cpp to check is there is no forgotten macro renaming.
Well, it was all of them in fact :-\
>
> Apart from this, I think the patch can be commited. Does everybody
> agree ?
Ready to commit...
Christophe
Re: [PATCH] Fix global namespace pollution (config_auto.h)
Posted by Michaël CATANZARITI <mc...@apache.org>.
Hi,
ok for me !
Christophe de VIENNE wrote:
> Cesar Eduardo Barros wrote:
>
>> On Thu, Jul 08, 2004 at 05:32:37PM -0300, Cesar Eduardo Barros wrote:
>>
>>
>>> log4cxx is including a copy of the autoconf-generated config_auto.h even
>>> when compiling another program which also uses autoconf, causing
>>> conflicts. However, only LOG4CXX_EXPORT is really needed. I made this
>>> quick patch to fix it.
>>>
>>> This patch reuses the LOG4CXX define used for almost the same purpose on
>>> config_msvc.h (to determine if we're compiling log4cxx itself or a
>>> program which uses log4cxx).
>>>
>>
>>
>> I didn't notice some headers were using the defines from config_auto.h
>> (HAVE_*, MUST_* and UNICODE). So, the simple solution above didn't work
>> when I tried to use the DOM configurator.
>>
>>
>
> I was trying to find a nice solution to this problem, it seems I don't
> have to anymore :-) Thanks !
>
>> This patch renames these defines on all headers and adds a sed-generated
>> copy of config_auto.h prepending LOG4CXX_ to all these defines and
>> removing the bogus PACKAGE_*, VERSION and STDC_HEADERS ones when not
>> being included within the log4cxx sources.
>>
>>
>
> This patch solves efficiently the macro problem. The idea to use sed is
> very good : nice job :-)
>
>
>> Tested compiling in the source directory, but should work with a
>> separate object directory too.
>>
>>
>
> No, but I fixed it (it was a previous issue).
> I also had to changed macros names in src/threadspecificdata.cpp.
> I'm reviewing src/*.cpp to check is there is no forgotten macro renaming.
>
> Apart from this, I think the patch can be commited. Does everybody agree ?
>
>
> Christophe
>
>
>
--
Michaël CATANZARITI
log4cxx project manager
log4cxx user mailing list:
mailto:log4cxx-user-subscribe@logging.apache.org
log4cxx developer mailing list:
mailto:log4cxx-dev-subscribe@logging.apache.org