1317638 - Fix build fail in TestBlockingProcess.cpp if enabling warnings as errors.

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[James Cheng[:JamesCheng]]

I will get compile error

/media/hdd/Projects/gecko-mozilla3/xpcom/tests/TestBlockingProcess.cpp: In function 'int main()':
 0:29.62 /media/hdd/Projects/gecko-mozilla3/xpcom/tests/TestBlockingProcess.cpp:6:37: error: ignoring return value of 'size_t fread(void*, size_t, size_t, FILE*)', declared with attribute warn_unused_result [-Werror=unused-result]
 0:29.62 cc1plus: all warnings being treated as errors

by adding ac_add_options --enable-warnings-as-errors in mozconfig.

I'm wondering why I did not meet "warning as error build break" before.

Comment 1

[James Cheng[:JamesCheng]]

Attachment: Bug 1317638 - Fix build fail in TestBlockingProcess.cpp if enabling warnings as errors.

Review commit: https://reviewboard.mozilla.org/r/93072/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/93072/

Comment 2

[James Cheng[:JamesCheng]]

Hi gps,

Could you please help me to clarify that I thought warning-as-error is a default setting for try server...

But it seems not, the code is landed to central without any build break...

How could I ensure the build config on try build to make sure it is not "warning-as-error"?

If I add adding ac_add_options --enable-warnings-as-errors in mozconfig, does it effect on whole gecko since there still has lot of warning message but no build failure on my local machine but why?

Thank you!

Comment 3

[James Cheng[:JamesCheng]]

Additional: My local machine is Ubuntu 14.04

and my mozconfig is 
===============================================
. "$topsrcdir/browser/config/mozconfig"

mk_add_options AUTOCLOBBER=1

ac_add_options --enable-warnings-as-errors
ac_add_options --enable-debug
===============================================

Comment 4

[Eric Rahm [:erahm]]

Comment on attachment 8810748 [details]
Bug 1317638 - Fix build fail in TestBlockingProcess.cpp if enabling warnings as errors.

https://reviewboard.mozilla.org/r/93072/#review93182

Thanks for the fix!

Comment 5

[Gregory Szorc [:gps]]

Running warnings-as-errors locally is dangerous because your local toolchain may differ from what automation is using. So there may be warnings in automation your local toolchain doesn't produce and there may be warnings in your local toolchain that automation doesn't produce.

This is really a poor experience for C++ developers, as you shouldn't have to push to Try to know if a compilation warning will present itself (at least for the platform you are developing on). There has been talk of adding a mode to configure that automatically downloaded and used the toolchain that automation is configured to use. That would help prevent discrepancies between local builds and automation and allow warnings as errors to be safely enabled.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6026ad16288
Fix build fail in TestBlockingProcess.cpp if enabling warnings as errors. r=erahm

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/c6026ad16288

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

This broke my local Windows build:

15:51.82 TestBlockingProcess.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) struct mozilla::unused_t const mozilla::Unused" (__imp_?Unused@mozilla@@3Uunused_t@1@B)
15:51.82
15:51.82 TestBlockingProcess.exe : fatal error LNK1120: 1 unresolved externals

Comment 9

[Eric Rahm [:erahm]]

(In reply to David Anderson [:dvander] from comment #8)
> This broke my local Windows build:
> 
> 15:51.82 TestBlockingProcess.obj : error LNK2001: unresolved external symbol
> "__declspec(dllimport) struct mozilla::unused_t const mozilla::Unused"
> (__imp_?Unused@mozilla@@3Uunused_t@1@B)
> 15:51.82
> 15:51.82 TestBlockingProcess.exe : fatal error LNK1120: 1 unresolved
> externals

Ehsan landed a fix on inbound in bug 1318678.

Comment 10

[areya]

https://hg.mozilla.org/mozilla-central/rev/c6026ad16288