Closed Bug 1516829 Opened 6 years ago Closed 6 years ago

Warnings under servo/ break build with Rust Nightly and --enable-warnings-as-errors

Categories

(Core :: CSS Parsing and Computation, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: jbeich, Assigned: xidorn)

References

Details

Attachments

(1 file)

Stylo builds a lot of vendored code under servo/ directory. Due to high churn rate there it'd probably be better to disable warnings like under third_party/rust/. $ rustc -vV rustc 1.33.0-nightly binary: rustc commit-hash: 60e825389db3 commit-date: 2018-12-28 host: x86_64-unknown-freebsd release: 1.33.0-nightly LLVM version: 8.0 $ echo "ac_add_options --enable-warnings-as-errors # mimic MOZ_AUTOMATION" >>.mozconfig $ ./mach build [...] error: use of deprecated item 'bitflags::core::str::<impl str>::trim_right': superseded by `trim_end` --> servo/components/style/values/computed/font.rs:781:15 | 781 | slice.trim_right().to_css(dest) | ^^^^^^^^^^ | = note: `-D deprecated` implied by `-D warnings` error: use of deprecated item 'bitflags::core::str::<impl str>::trim_left_matches': superseded by `trim_start_matches` --> servo/components/style/values/specified/position.rs:695:27 | 695 | let rest = self.0.trim_left_matches(HTML_SPACE_CHARACTERS); | ^^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors error: Could not compile `style`.
Version: 60 Branch → Trunk
Attachment #9033676 - Attachment description: Bug 1516829 - Replace trim_{left,right}* with trim_{start,end}*. #style → Bug 1516829 - Replace trim_{left,right}* with trim_{start,end}*.
(In reply to Jan Beich from comment #0) > Stylo builds a lot of vendored code under servo/ directory. Due to high > churn rate there it'd probably be better to disable warnings like under > third_party/rust/. The code there is not vendored, fwiw. > $ echo "ac_add_options --enable-warnings-as-errors # mimic MOZ_AUTOMATION" > >>.mozconfig Well, we build with --enable-warnings-as-errors, but not with rust nightly. The same happens when clang introduces a warning if you use a clang version newer than what we use on automation, right?
Pushed by mozilla@upsuper.org: https://hg.mozilla.org/integration/autoland/rev/a22cf2ec4f2d Replace trim_{left,right}* with trim_{start,end}*. r=emilio
Maybe we can setup a Tier-3 task to build and test with nightly and/or beta, so that we can find this kind of issues quickly. This would help us tracking upstream changes closely, and help contributors who may use newer versions to be able to build most of the time. Gecko's Rust codebase would also be a good real world testcase for Rust, which can help Rust team as well. I recall that Travis CI's Rust page mentions[1]: > The Rust team appreciates testing against the beta and nightly channels, even if you are only targeting stable. Although most of the time our experience with Rust upgrade was good, I still recall there was once that we faced several regressions which caused us not able to bump the Rust version for several (Rust) releases. Testing latest toolchain should help with that. This doesn't need to run very frequently, so a T3 task on m-c should be enough. WDYT? [1] https://docs.travis-ci.com/user/languages/rust/
(In reply to Xidorn Quan [:xidorn] UTC+11 from comment #5) > This would help us tracking upstream changes closely, and help contributors > who may use newer versions to be able to build most of the time. Well sure, though that only helps people that build with --enable-warnings-as-errors, which I think it's a known may-break flag. > Although most of the time our experience with Rust upgrade was good, I still > recall there was once that we faced several regressions which caused us not > able to bump the Rust version for several (Rust) releases. Testing latest > toolchain should help with that. Those were mostly perf regressions right? So they wouldn't be caught by this. > This doesn't need to run very frequently, so a T3 task on m-c should be > enough. WDYT? Sounds fine in any case :)
Assignee: nobody → xidorn+moz
(In reply to Emilio Cobos Álvarez (:emilio) from comment #6) > (In reply to Xidorn Quan [:xidorn] UTC+11 from comment #5) > > This would help us tracking upstream changes closely, and help contributors > > who may use newer versions to be able to build most of the time. > > Well sure, though that only helps people that build with > --enable-warnings-as-errors, which I think it's a known may-break flag. Yeah, but I believe people are, or should be, encouraged to use that since that's what we do on CI, otherwise they may need more rounds to get code in since there would be a larger chance to see failures not present locally. > > Although most of the time our experience with Rust upgrade was good, I still > > recall there was once that we faced several regressions which caused us not > > able to bump the Rust version for several (Rust) releases. Testing latest > > toolchain should help with that. > > Those were mostly perf regressions right? So they wouldn't be caught by this. Perf regressions can still be caught with talos if we run that :) I think there were something related to backtrace or symbol stuff... I forgot exactly what was that, though. > > This doesn't need to run very frequently, so a T3 task on m-c should be > > enough. WDYT? > > Sounds fine in any case :) I'll file a bug :)
Summary: Warnings under servo/ break build with Rust Nightly → Warnings under servo/ break build with Rust Nightly and --enable-warnings-as-errors
Priority: -- → P2
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: