After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 437579 - Evolution has compilation warnings
Evolution has compilation warnings
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: Developer Documentation
2.12.x (obsolete)
Other All
: Normal normal
: ---
Assigned To: Radhika PC
Evolution QA team
evolution[codecleanup]
: 507235 (view as bug list)
Depends on: 437584 437935 438467 439118 439122 440272 441014 441055
Blocks:
 
 
Reported: 2007-05-10 23:23 UTC by Gilles Dartiguelongue
Modified: 2008-04-13 03:44 UTC
See Also:
GNOME target: ---
GNOME version: 2.17/2.18


Attachments
proposed evo patch (43.11 KB, patch)
2007-10-01 13:39 UTC, Milan Crha
none Details | Review
proposed evo patch ][ (45.21 KB, patch)
2007-10-01 14:44 UTC, Milan Crha
none Details | Review
Revised patch (64.12 KB, patch)
2007-10-10 04:59 UTC, Matthew Barnes
committed Details | Review
Documentation patch (1.02 KB, patch)
2007-10-10 12:20 UTC, Matthew Barnes
committed Details | Review
proposed evo patch (903 bytes, patch)
2007-11-02 13:05 UTC, Milan Crha
committed Details | Review
Revised documentation patch (65.29 KB, patch)
2008-01-09 17:02 UTC, Matthew Barnes
committed Details | Review

Description Gilles Dartiguelongue 2007-05-10 23:23:37 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:
Comment 1 Matthew Barnes 2007-05-11 01:09:51 UTC
Confirming this, obviously.  I think the criteria for closing this bug should be successful compilation (including all plugins) with the -Werror compiler flag.
Comment 2 Gilles Dartiguelongue 2007-05-24 22:12:07 UTC
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 !
Comment 3 Matthew Barnes 2007-06-03 03:12:43 UTC
Gilles, just wanted to thank you for the awesome work.
I'll take a shot at cleaning up the remaining warnings.
Comment 4 Milan Crha 2007-10-01 13:39:16 UTC
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.
Comment 5 Matthew Barnes 2007-10-01 13:57:09 UTC
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 */
Comment 6 Milan Crha 2007-10-01 14:44:50 UTC
Created attachment 96465 [details] [review]
proposed evo patch ][

for evolution;

ok, changed as you suggested, #warning changed to comment with "FIXME:".
Comment 7 Srinivasa Ragavan 2007-10-04 07:37:40 UTC
Matthew, please review and get this in before 2.21.1
Comment 8 Matthew Barnes 2007-10-10 04:59:56 UTC
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).
Comment 9 Matthew Barnes 2007-10-10 12:20:07 UTC
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.
Comment 10 Matthew Barnes 2007-10-10 12:22:26 UTC
Changing component to 'Developer Documentation' since that's all that's left to do here.
Comment 11 Milan Crha 2007-10-11 16:20:41 UTC
(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?
Comment 12 Philip Van Hoof 2007-10-11 16:33:57 UTC
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.
Comment 13 Matthew Barnes 2007-10-11 17:47:45 UTC
(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
Comment 14 Matthew Barnes 2007-10-31 19:23:37 UTC
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
Comment 15 Milan Crha 2007-11-02 13:05:23 UTC
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...
Comment 16 Matthew Barnes 2007-11-02 13:23:42 UTC
Looks fine, commit to trunk and stable branch.
Comment 17 Milan Crha 2007-11-02 13:37:46 UTC
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.)
Comment 18 Srinivasa Ragavan 2007-11-27 18:16:13 UTC
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 19 Matthew Barnes 2007-12-13 21:37:16 UTC
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.
Comment 20 Claude Paroz 2007-12-25 21:19:44 UTC
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.
Comment 21 Radhika PC 2008-01-09 10:22:49 UTC
(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..?
Comment 22 Srinivasa Ragavan 2008-01-09 12:43:08 UTC
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.
Comment 23 Matthew Barnes 2008-01-09 13:09:30 UTC
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
Comment 24 André Klapper 2008-01-09 15:03:58 UTC
see bug 507235, probably triggered by this.
Comment 25 Matthew Barnes 2008-01-09 17:02:12 UTC
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).
Comment 26 Radhika PC 2008-01-10 05:53:47 UTC
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....
Comment 27 Claude Paroz 2008-01-12 09:25:28 UTC
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!
Comment 28 André Klapper 2008-01-12 09:57:14 UTC
claude: afaik, radhika tries to switch to conglomerate instead of framemaker to workaround the limitations of framemaker.
Comment 29 Srinivasa Ragavan 2008-01-12 15:36:30 UTC
Yes, Radhika is already on it.
Comment 30 Wouter Bolsterlee (uws) 2008-01-30 21:44:06 UTC
(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.

Comment 31 Wouter Bolsterlee (uws) 2008-01-30 21:48:47 UTC
Oh, I opened a new bug for the doc issues: Bug #513285
Comment 32 Matthew Barnes 2008-02-04 16:53:26 UTC
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.
Comment 33 Matthew Barnes 2008-03-11 00:36:09 UTC
Bumping version to a stable release.
Comment 34 Matthew Barnes 2008-03-14 18:13:54 UTC
Patch adapted to trunk and committed (revision 35192).
Comment 35 Matthew Barnes 2008-04-13 03:44:32 UTC
*** Bug 507235 has been marked as a duplicate of this bug. ***