Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Build depends on forked rust-cssparser #20080

Closed
rillian opened this issue Dec 13, 2021 · 0 comments · Fixed by brave/brave-core#11582
Closed

Build depends on forked rust-cssparser #20080

rillian opened this issue Dec 13, 2021 · 0 comments · Fixed by brave/brave-core#11582
Assignees
@rillian
Labels
OS/Android Fixes related to Android browser functionality OS/Desktop QA/No release-notes/exclude
Milestone
1.36.x - Release

Comments

@rillian
Copy link

rillian commented Dec 13, 2021

Description

We override the cssparser crate, which is a dependency of some of our rust code, to use a forked version working around a linkage issue with glibc. This makes maintenance harder. We should migrate to the published upstream version, or if that remains infeasible, move to a fork under the brave organization and set up a better audit/update process for it.

Steps to Reproduce

  1. $ grep git+http src/brave/build/rust/Cargo.lock
  2. Observe source lines pointing to a personal repo.

Actual result:

source = "git+https://github.com/AndriusA/rust-cssparser?branch=glibc#ad2f4d3c89e247ec1693a581ecace199e46f30fa"

Expected result:

source = "registry+https://github.com/rust-lang/crates.io-index" or at least something like source = "git+https://github.com/brave/rust-cssparser?branch=glibc#ad2f4d3c89e247ec1693a581ecace199e46f30fa".

Reproduces how often:

Always

Desktop Brave version:

1.35.27

Android Device details:

  • Install type (ARM, x86):
  • Device type (Phone, Tablet, Phablet):
  • Android version:

Version/Channel Information:

  • Can you reproduce this issue with the current release? yes
  • Can you reproduce this issue with the beta channel? yes
  • Can you reproduce this issue with the nightly channel? yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields?
  • Does the issue resolve itself when disabling Brave Rewards?
  • Is the issue reproducible on the latest version of Chrome?

Miscellaneous Information:
@rillian rillian added QA/No release-notes/exclude OS/Android Fixes related to Android browser functionality OS/Desktop labels Dec 13, 2021
@rillian rillian self-assigned this Dec 13, 2021
rillian added a commit to brave/brave-core that referenced this issue Dec 13, 2021
@rillian
Remove rust-cssparser diversion.
45c901b 
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
@rillian rillian mentioned this issue Dec 13, 2021
Merged
25 tasks
rillian added a commit to brave/brave-core that referenced this issue Jan 26, 2022
@rillian
Remove rust-cssparser diversion.
11fadbd 
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
@rillian rillian added this to the 1.36.x - Nightly milestone Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rillian rillian

Labels
OS/Android Fixes related to Android browser functionality OS/Desktop QA/No release-notes/exclude
Projects
None yet
Milestone
1.36.x - Release
Development

Successfully merging a pull request may close this issue.

Remove rust-cssparser diversion. brave/brave-core
1 participant
@rillian