1513009 - Deny Rust warnings by default in automation.

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1513009 - Deny Rust warnings on automation.

I've chosen linux64-debug since it's the most visible build I usually do, but I
could do another build task or something, or use the static analysis builds, or
what not. Just let me know if there's a better way to do this.

Caveat: This might make updating Rust toolchains a bit more painful. I think
this is better and we should just deal with warnings before updating toolchains,
but I don't know if there'd be strong opposition to that.

Proof that it works:

 * https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ad1e4e1392f71b574cff683e90c7b13bf8781d1
 * https://treeherder.mozilla.org/#/jobs?repo=try&revision=57604f92624bbe49037eee87c56fdb6bf2b5017d

Comment 2

[(not currently active) Ted Mielczarek]

This might get frustrating with vendored third-party crates, though. I wonder if we can find a way to enable this only for our first-party crates? We use a lot of `AllowCompilerWarnings()` for third-party C++ code because getting warnings fixed in that kind of code can be a hassle and it's annoying to do a toolchain update or update to a newer version of some third-party library only to have the build fail because of new warnings. With Rust crates it'd be even worse--you could be pulling in a new crate dependency or updating an existing crate version and hit the same situation.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

(In reply to Ted Mielczarek [:ted] [:ted.mielczarek] from comment #2)
> This might get frustrating with vendored third-party crates, though. I
> wonder if we can find a way to enable this only for our first-party crates?
> We use a lot of `AllowCompilerWarnings()` for third-party C++ code because
> getting warnings fixed in that kind of code can be a hassle and it's
> annoying to do a toolchain update or update to a newer version of some
> third-party library only to have the build fail because of new warnings.
> With Rust crates it'd be even worse--you could be pulling in a new crate
> dependency or updating an existing crate version and hit the same situation.

Hmm, I don't think third party code is a problem in practice, since cargo does pass --cap-lints warn to rustc, so it will never error.

From the log for example:

[task 2018-12-10T15:08:22.735Z] 15:08:22     INFO -       Running `/builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/rustc/bin/rustc --crate-name libc /builds/worker/workspace/build/src/third_party/rust/libc/src/lib.rs --color never --crate-type lib --emit=dep-info,link -C opt-level=1 -C panic=abort -C debuginfo=2 -C debug-assertions=on --cfg 'feature="default"' --cfg 'feature="use_std"' -C metadata=aca576200cb12fcd -C extra-filename=-aca576200cb12fcd --out-dir /builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-unknown-linux-gnu/debug/deps --target x86_64-unknown-linux-gnu -C linker=/builds/worker/workspace/build/src/build/cargo-linker -L dependency=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-unknown-linux-gnu/debug/deps -L dependency=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./debug/deps --cap-lints warn -C opt-level=2 -C debuginfo=2 -Dwarnings`

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e348821b49f
Deny Rust warnings on automation. r=firefox-build-system-reviewers,ted

Comment 5

[Raul Gurzau (:RaulG)]

Backed out changeset 8e348821b49f (bug 1513009) due to build compile failure on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/autoland/rev/105fc79aee5ffe6b5f6a5973f3e4878cc4b8e198

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=8e348821b49fa0913c8ce9c5be9b8787a7d8f970

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=216912263&repo=autoland&lineNumber=13330

Log snippet: 


[task 2018-12-13T23:41:21.850Z] 23:41:21     INFO -  warning: use of deprecated item 'std::ascii::AsciiExt': use inherent methods instead
[task 2018-12-13T23:41:21.850Z] 23:41:21     INFO -    --> /builds/worker/workspace/build/src/third_party/rust/cssparser-macros/lib.rs:12:30
[task 2018-12-13T23:41:21.850Z] 23:41:21     INFO -     |
[task 2018-12-13T23:41:21.851Z] 23:41:21     INFO -  12 | #[allow(unused_imports)] use std::ascii::AsciiExt;
[task 2018-12-13T23:41:21.851Z] 23:41:21     INFO -     |                              ^^^^^^^^^^^^^^^^^^^^
[task 2018-12-13T23:41:21.851Z] 23:41:21     INFO -     |
[task 2018-12-13T23:41:21.851Z] 23:41:21     INFO -     = note: #[warn(deprecated)] on by default
[task 2018-12-13T23:41:21.852Z] 23:41:21     INFO -  error: unused import: `core_foundation_sys::dictionary::*`
[task 2018-12-13T23:41:21.852Z] 23:41:21     INFO -    --> dom/webauthn/u2f-hid-rs/src/macos/iokit.rs:15:5
[task 2018-12-13T23:41:21.852Z] 23:41:21     INFO -     |
[task 2018-12-13T23:41:21.852Z] 23:41:21     INFO -  15 | use core_foundation_sys::dictionary::*;
[task 2018-12-13T23:41:21.853Z] 23:41:21     INFO -     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[task 2018-12-13T23:41:21.853Z] 23:41:21     INFO -     |
[task 2018-12-13T23:41:21.853Z] 23:41:21     INFO -     = note: `-D unused-imports` implied by `-D warnings`
[task 2018-12-13T23:41:21.853Z] 23:41:21     INFO -  error: unused import: `core_foundation_sys::string::*`
[task 2018-12-13T23:41:21.853Z] 23:41:21     INFO -    --> dom/webauthn/u2f-hid-rs/src/macos/iokit.rs:17:5
[task 2018-12-13T23:41:21.854Z] 23:41:21     INFO -     |
[task 2018-12-13T23:41:21.854Z] 23:41:21     INFO -  17 | use core_foundation_sys::string::*;
[task 2018-12-13T23:41:21.854Z] 23:41:21     INFO -     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[task 2018-12-13T23:41:21.854Z] 23:41:21     INFO -  error: aborting due to 2 previous errors
[task 2018-12-13T23:41:21.854Z] 23:41:21    ERROR -  error: Could not compile `u2fhid`.
[task 2018-12-13T23:41:21.855Z] 23:41:21     INFO -  Caused by:
[task 2018-12-13T23:41:21.855Z] 23:41:21     INFO -    process didn't exit successfully: `/builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/rustc/bin/rustc --crate-name u2fhid dom/webauthn/u2f-hid-rs/src/lib.rs --color never --crate-type lib --emit=dep-info,link -C opt-level=1 -C panic=abort -C debuginfo=2 -C debug-assertions=on -C metadata=4aee4db08318c741 -C extra-filename=-4aee4db08318c741 --out-dir /builds/worker/workspace/build/src/obj-firefox/x86_64-apple-darwin/debug/deps --target x86_64-apple-darwin -C linker=/builds/worker/workspace/build/src/build/cargo-linker -L dependency=/builds/worker/workspace/build/src/obj-firefox/x86_64-apple-darwin/debug/deps -L dependency=/builds/worker/workspace/build/src/obj-firefox/debug/deps --extern bitflags=/builds/worker/workspace/build/src/obj-firefox/x86_64-apple-darwin/debug/deps/libbitflags-59f5ca3d9c98e954.rlib --extern boxfnonce=/builds/worker/workspace/build/src/obj-firefox/x86_64-apple-darwin/debug/deps/libboxfnonce-993ee7986736c14c.rlib --extern core_foundation=/builds/worker/workspace/build/src/obj-firefox/x86_64-apple-darwin/debug/deps/libcore_foundation-f9f632fc231bc48a.rlib --extern core_foundation_sys=/builds/worker/workspace/build/src/obj-firefox/x86_64-apple-darwin/debug/deps/libcore_foundation_sys-c427f7450932b930.rlib --extern libc=/builds/worker/workspace/build/src/obj-firefox/x86_64-apple-darwin/debug/deps/liblibc-50993e3425e943f1.rlib --extern log=/builds/worker/workspace/build/src/obj-firefox/x86_64-apple-darwin/debug/deps/liblog-42fe099e03144426.rlib --extern rand=/builds/worker/workspace/build/src/obj-firefox/x86_64-apple-darwin/debug/deps/librand-622971baeb92210f.rlib --extern runloop=/builds/worker/workspace/build/src/obj-firefox/x86_64-apple-darwin/debug/deps/librunloop-44e9415db1874575.rlib -C opt-level=2 -C debuginfo=2 -Dwarnings` (exit code: 1)
[task 2018-12-13T23:41:21.856Z] 23:41:21     INFO -  warning: build failed, waiting for other jobs to finish...

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Allow existing unused imports in u2f-hid-rs's mac stuff.

Which lets the osx build succeed with the other patch in this bug.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9031408 [details] [diff] [review]
Allow existing unused imports in u2f-hid-rs's mac stuff.

I'm not sure who should review this, whoever gets to it first I guess, since it's pretty trivial.

I thought of doing it in a more fine-grained way, but I don't have an OSX machine to test and this is OSX-only code.

Comment 8

[J.C. Jones [:jcj] (he/him)]

Comment on attachment 9031408 [details] [diff] [review]
Allow existing unused imports in u2f-hid-rs's mac stuff.

Mm, we should just vendor the next version of u2f-hid-rs that fixes this (and stops using core_foundation_sys entirely) (0.2.3) rather than patch in-tree. I opened Bug 1514247 for that and will file the patch today.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Cool, thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a1d7cb40a36
Fix a build warning in the stylo tests on linux32.
https://hg.mozilla.org/integration/autoland/rev/19447cbd145a
Deny Rust warnings on automation. r=ted

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/4a1d7cb40a36
https://hg.mozilla.org/mozilla-central/rev/19447cbd145a