We have detected a build metrics regression from push: https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=329e64cecd9822d76fff76c0ef42b0f75ede30dd As author of one of the patches included in that push, we need your help to address this regression. Regressions: 17% compiler warnings windows2012-64 asan debug 899.00 -> 1,055.00 17% compiler warnings windows2012-64 asan opt 908.00 -> 1,064.00 17% compiler warnings windows2012-64 debug plain 924.00 -> 1,080.00 17% compiler warnings windows2012-64 debug 926.00 -> 1,082.00 17% compiler warnings windows2012-64 debug static-analysis928.00 -> 1,084.00 17% compiler warnings windows2012-64 opt static-analysis934.00 -> 1,090.00 17% compiler warnings windows2012-64 opt plain 934.00 -> 1,090.00 17% compiler warnings windows2012-64 opt rusttests 934.00 -> 1,090.00 17% compiler warnings windows2012-64 pgo 934.00 -> 1,090.00 17% compiler warnings windows2012-64 opt 934.00 -> 1,090.00 13% compiler warnings windows2012-32 debug 890.00 -> 1,002.00 12% compiler warnings windows2012-32 opt rusttests 898.00 -> 1,010.00 12% compiler warnings windows2012-32 pgo 898.00 -> 1,010.00 12% compiler warnings windows2012-32 opt 898.00 -> 1,010.00 12% compiler warnings windows2012-32 debug static-analysis899.00 -> 1,011.00 12% compiler warnings windows2012-32 opt static-analysis905.00 -> 1,017.00 You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=14920 On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the jobs in a pushlog format. To learn more about the regressing test(s), please see: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Automated_Performance_Testing_and_Sheriffing/Build_Metrics

There are a few new Rust warnings in the added crate synstructure v0.9.0: > Compiling synstructure v0.9.0 > … > … warning: use of deprecated item 'proc_macro2::TokenStream::empty': please use TokenStream::new > … --> third_party\rust\synstructure\src\lib.rs:544:21 > … | > … 544 | let mut t = TokenStream::empty(); > … | ^^^^^^^^^^^^^^^^^^ > … | > … = note: #[warn(deprecated)] on by default > … warning: use of deprecated item 'proc_macro2::TokenStream::empty': please use TokenStream::new > … --> third_party\rust\synstructure\src\lib.rs:624:21 > … | > … 624 | let mut t = TokenStream::empty(); > … | ^^^^^^^^^^^^^^^^^^ > … warning: use of deprecated item 'proc_macro2::TokenStream::empty': please use TokenStream::new > … --> third_party\rust\synstructure\src\lib.rs:693:24 > … | > … 693 | let mut body = TokenStream::empty(); > … | ^^^^^^^^^^^^^^^^^^ > … warning: use of deprecated item 'proc_macro2::TokenStream::empty': please use TokenStream::new > … --> third_party\rust\synstructure\src\lib.rs:1081:21 > … | > … 1081 | let mut t = TokenStream::empty(); > … | ^^^^^^^^^^^^^^^^^^ > … warning: use of deprecated item 'proc_macro2::TokenStream::empty': please use TokenStream::new > … --> third_party\rust\synstructure\src\lib.rs:1135:21 > … | > … 1135 | let mut t = TokenStream::empty(); > … | ^^^^^^^^^^^^^^^^^^ > … warning: use of deprecated item 'proc_macro2::TokenStream::empty': please use TokenStream::new > … --> third_party\rust\synstructure\src\lib.rs:1189:21 > … | > … 1189 | let mut t = TokenStream::empty(); > … | ^^^^^^^^^^^^^^^^^^ But the majority of the new warnings are in the C code added by lmdb-sys: > …\mdb.c:1501 [C4996] 'strerror': This function or variable may be unsafe. Consider using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. > …\mdb.c:1531 [C4996] 'sprintf': This function or variable may be unsafe. Consider using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. > …\mdb.c:1825 [C4267] 'initializing': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:1919 [C4389] '==': signed/unsigned mismatch > …\mdb.c:1946 [C4389] '==': signed/unsigned mismatch > …\mdb.c:2008 [C4267] '+=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:2022 [C4267] 'initializing': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:2047 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:2158 [C4267] 'initializing': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:2213 [C4127] conditional expression is constant > …\mdb.c:2216 [C4127] conditional expression is constant > …\mdb.c:2252 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:2272 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:2433 [C4127] conditional expression is constant > …\mdb.c:2433 [C4267] '=': conversion from 'size_t' to 'unsigned short', possible loss of data > …\mdb.c:2887 [C4267] '=': conversion from 'size_t' to 'int', possible loss of data > …\mdb.c:2888 [C4267] 'function': conversion from 'size_t' to 'int', possible loss of data > …\mdb.c:3263 [C4267] 'initializing': conversion from 'size_t' to 'int', possible loss of data > …\mdb.c:3323 [C4267] '=': conversion from 'size_t' to 'DWORD', possible loss of data > …\mdb.c:3324 [C4267] 'function': conversion from 'size_t' to 'DWORD', possible loss of data > …\mdb.c:3467 [C4244] '=': conversion from 'unsigned int' to 'unsigned char', possible loss of data > …\mdb.c:3473 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:3477 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:3513 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:3516 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:3531 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:3586 [C4456] declaration of 'i' hides previous local declaration > …\mdb.c:3838 [C4244] '+=': conversion from '__int64' to 'off_t', possible loss of data > …\mdb.c:3951 [C4267] '=': conversion from 'size_t' to 'LONG', possible loss of data > …\mdb.c:4193 [C4100] 'mode': unreferenced formal parameter > …\mdb.c:4205 [C4996] 'wcscpy': This function or variable may be unsafe. Consider using wcscpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. > …\mdb.c:4297 [C4996] 'GetVersion': was declared deprecated > …\mdb.c:4465 [C4100] 'module': unreferenced formal parameter > …\mdb.c:4465 [C4100] 'ptr': unreferenced formal parameter > …\mdb.c:4682 [C4244] 'function': conversion from 'mdb_hash_t' to 'unsigned long', possible loss of data > …\mdb.c:4798 [C4996] 'sprintf': This function or variable may be unsafe. Consider using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. > …\mdb.c:4799 [C4996] 'sprintf': This function or variable may be unsafe. Consider using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. > …\mdb.c:4942 [C4996] 'strdup': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _strdup. See online help for details. > …\mdb.c:5189 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:5192 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:5197 [C4244] 'return': conversion from 'ssize_t' to 'int', possible loss of data > …\mdb.c:5223 [C4244] 'return': conversion from 'ssize_t' to 'int', possible loss of data > …\mdb.c:5315 [C4244] '=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:5412 [C4456] declaration of 'x' hides previous local declaration > …\mdb.c:5579 [C4457] declaration of 'flags' hides function parameter > …\mdb.c:5671 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:5679 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:5691 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:5692 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:6044 [C4244] '=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:6081 [C4244] '=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:6297 [C4244] '=': conversion from 'int' to 'indx_t', possible loss of data > …\mdb.c:6517 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:6632 [C4267] '=': conversion from 'size_t' to 'uint16_t', possible loss of data > …\mdb.c:6714 [C4267] '=': conversion from 'size_t' to 'uint16_t', possible loss of data > …\mdb.c:6720 [C4267] '=': conversion from 'size_t' to 'indx_t', possible loss of data > …\mdb.c:6776 [C4267] '=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\mdb.c:6785 [C4244] '=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:6792 [C4244] '=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:6811 [C4267] 'initializing': conversion from 'size_t' to 'int', possible loss of data > …\mdb.c:6855 [C4146] unary minus operator applied to unsigned type, result still unsigned > …\mdb.c:6863 [C4267] '=': conversion from 'size_t' to 'unsigned short', possible loss of data > …\mdb.c:6910 [C4456] declaration of 'mp' hides previous local declaration > …\mdb.c:6963 [C4456] declaration of 'mp' hides previous local declaration > …\mdb.c:6986 [C4267] '=': conversion from 'size_t' to 'int', possible loss of data > …\mdb.c:7146 [C4244] '=': conversion from 'uint32_t' to 'uint16_t', possible loss of data > …\mdb.c:7148 [C4244] '=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:7263 [C4267] '-=': conversion from 'size_t' to 'indx_t', possible loss of data > …\mdb.c:7276 [C4267] 'initializing': conversion from 'size_t' to 'int', possible loss of data > …\mdb.c:7303 [C4267] '=': conversion from 'size_t' to 'indx_t', possible loss of data > …\mdb.c:7311 [C4267] '=': conversion from 'size_t' to 'unsigned short', possible loss of data > …\mdb.c:7312 [C4244] '=': conversion from 'unsigned int' to 'unsigned short', possible loss of data > …\mdb.c:7314 [C4267] '=': conversion from 'size_t' to 'unsigned short', possible loss of data > …\mdb.c:7316 [C4267] '=': conversion from 'size_t' to 'unsigned short', possible loss of data > …\mdb.c:7316 [C4127] conditional expression is constant > …\mdb.c:7377 [C4267] '+=': conversion from 'size_t' to 'indx_t', possible loss of data > …\mdb.c:7396 [C4244] '+=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:7405 [C4244] '+=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:7439 [C4333] '>>': right shift by too large amount, data loss > …\mdb.c:7762 [C4244] '-=': conversion from 'int' to 'indx_t', possible loss of data > …\mdb.c:7768 [C4244] '-=': conversion from 'int' to 'indx_t', possible loss of data > …\mdb.c:7775 [C4267] '=': conversion from 'size_t' to 'unsigned short', possible loss of data > …\mdb.c:7848 [C4244] '=': conversion from 'unsigned int' to 'unsigned short', possible loss of data > …\mdb.c:7849 [C4244] '=': conversion from 'unsigned int' to 'unsigned short', possible loss of data > …\mdb.c:7875 [C4244] '=': conversion from 'unsigned int' to 'unsigned short', possible loss of data > …\mdb.c:7876 [C4244] '=': conversion from 'unsigned int' to 'unsigned short', possible loss of data > …\mdb.c:7898 [C4267] 'function': conversion from 'size_t' to 'int', possible loss of data > …\mdb.c:8060 [C4244] '=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:8147 [C4244] '+=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:8165 [C4244] '=': conversion from 'unsigned int' to 'unsigned short', possible loss of data > …\mdb.c:8166 [C4244] '=': conversion from 'unsigned int' to 'unsigned short', possible loss of data > …\mdb.c:8643 [C4244] '-=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:8644 [C4244] '+=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:8645 [C4244] '+=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:8646 [C4244] '-=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:8660 [C4267] '-=': conversion from 'size_t' to 'indx_t', possible loss of data > …\mdb.c:8668 [C4267] '-=': conversion from 'size_t' to 'indx_t', possible loss of data > …\mdb.c:8669 [C4244] '=': conversion from 'int' to 'indx_t', possible loss of data > …\mdb.c:8676 [C4267] '=': conversion from 'size_t' to 'int', possible loss of data > …\mdb.c:8678 [C4267] '=': conversion from 'size_t' to 'int', possible loss of data > …\mdb.c:8690 [C4244] '=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:8825 [C4244] '=': conversion from 'int' to 'indx_t', possible loss of data > …\mdb.c:8844 [C4244] 'function': conversion from 'int' to 'indx_t', possible loss of data > …\mdb.c:8941 [C4244] '-=': conversion from 'int' to 'indx_t', possible loss of data > …\mdb.c:9168 [C4457] declaration of 'pg' hides function parameter > …\mdb.c:9226 [C4457] declaration of 'pg' hides function parameter > …\mdb.c:9260 [C4127] conditional expression is constant > …\mdb.c:9260 [C4267] '=': conversion from 'size_t' to 'unsigned short', possible loss of data > …\mdb.c:9447 [C4267] '=': conversion from 'size_t' to 'DWORD', possible loss of data > …\mdb.c:9483 [C4267] '=': conversion from 'size_t' to 'DWORD', possible loss of data > …\mdb.c:9762 [C4996] 'strdup': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _strdup. See online help for details. > …\mdb.c:9773 [C4456] declaration of 'dummy' hides previous local declaration > …\mdb.c:9785 [C4244] '=': conversion from 'int' to 'unsigned char', possible loss of data > …\mdb.c:9916 [C4244] '=': conversion from 'unsigned int' to 'indx_t', possible loss of data > …\mdb.c:10033 [C4100] 'env': unreferenced formal parameter > …\mdb.c:10056 [C4996] 'sprintf': This function or variable may be unsafe. Consider using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. > …\midl.c:44 [C4267] 'initializing': conversion from 'size_t' to 'unsigned int', possible loss of data > …\midl.c:146 [C4267] '+=': conversion from 'size_t' to 'unsigned int', possible loss of data > …\midl.c:176 [C4267] 'function': conversion from 'size_t' to 'int', possible loss of data > … > …\mdb.c:1509 [C4172] returning address of local variable or temporary: buf > …\mdb.c:2069 [C4706] assignment within conditional expression > …\mdb.c:2219 [C4701] potentially uninitialized local variable 'last' used > …\mdb.c:2254 [C4706] assignment within conditional expression > …\mdb.c:2288 [C4706] assignment within conditional expression > …\mdb.c:2421 [C4706] assignment within conditional expression > …\mdb.c:2700 [C4706] assignment within conditional expression > …\mdb.c:2727 [C4706] assignment within conditional expression > …\mdb.c:2743 [C4706] assignment within conditional expression > …\mdb.c:2863 [C4706] assignment within conditional expression > …\mdb.c:3473 [C4706] assignment within conditional expression > …\mdb.c:3619 [C4706] assignment within conditional expression > …\mdb.c:4733 [C4706] assignment within conditional expression > …\mdb.c:4934 [C4706] assignment within conditional expression > …\mdb.c:4990 [C4706] assignment within conditional expression > …\mdb.c:5495 [C4706] assignment within conditional expression > …\mdb.c:5538 [C4706] assignment within conditional expression > …\mdb.c:5620 [C4706] assignment within conditional expression > …\mdb.c:6594 [C4706] assignment within conditional expression > …\mdb.c:6602 [C4706] assignment within conditional expression > …\mdb.c:6774 [C4706] assignment within conditional expression > …\mdb.c:7037 [C4706] assignment within conditional expression > …\mdb.c:7114 [C4706] assignment within conditional expression > …\mdb.c:7142 [C4706] assignment within conditional expression > …\mdb.c:7284 [C4706] assignment within conditional expression > …\mdb.c:7817 [C4706] assignment within conditional expression > …\mdb.c:8052 [C4706] assignment within conditional expression > …\mdb.c:8578 [C4706] assignment within conditional expression > …\mdb.c:8589 [C4706] assignment within conditional expression > …\mdb.c:9286 [C4706] assignment within conditional expression > …\mdb.c:9322 [C4706] assignment within conditional expression > …\mdb.c:9435 [C4706] assignment within conditional expression > …\mdb.c:9473 [C4706] assignment within conditional expression > …\mdb.c:10151 [C4706] assignment within conditional expression > …\mdb.c:10215 [C4706] assignment within conditional expression > …\midl.c:123 [C4706] assignment within conditional expression > …\midl.c:149 [C4706] assignment within conditional expression Nathan: do we have a policy for how to handle such warnings? I can work with upstream to address them, but I expect that to be a extended process (with uncertain result, depending on how amenable the upstream maintainers are to accepting fixes for such warnings).

For Rust warnings, we just try to work with upstream. I think we have to just deal with the warnings from the C code as well, though I thought Ted was making it so the compiler that Cargo thinks it should be invoking didn't warn for many things?

(In reply to Nathan Froyd [:froydnj] from comment #2) > For Rust warnings, we just try to work with upstream. I submitted https://github.com/mystor/synstructure/pull/17 to fix these upstream.

It looks like third_party/rust/bzip2-sys/build.rs disables warnings when building libbz2: https://searchfox.org/mozilla-central/source/third_party/rust/bzip2-sys/build.rs#7 Ted: would that be a reasonable solution here?

(In reply to Nathan Froyd [:froydnj] from comment #2) > For Rust warnings, we just try to work with upstream. > > I think we have to just deal with the warnings from the C code as well, > though I thought Ted was making it so the compiler that Cargo thinks it > should be invoking didn't warn for many things? I disabled warnings-as-errors for C code compiled via Rust build scripts in bug 1409276, but I didn't disable warnings: https://hg.mozilla.org/mozilla-central/rev/5f20a8a55268 (In reply to Myk Melez [:myk] [@mykmelez] from comment #4) > It looks like third_party/rust/bzip2-sys/build.rs disables warnings when > building libbz2: > > https://searchfox.org/mozilla-central/source/third_party/rust/bzip2-sys/ > build.rs#7 > > Ted: would that be a reasonable solution here? That seems fine since it's all third-party code anyway, but my patches in bug 1409276 probably make us enable warnings anyway since we'll now be passing down all our CFLAGS, which include a bunch of -Whatever. Unfortunately I don't think it'd be straightforward to remove warning flags from our set of cflags for Rust compiles, as they're currently just jammed into CFLAGS in old-configure: https://dxr.mozilla.org/mozilla-central/rev/4e56a2f51ad739ca52046723448f3129a58f1666/old-configure.in#4371

Indeed, tracing the code, it looks like the cc crate first populates Tool::args from CFLAGS: https://github.com/alexcrichton/cc-rs/blob/master/src/lib.rs#L1122-L1124 And then adds a warning arg if warnings are enabled (which they are by default): https://github.com/alexcrichton/cc-rs/blob/master/src/lib.rs#L1297-L1300 But it doesn't remove a warning arg from Tool::args if warnings are explicitly disabled via `Build::warnings(false)`. So a warning arg in CFLAGS overrides a call to `Build::warnings(false)`. Assuming acrichto (cc:ed) is amenable, we could use the "removed_args" feature that jdm landed recently in the cc crate to make the crate strip out the warning arg if warnings are explicitly disabled: https://github.com/alexcrichton/cc-rs/commit/d37bd811a2319fe5b4c78860cb355b047798791b And then I could ask the maintainer of the lmdb-sys crate to explicitly disable warnings; and fork that crate to make the change myself if they decline. Alternately, we could decide to accept the regression in warning count, since it's in third-party code (twice over) rather than our own; although I'm unsure who would make that decision. Ted: do you have a preference or a third alternative?

It seems difficult to get this right from the perspective of the build script or the cc crate. It's kind of on us for passing all those flags in in CFLAGS. I'd say we either fix this by giving ourselves a way to elide warnings flags from moz.build (and then using it in moz.build files where we build Rust code), or we just take the warnings regression. The whole point of tracking warnings is just to try to keep build output readable, so it'd be unfortunate to do that of course. After writing comment 5 I realized that we probably could move `WARNINGS_CFLAGS` out of old-configure, since the only thing that happens there is we add `-Qunused-arguments` to it when compiling with clang, then it gets inserted into `CFLAGS`. We could instead set a `WARNINGS_CFLAGS` var and plumb that through the `COMPILE_FLAGS` setup so we could do something like `COMPILE_FLAGS['WARNINGS'] = []`.

FWIW I'd be definitely willing to have `cc` updated to strip out any flag that looks like a warning from CFLAGS if warnings are disabled. I agree with Ted that it's sort of tricky here but it seems like it could help at least for `cc` to strip those out! (I'm not sure if categorizing with 100% fidelity is possible, but we can tweak the heuristics over time)

MozReview-Commit-ID: 9CZgLGbWjbA

(In reply to Ted Mielczarek [:ted] [:ted.mielczarek] from comment #7) > After writing comment 5 I realized that we probably could move > `WARNINGS_CFLAGS` out of old-configure, since the only thing that happens > there is we add `-Qunused-arguments` to it when compiling with clang, then > it gets inserted into `CFLAGS`. We could instead set a `WARNINGS_CFLAGS` var > and plumb that through the `COMPILE_FLAGS` setup so we could do something > like `COMPILE_FLAGS['WARNINGS'] = []`. I have a patch that does this more or less, I think. It's hard to say for sure because I haven't been able to reproduce the warnings locally (and also because I'm unfamiliar with the build system code). I've pushed it to tryserver to see how it does there: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b076febb54f128543737435183d8fcefe4639bdd Note: the patch sets separate WARNINGS_CFLAGS and WARNINGS_CXXFLAGS vars because old-configure distinguishes between them, and Rust crates can compile both C and C++ code (although this particular regression is the result of C code compilation). Not asking for review yet, pending results on tryserver and further attempts to reproduce locally.

(In reply to Alex Crichton [:acrichto] from comment #8) > FWIW I'd be definitely willing to have `cc` updated to strip out any flag > that looks like a warning from CFLAGS if warnings are disabled. I agree with > Ted that it's sort of tricky here but it seems like it could help at least > for `cc` to strip those out! (I'm not sure if categorizing with 100% > fidelity is possible, but we can tweak the heuristics over time) Would you instead be amenable to `cc` not enabling warnings by default if CFLAGS is defined? Currently, `cc` enables warnings by default, even if CFLAGS is defined, which means that each individual crate must explicitly disable them via `Build::warnings(false)`. That's straightforward for a crate that Mozilla maintains, like bzip2-sys, where you disabled warnings in https://github.com/alexcrichton/bzip2-rs/commit/fd2c7257ce8d15d20b25dd0b8db3868fe9b9895c. But it's trickier for a true third-party crate like lmdb-sys, whose author may disagree about disabling warnings. Since Mozilla's build system defines CFLAGS, it has presumably already determined the warning flags that it wishes to set. And, if my patch here is successful, it won't set those flags when compiling Rust crates. Thus it would be helpful if `cc` didn't override that choice by enabling warnings anyway, so we don't have to land changes to disable warnings upstream into lmdb-sys and every other third-party crate that compiles C code.

Seems reasonable to me!

Myk, are you proceeding as stated, then, in #c11?

(In reply to David Durst [:ddurst] (REO for 63) from comment #13) > Myk, are you proceeding as stated, then, in #c11? Yes, that's right. I requested https://github.com/alexcrichton/cc-rs/pull/342 for the upstream changes in the cc crate that are needed to support this approach, and acrichto has merged those changes. I also updated the patch in https://phabricator.services.mozilla.com/D3939 and requested review from Ted, who redirected the request to Chris Manchester. There's still an issue on Windows that I need to resolve, but overall the approach seems viable, so it's the one I'm pursuing.

Update: I resolved the Windows issue in https://phabricator.services.mozilla.com/D3939 and requested publication of a new version of the cc crate to crates.io in https://github.com/alexcrichton/cc-rs/pull/343 so that I can then vendor it into mozilla-central.

Comment on attachment 9002975 [details] Bug 1482810 - set COMPILE_FLAGS var to hide warnings for Rust crates Chris Manchester (:chmanchester) has approved the revision.

The fix for bug 1482810 requires an upstream change to the cc crate: if CFLAGS is defined, then don't enable warnings by default. That change was included in cc crate version 1.0.23, and this change vendors that version of the cc crate into mozilla-central. MozReview-Commit-ID: Krfrs1dSN9d

Comment on attachment 9005260 [details] Bug 1482810 - update cc crate from 1.0.18 to 1.0.23 Nathan Froyd [:froydnj] has approved the revision.

Pushed by myk@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b9197ece7955 set COMPILE_FLAGS var to hide warnings for Rust crates r=chmanchester

Pushed by myk@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7eae34bda99c update cc crate from 1.0.18 to 1.0.23 r=froydnj

https://hg.mozilla.org/mozilla-central/rev/b9197ece7955 https://hg.mozilla.org/mozilla-central/rev/7eae34bda99c