968363 - Mark security/manager/ssl/src/ as FAIL_ON_WARNINGS

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Daniel Holbert [:dholbert]]

With bug 968338 and bug 968348 fixed, security/manager/ssl/src/ is build-warning-free on all TBPL-tested build configurations.

Let's mark it as FAIL_ON_WARNINGS to keep it that way.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Thanks for the patch.

IIRC, FAIL_ON_WARNINGS isn't the default on Windows builds, right? Can we change that for this module? (I almost always build on Windows.)

Comment 3

[Daniel Holbert [:dholbert]]

It's not the default anywhere (you have to put "ac_add_options --enable-warnings-as-errors" in your .mozconfig).

We do have that in the TBPL mozconfigs for Windows/Mac/Linux/Android at least, though, and you should be able to build (and catch yourself from introducing warnings) if you put that in your .mozconfig.

(It's not the default -- i.e. you have to opt into it -- because we want to minimize the chances of triggering errors in new contributors' local (clean) builds, if they're using a particularly old or new compiler that we don't test on TBPL and may warn about things that our compilers don't warn about.)

Comment 4

[Daniel Holbert [:dholbert]]

(You're probably remembering the situation before bug 824247 was fixed, though. Before that, we didn't have the mozconfig flag on our windows TBPL builders, so you could e.g. get a green windows-only Try run and then bust Linux/Mac when you landed. That's not the case anymore (modulo differences between the things that compilers decide to warn about).)

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 8370936 [details] [diff] [review]
fix

Review of attachment 8370936 [details] [diff] [review]:
-----------------------------------------------------------------

NIT: please use r=briansmith in the commit message.

Thanks for this, and thanks for explaining how FAIL_ON_WARNINGS works in detail.

Comment 6

[Daniel Holbert [:dholbert]]

No problem -- thanks for the quick review turnarounds!

Try run: https://tbpl.mozilla.org/?tree=Try&rev=e4e130270e6f

Comment 7

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e6082884469c

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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