Bug 2579 - Installed headers redefine autotools-specific constants
Summary: Installed headers redefine autotools-specific constants
Status: RESOLVED FIXED
Alias: None
Product: Claws Mail (GTK 2)
Classification: Unclassified
Component: Other (show other bugs)
Version: 3.8.1
Hardware: PC Linux
: P3 normal
Assignee: users
URL:
Depends on:
Blocks:
 
Reported: 2012-01-14 18:35 UTC by Michał Górny
Modified: 2012-10-06 06:44 UTC (History)
0 users

See Also:


Attachments

Description Michał Górny 2012-01-14 18:35:42 UTC
Installed claws-mail headers include 'config.h' file with standard autotools PACKAGE_* macros defined. This file is included by various header files which causes the package-specific PACKAGE_* macros to be redefined and throws a lot of compiler warnings like:

In file included from src/plugin.c:13:
In file included from /usr/include/claws-mail/account.h:26:
In file included from /usr/include/claws-mail/prefs_account.h:24:
/usr/include/claws-mail/config.h:352:9: warning: 'PACKAGE_NAME' macro redefined
#define PACKAGE_NAME ""
        ^
./config.h:45:9: note: previous definition is here
#define PACKAGE_NAME "claws-mail-gkr"
        ^

Same goes for version.h.
Comment 1 Michał Górny 2012-06-04 07:36:41 UTC
Any explanation, please?
Comment 2 Ricardo Mones 2012-06-04 07:58:40 UTC
You shouldn't be shipping those files in your plugin. Have a look at other plugins' sources.
Comment 3 Michał Górny 2012-06-04 08:07:17 UTC
Shipping what files? You mean making it buildable outside of claws-mail codebase?
Comment 4 Ricardo Mones 2012-06-04 08:30:04 UTC
Looking at your sources (I guess we're talking of https://github.com/mgorny/claws-mail-gkr) you're including config.h, that shouldn't be there (forget my other explanation, I figured out things that weren't there ;-)
Comment 5 Michał Górny 2012-06-04 08:39:31 UTC
I'm including _my_ config.h which is part of my build system and shouldn't be disallowed. The claws-mail config.h is included by its other headers as the compiler trace shows.
Comment 6 Ricardo Mones 2012-06-04 09:02:46 UTC
Is not disallowed, but if you define things already defined in Claws Mail config.h, what do you expect?

If you're building a plugin for an application, I think you should follow the rules for building plugins for that application, and not redefining macros is a pretty simple rule IMO.

BTW, this is not a place for this, please followup any other question on devel list.
Comment 7 Holger Berndt 2012-06-28 21:53:57 UTC
Reopening as no argument supporting the classification as invalid was given on devel ML.
Comment 8 Paul 2012-06-29 06:03:19 UTC
wasn't comment #6 the reason?
Comment 9 Holger Berndt 2012-06-29 20:21:32 UTC
No, in comment #6 it was offered to answer questions on -devel, but the related mail was unfortunately ignored.

If this bug is invalid, I'd also be interested to know how to use autotools with AC_CONFIG_HEADERS / AM_CONFIG_HEADER from a plugin. The mentioned rule of "not redefining macros" is everything but simple (impossible?) to follow, as the names are not chosen by the developer, but by autotools machinery, and are thus doomed to collide.

In fact, as far as I can see, all external plugins that use this machinery (notification, gdata, attachwarner, pdf_viewer, geolocation, perl, fancy, python) have this problem. They just by coincidence redefine given #defines with empty strings, about which gcc currently doesn't complain about (even though it's likely invalid, or at the very least unclean).

Furthermore, the header might be incorrect, because Claws Mail might have been compiled by the distributor, and a plugin might be compiled by a user, so Claws Mail's installed config.h might not fit the actual machine.

The way I understand autotools is that it's supposed to be included in _all_ source files, before any other header, but it's not supposed to be included from any public header, and it's also not supposed to be installed. A more elaborate reasoning, mostly repeating what I wrote above, can be found in [1].

Now, I usually get autotools wrong, so if that's all wrong and the bug is indeed invalid, please tell me (and Sal, and Mones, and Michal) how to fix our code.

[1] http://inaugust.com/post/68
Comment 10 Paul 2012-06-30 07:17:51 UTC
Holger, just because it didn't (yet?) receive the number of replies which goes above your 'ignored' threshold does not mean that it was ignored - you cannot tell how many read it and have since been considering it.

I think your comment #9 could be exactly the sort of thing that is referred to in paragraph 3 of comment #6.
Comment 11 Holger Berndt 2012-06-30 07:54:40 UTC
> Holger, just because it didn't (yet?) receive the number of replies
> which goes above your 'ignored' threshold does not mean that it
> was ignored

The definition of my "ignored threshold" is pretty simple: It's 0. In words: Zero. Not counting my very own "I think you're right" reply.

Paul, as you refer once more to discussion on -devel, feel free to close this bug again as invalid, but please, explain the reasoning also here on this very tracker, so that developers that stumble upon this problem learn how to fix their stuff.

Alternatively, if Mones fixes his very own plugin which has this very bug as well (which I guess he knows how to do, as of the often-cited comment #6), I'll add an explanation to this tracker, so you don't have to bother any more.
Comment 12 users 2012-06-30 09:04:36 UTC
Changes related to this bug have been committed.
Please check latest CVS and update the bug accordingly.
You can also get the patch from:
http://www.claws-mail.org/tracker/

2012-06-30 [paul]	3.8.1cvs1

	* Makefile.am
		fix bug 2579, 'Installed headers redefine autotools-specific constants'

2012-06-27 [paul]	3.8.1
	* NEWS
	* README
	* RELEASE_NOTES	
		3.8.1 unleashed!
		

Note You need to log in before you can comment on or make changes to this bug.