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)
Core
CSS Parsing and Computation
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`.
Assignee | ||
Comment 1•6 years ago
|
||
Updated•6 years ago
|
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}*.
Comment 2•6 years ago
|
||
(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?
Assignee | ||
Comment 3•6 years ago
|
||
Pushed by mozilla@upsuper.org:
https://hg.mozilla.org/integration/autoland/rev/a22cf2ec4f2d
Replace trim_{left,right}* with trim_{start,end}*. r=emilio
Assignee | ||
Comment 5•6 years ago
|
||
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/
Comment 6•6 years ago
|
||
(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 :)
Updated•6 years ago
|
Assignee: nobody → xidorn+moz
Assignee | ||
Comment 7•6 years ago
|
||
(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 :)
Updated•6 years ago
|
Summary: Warnings under servo/ break build with Rust Nightly → Warnings under servo/ break build with Rust Nightly and --enable-warnings-as-errors
Updated•6 years ago
|
Priority: -- → P2
Comment 8•6 years ago
|
||
bugherder |
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.
Description
•