Closed Bug 1317638 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: JamesCheng, Assigned: JamesCheng)

References

Details

Attachments

(1 file)

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.
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!
Flags: needinfo?(gps)
Attachment #8810748 - Flags: review?(erahm)
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 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!
Attachment #8810748 - Flags: review?(erahm) → review+
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.
Flags: needinfo?(gps)
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c6026ad16288
Fix build fail in TestBlockingProcess.cpp if enabling warnings as errors. r=erahm
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c6026ad16288
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Depends on: 1318678
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
Flags: needinfo?(jacheng)
(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.
Flags: needinfo?(jacheng)
You need to log in before you can comment on or make changes to this bug.