-
Notifications
You must be signed in to change notification settings - Fork 879
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Combine rust libs #6408
Combine rust libs #6408
Conversation
655227f
to
9f461d9
Compare
warning: unused variable: `origin` --> /Users/jenkins/jenkins/workspace/pr-brave-browser-combine-rust-libs-ios/src/brave/components/speedreader/rust/lib/src/rewriter_config_builder.rs:40:5 | 40 | origin: &str, | ^^^^^^ help: consider prefixing with an underscore: `_origin` | = note: `#[warn(unused_variables)]` on by default Compiling speedreader-ffi v0.1.0 (/Users/jenkins/jenkins/workspace/pr-brave-browser-combine-rust-libs-ios/src/brave/components/speedreader/rust/ffi) Compiling adblock-ffi v0.1.0 (/Users/jenkins/jenkins/workspace/pr-brave-browser-combine-rust-libs-ios/src/brave/vendor/adblock_rust_ffi)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delegating to @AndriusA , I'm on PTO and can't take a look
"//base", | ||
] | ||
|
||
if (is_mac || is_ios ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: extra space before )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This did pass windows at one point, but now I keep getting various errors that seem to be related to windows path length limitations
I'm going to merge and see if the problem shows up in nightly which should have a shorter path ( |
We were patching the rust cssparser crate to work around an issue with `f64::powf` linking to symbols from the system glibc which couldn't be satisfied by the sysroot the official builds link against. This has since been addressed by having cargo target the same sysroot, I think by #6408, so we can remove the diversion and use the upstream crate which lets us update more easily and benefit from shared audits. Resolves brave/brave-browser#20080
We were patching the rust cssparser crate to work around an issue with `f64::powf` linking to symbols from the system glibc which couldn't be satisfied by the sysroot the official builds link against. This has since been addressed by having cargo target the same sysroot, I think by #6408, so we can remove the diversion and use the upstream crate which lets us update more easily and benefit from shared audits. Resolves brave/brave-browser#20080
Resolves brave/brave-browser#6515
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist:
After-merge Checklist:
changes has landed on.