GNOME Bugzilla – Bug 437579
Evolution has compilation warnings
Last modified: 2008-04-13 03:44:32 UTC
Please describe the problem: evolution compilation throws a lot of (mostly) easily fixable warnings. This bug is to be used as a tracker for bugs attempting to fix the current state of the tree. Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information:
Confirming this, obviously. I think the criteria for closing this bug should be successful compilation (including all plugins) with the -Werror compiler flag.
ok, mostly there. Main folders are now cleaned up. Principals warnings left: - plugins I don't compile by default (those not in all) - type-punned warnings - some print stuff in calendar - some unhandled cases in a switch in calendar This is something like 5% the volume of warning output before applying those patchs. We are close to -Werror !
Gilles, just wanted to thank you for the awesome work. I'll take a shot at cleaning up the remaining warnings.
Created attachment 96460 [details] [review] proposed evo patch for evolution; here's a patch, which will solve most of the compiler warnings in Evolution. For calendar subdirectory is bug #439122; for file mail-send-recv.c is bug #473903. Here only left non-compiler warnings (warnings defined in code): ----- e-table-field-chooser-item.c:210:2: warning: #warning Group Child bounds !? e-table-header-item.c:222:2: warning: #warning FOO BAA e-table.c:2930:2: warning: #warning FIXME e-text.c:624:2: warning: #warning "AIEEEE FIX ME. a pango layout per calc_ellipsis sucks" e-text.c:1253:2: warning: #warning Color brokenness ... e-text.c:1942:2: warning: #warning "need to sort out tooltip stuff." e-unicode.c:57:2: warning: #warning FIXME: this has not been ported fully yet - non ASCII people beware. ----- Which are enclosed in #ifndef NO_WARNINGS ... #endif now, so almost easy way to get them out. I also had a chat with a man which knows much about gcc and strict-aliasing, so I tried to fix this as good as I was able.
Please change the remaining #warning messages to "XXX" or "FIXME" comments. There's no reason to make the compiler spew messages like that. e.g. - #warning AIEEEE FIX ME. a pango layout per calc_ellipsis sucks + /* FIXME - A Pango laybout per calc_ellipsis sucks */
Created attachment 96465 [details] [review] proposed evo patch ][ for evolution; ok, changed as you suggested, #warning changed to comment with "FIXME:".
Matthew, please review and get this in before 2.21.1
Created attachment 96978 [details] [review] Revised patch I reviewed Milan's patch in detail and fixed some additional warnings in conduits as well as some recent ones that have crept in. Here's the patch I finally committed. I think that should do it for warnings from C code; all that remains are documentation warnings. Committed to Subversion trunk (revision 34368).
Created attachment 96986 [details] [review] Documentation patch This patch fixes all the "entity not defined" warnings that get generated when we run xml2po on the help documentation. Not sure if this is the correct way to fix them but it works.
Changing component to 'Developer Documentation' since that's all that's left to do here.
(In reply to comment #8) > Created an attachment (id=96978) [edit] > Revised patch > > I reviewed Milan's patch in detail and fixed some additional warnings in > conduits as well as some recent ones that have crept in. Here's the patch I > finally committed. I think that should do it for warnings from C code; all > that remains are documentation warnings. > > Committed to Subversion trunk (revision 34368). > Matt, you forget to include part of my patch from exchange-delegates-user.c, so I still see these warnings: exchange-delegates-user.c: In function 'exchange_delegates_user_edit': exchange-delegates-user.c:276: warning: dereferencing type-punned pointer will break strict-aliasing rules exchange-delegates-user.c:280: warning: dereferencing type-punned pointer will break strict-aliasing rules --- even my patch contains fix for it :) Other warnings: memo-page.c: In function 'source_changed_cb': memo-page.c:1115: warning: 'def_address' may be used uninitialized in this function e-itip-control.c: In function 'insert_boxes': e-itip-control.c:2203: warning: dereferencing type-punned pointer will break strict-aliasing rules e-itip-control.c:2208: warning: dereferencing type-punned pointer will break strict-aliasing rules e-itip-control.c: In function 'insert_ok': e-itip-control.c:2266: warning: dereferencing type-punned pointer will break strict-aliasing rules --- will be fixed when patch in bug #439122 will be committed. Matt, your patch produces new warning on my 64bit machine, it's this one: em-format.c: In function 'em_format_add_puri': em-format.c:369: warning: format '%d' expects type 'int', but argument 4 has type 'size_t' I guess, on your 32bit machine it produced some similar warning, when there was "%ld", right?
Note about camel: a lot of compilation warnings (even with -pedantic) got fixed in camel-lite. Using some recursive diffing you can probably filter these out o camel-lite's changes easily. There are indeed still some compilation warnings in upstream camel (that are fixed in camel-lite). Please notify me on IRC if you need assistance with these.
(In reply to comment #11) > Matt, your patch produces new warning on my 64bit machine, it's this one: > em-format.c: In function 'em_format_add_puri': > em-format.c:369: warning: format '%d' expects type 'int', but argument 4 has > type 'size_t' > I guess, on your 32bit machine it produced some similar warning, when there was > "%ld", right? Good catch. GLib to the rescue: G_GSIZE_FORMAT
Shaun McCance suggested in [1] a better solution to the documentation errors than the patch I submitted in comment #9. Making a note of it here for my own reference. [1] http://bugzilla.gnome.org/show_bug.cgi?id=373421#c19
Created attachment 98373 [details] [review] proposed evo patch for evolution; I cause trouble with my patch on one place (see attachment), so here's a fix for that. I hope there wasn't more places like this...
Looks fine, commit to trunk and stable branch.
Committed to trunk. Committed revision 34484. I'm sorry, I was wrong, it isn't in stable yet (I thought by date it is, but it isn't, my fault.)
Radhika, you need to have the patch applied to the xml you generate out of Frame. No need to commit this, any doc drop would over write it. So it should be taken separately.
Committing the final patch here (comment #9) for 2.21.4 since I'm tired of waiting and, given the recent discussion on evolution-hackers, I trust it will not be overwritten.
Sorry Matthew, but the patch you committed on the doc (rev. 34695) makes xml2po now crash on my system when I try to update. trunk/help/fr$ xml2po -e -u fr.po ../C/evolution.xml.
(In reply to comment #18) > Radhika, you need to have the patch applied to the xml you generate out of > Frame. No need to commit this, any doc drop would over write it. So it should > be taken separately. > Comment #20 says the patch makes xml2po crash..Should I merge this patch to internal one or omit before I commit..?
Radhika, for new you can revert in your internal work space and go ahead. Shouldn't matter much for you really. Just take any version either trunk or trunk-this and go ahead with your changes.
It doesn't necessarily make xml2po crash, but it does seem to make it consume nearly a gig of RAM while processing. If your system doesn't have that much RAM then xml2po will abort. I don't know why my solution causes such extreme memory consumption. Shaun McCance has a alternate solution to the "entity not defined" errors in [1]. I'll try to put a patch together based on his recommendations. [1] http://bugzilla.gnome.org/show_bug.cgi?id=373421#c19
see bug 507235, probably triggered by this.
Created attachment 102477 [details] [review] Revised documentation patch This patch reverts my previous solution to the "entity not defined" warnings and instead follows Shaun's suggestion of using DocBook tags like <trademark> and <quote> which, looking back now, is obviously the right way to do it. I get the feeling I'm only scratching the surface with some of the tags I added like <guilabel> and <userinput>. There seems to be a whole lot of content that isn't properly tagged. Oh, and it also fixes xml2po's enormous memory consumption (bug #507235).
As per GNOME Style Guide, http://library.gnome.org/devel/gdp-style-guide/2.20/trademarks.html.en we must avoid using trademarked terms in free software documentation. Does it imply that we must not use trademark symbol while quoting Evolution. Request suggestions....
Matthew, your latest patch is clearly the way to go. But the problem is that Radhika uses a GUI tool that doesn't seem to provide correct tagging... And please take into account that each time you modifiy an entity, translators have to review tens or hundreds of strings because entities are expanded before translation toolchain. So either solution you choose, keep it in the long term!
claude: afaik, radhika tries to switch to conglomerate instead of framemaker to workaround the limitations of framemaker.
Yes, Radhika is already on it.
(In reply to comment #23) > It doesn't necessarily make xml2po crash, but it does seem to make it consume > nearly a gig of RAM while processing. If your system doesn't have that much > RAM then xml2po will abort. I don't know why my solution causes such extreme > memory consumption. > > Shaun McCance has a alternate solution to the "entity not defined" errors in > [1]. I'll try to put a patch together based on his recommendations. > > [1] http://bugzilla.gnome.org/show_bug.cgi?id=373421#c19 Ok, this bug hit me too. I'll attach a patch to the documentation that removes ALL entities that can also be expressed as native UTF-8 chars, and adds a few cleanups as well.
Oh, I opened a new bug for the doc issues: Bug #513285
Marking the patch in comment #25 as "commit_after_freeze" so I don't forget. Srini approved it in bug #513285. Will commit for 2.23.1.
Bumping version to a stable release.
Patch adapted to trunk and committed (revision 35192).
*** Bug 507235 has been marked as a duplicate of this bug. ***