908142 - Move the remainder of FAIL_ON_WARNINGS into moz.build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

+++ This bug was initially created as a clone of Bug #882859 +++

Comment 1

[Joey Armstrong [:joey]]

% find . -name 'Makefile.in' | xargs grep FAIL_ON_WARNINGS
./js/xpconnect/shell/Makefile.in:FAIL_ON_WARNINGS := 1
./uriloader/exthandler/tests/Makefile.in:FAIL_ON_WARNINGS = 1
./xpcom/tests/Makefile.in:FAIL_ON_WARNINGS = 1
./security/sandbox/Makefile.in:FAIL_ON_WARNINGS = 1
./security/manager/ssl/tests/unit/test_ocsp_stapling/Makefile.in:FAIL_ON_WARNINGS := 1
./netwerk/test/Makefile.in:FAIL_ON_WARNINGS := 1

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I've got patches, but I need to double-check them.

Comment 3

[Gregory Szorc [:gps]]

Attachment: Finish moving FAIL_ON_WARNINGS into moz.build

I grew tired of seeing these variables in the tree.

https://tbpl.mozilla.org/?tree=Try&rev=0e25a6a4bd7e

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 814524 [details] [diff] [review]
Finish moving FAIL_ON_WARNINGS into moz.build

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

So if you have FAIL_ON_WARNINGS in the moz.build and the following in the Makefile:

include $(topsrcdir)/config/config.mk
include $(topsrcdir)/config/rules.mk

then FAIL_ON_WARNINGS is defined in backend.mk which is included in rules.mk, but it needs to be defined by the time config.mk is included. Otherwise the assignment doesn't take effect.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part a: uriloader/exthandler/tests/

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part b: security/sandbox/

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part c: xpcom/tests/

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part d: js/xpconnect/shell/

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part e: netwerk/test/

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part f: Disallow FAIL_ON_WARNINGS in Makefiles

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 814743 [details] [diff] [review]
Part d: js/xpconnect/shell/

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

In what is surely an oversight, DEFINES isn't in FROZEN_VARIABLES. Mucking about with the compile variables after rules.mk inclusion is bad.

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 814745 [details] [diff] [review]
Part f: Disallow FAIL_ON_WARNINGS in Makefiles

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

I guess I can't review this just yet because of the lingering reference from the r-'d patch.

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part g: Remove defines from js/xpconnect/shell/Makefile.in

Comment 14

[Gregory Szorc [:gps]]

Comment on attachment 814745 [details] [diff] [review]
Part f: Disallow FAIL_ON_WARNINGS in Makefiles

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

I guess I can't review this just yet because of the lingering reference from the r-'d patch.

Comment 15

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/1899772c2f83
https://hg.mozilla.org/mozilla-central/rev/66bb2a099016
https://hg.mozilla.org/mozilla-central/rev/a27418c31cd3
https://hg.mozilla.org/mozilla-central/rev/88e3a46b2c80
https://hg.mozilla.org/mozilla-central/rev/a9b08bb0f5fb
https://hg.mozilla.org/mozilla-central/rev/a8ba29271998

I couldn't actually disallow it in makefiles because js/src is doing something weird. I'll file a followup.