Skip to content

London Oxidation

Jump to bottom
Lars Bergstrom edited this page Jun 17, 2016 · 1 revision

Repo integration for vendoring Servo in m-c

  • bholley: Can we use the autolander or a mozilla-servo that just handles the instant merges with servo? Or should we use the autolander to sync between github and m-c?
  • glandium: Not sure that removing m-i is happening on the timeline we'd like. So, shouldn't plan on that.
  • bholley: Can temporarily use an integration branch for the Servo stuff rather than trying to land on inbound & get the backouts, etc. Sherrifs already can handle fx-team and inbound.

Goals

  • bholley: share code between servo & gecko, working in either repo without additional costs but still get all the benefits.

Ergonomics

  • bholley: Really want to make atomic changes across the gecko, servo, and non-servo code. People {like/have} to do large cross-cutting changes. Don't want to make separate changes and bump upstream versions.
  • wycats: Expressing it as atomic is important.
  • jack: Some pieces like rust-cssparser & rust-selectors are on crate.io.
  • bholley: Already a hassle for Servo. Drifted towards all of the cross-cutting stuff living in servo/servo.
  • wycats: Atomic commits might be atomic in the gecko repo, but would not be atomic showing up in the ecosystem. So, the code history will be fine, but there might be an upstreaming issue.
  • larsberg: We typically break things out to crates.io when there is a third party.
  • jack: The workflow is a little rough - push change + version bump to rust-cssparser, then publish to crates.io, then pull into servo/servo via a cargo update.
  • wycats: There's just a synchronization problem when you are upstreaming that your upstream pushes may not be pushed correctly.
  • gps: This definitely will cause conflicts.
  • wycats: That's the conflict between having atomic commits cross-repo, but not actually owning the upstream.
  • bholley: That's why I want to own servo/servo.
  • gps: Worried that once we start vendoring things in m-c and have big commits, where does code review happen?
  • bholley: We have a commit hook for a servo-appropriate reviewers. Need to have them know about the impact on their changes on Gecko.
  • wycats: Like having a PR onto servo/servo.
  • gps: Tension between project history if you're on m-c or servo side. Want more Firefox stuff happening in bugzilla, mozreview, etc.?
  • wycats: Seems incompatible with Github packages.
  • gps: Fragmenting code review feels weird.
  • wycats: Mainly because the review in GitHub servo/servo is not happening in m-c.
  • bholley: Idea is that we have to sync that.
  • gps: There's an issue with commits from untrusted parties, backdoors being inserted, having as much stuff as possible happening... what about dbryant/dougt?
  • glandium: We have this problem with webrtc.
  • gps: Trust of third party code is a hard problem.
  • wycats: Is the claim that m-c review will catch security bugs?
  • gps: Possibly want GPG signed code reviews.
  • wycats: What makes servo/servo make Mozilla Security people happy.
  • jack: We can make things harder for reviewers, but not for contributors.

m-c

  • wycats: Multiple branches?
  • gps: There's one head branch - m-c. Pretend that it's linear and everything comes from autoland. That's where we're going.
  • bholley: Merges happen from m-i to m-c.
  • gps: We have integration branches due to tree closures. Trying to move away from it just to m-c.
  • wycats: {describes github PR process}
  • gps: Similar to github. Do a clone of m-c. Push commits to the mozreview "mr" repo. Has thousands of heads. Those turn into review requests in mozreview. From there, get submitted to the try servers. Once you get r+, can click a button and the autolander, rebases them onto m-c, and lands them.
  • bholley: I think we're all aware of this problem and that the solution to this problem is that we use the same gate/autolander to push to m-c and servo/servo atomically.
  • wycats: Missing some details. Problem is that there are two inputs in the autolanding linearizer. So, how do we do this automatically with minimally manually landed comments? I think that having a project with an embedded servo/servo in it is fine. The question is: how does the PR process work.
  • jack: Two choices. 1) land code from the gecko side, do we open a PR on the servo/servo side and use the same process for it? seems like work, too, as then mozreview & servo review has to sync. 2) reviewers have to handle both mozreview and github reviews. And if it's reviewed we land it in servo/servo without creating a "fake" PR on the servo side.
  • larsberg: Yes, that's the idea.
  • wycats: So the autolander needs to push to both branches.
  • gps: What do you do with a failure in m-c and the commit is backed out?
  • jack: That's a fundamental problem.
  • wycats: We do now have the linear history in two places at once nightly.
  • gps: We can't test autoland to commit+test before land because it takes > 1.5 hours especially if you need PGO builds. Now we land on m-i and it eventually lands on m-c. Developers are supposed to work off m-c. Trying to get to test the commits on a branch, then rebase that to central. Then, central is your golden tip.
  • bholley: Re-run tests after the merge?
  • gps: Not a merge; rebase.
  • edunham: Can we parallelize the CI better to land it all.
  • gps: Doesn't scale.
  • jack: Rust has a higher commit velocity, but manages the changes with rollups.
  • gps: Don't like rollups b/c they break commit-level bisection.
  • wycats: What's the FINAL branch that doesn't get backed out and is fully tested?
  • gps: Ideally, that will be m-c.
  • wycats: Maybe there's another step after m-c which is more stable and tested?
  • gps: That should eventually be m-c.
  • wycats: Concerns?
  • jack: What happens when the tree is closed?
  • gps: They last hours to days. There's also infra closures for AWS, GitHub, etc.
  • jack: Not really a big deal if it's infra - that just happens and it's behind the autolander.
  • gps: Tree is closed for one of two reasons: 1) infra issues; 2) bad code & need a backout.
  • edunham: So we land to a special mozilla-servo branch & push from there.
  • gps: Tree may be closed if there are infra issues.
  • jack: That's fine.
  • wycats: Just need to ensure that m-c does not edit the servo bits.
  • gps: What about atomic bits across multiple projects? Then the ff gets confusing.
  • larsberg: I think an integration repo is the right place to be, for now.
  • gps: We want to get rid of this. Land to autoland repo.
  • bholley: What's eventually?
  • gps: I'd be surprised if it's done in 2016.
  • bholley: Then we should do something in the interim.
  • gps: Or the bot can land to the autoland repo. Two autoland bots.
  • edunham: What does your bot look like?
  • gps: Python demon that listens to webservice requests.

Style system

  • jack: How do we not avoid rippled out changes from the style system in servo that breaks gecko?
  • wycats: Servo needs to use version numbers. Gecko might have to be behind for a while. If servo needs to make a breaking change, it should not force downstream to take it.
  • bholley: The whole point is NOT to have the versioning. If you are making a change to the style system that breaks gecko, you need to fix it.
  • larsberg: This is the whole goal of the integration. So that all changes are broken.
  • wycats: Why can't we pause?
  • gps: This is the whole reason we have a monorepo. There are standalone projects, but Gecko drives them.

MSVC status

  • Google is working on emitting CodeView information from LLVM

  • They currently have very basic type support

  • No debug symbols for libstd/libcore

Just build a rust with debug info / missing line number. Google is working on proper PDB support. Add that support & we'll have basic debugging support and get line number support / look at variables. That we can do.

Jeff has looked into adding initial support for line numbers in code view, but first attempt. Need $$$ to land patches in LLVM. Linux has debug info because rillian creates a custom Rust build for Linux for tooltool.

TODO: Package up the PDB. So we need to package it up.

Crash reports

Crash reports caused by panics in Rust code are inadequate (bug https://bugzilla.mozilla.org/show_bug.cgi?id=1268328 ).

TODO: make the panic handler pass along the file and line number in the NS_EXCEPTION.

Just need to recognize rust_begin_unwind.

TODO(ted): do that.

Mangling is all GNU. ehsan thinks we should really try to do MSVC mangling on Windows. TODO(alex?): Use the clang code or call into it for MSVC name mangling

Android

Android - ARM target built with s/w float. Need rust target w/ h/w float. Even if it uses s/w arg passing.

There is an armv7 target for android. There's no standard library target. It's on nightly. Can uplift to Beta if we need it sooner.

TODO(alex):that

Q: armv6? A: no.

Q: Android binary size? Does LTO work?

A: LTO threw out everything other than 8k w/o

TODO: A try run and check out

Allocations

jemalloc: not shared on Windows; it is on other platforms.

Q: what's the blocker here?

Alex: call heapalloc/heapfree on Windows. Gecko can redirect to them.

There's a nightly solution that we could stabilize if required. Do you need to reallocat.

glandium: We need to do the redirect on the Gecko side for that same reason.

TODO: glandium will test the override, otherwise we'll need the rust thing

Cargo

how do we make the "build an rlib thing better?"

TODO(nfroydj): switch to cargo to build Rust code

  • blocker: can't hit the network. maybe just path dep everything when vendoring
  • There is a "cargo vendor" tool.

gps: I don't want to use cargo build. Not on the path to using bazel. Build systems embedded in build systems are annoying.

alex: Dropbox has the same problem. They use bazel & cargo. Plan is for cargo to generate a build file that bazel drives. We'll fix the integration bugs there.

wycats: Same opinion here. Other big companies need it.

rust-bindgen at build time

servo style system needs bindings between c++ and rust

Run it manually today. rust-bindgen has a LLVM & rust-nightly dependency.

We use clang to parse C++. libllvm & libclang.

gps: Once we force people to have libclang, there are other things we can do there.

emilio: SM has the same problem. They have to run bindgen on CI. One idea was stick the binary of bindgen on tooltool there.

ted: concerned about end users here.

TODO: try to figure out how to do this

when do we start requiring rust for developer builds as well as automation?

"Just a policy decision"

blocks on Android working properly

blocks on Windows crash reporting being better

possibly blocks on jemalloc integration

question about whether we're going to require libclang and rust-bindgen

add something to mach bootstrap

gps: Don't like that our dev builds are different from release.

ted: Turn on --enable-rust by default, leaving --disable-rust?

TODO: do that next week? (ted: I have a patch)

TODO: add bootstrapping stuff first

SIMD

hsivonen: looking at SIMD crate; it has LLVM SSE2 intrinsics, exposed to rustc. Then magic around the load instructions from memory to registers. Is this available on the stable channel?

alex: No, not yet.

hsivonen: When is SIMD & intrinsics coming?

alex: at least 3 months for trains coming to stable. Can accelerate if it's important.

Updating rust compiler versions

Q: Rust compiler version? Latest stable?

ted: OK with latest stable for now. Want to back off at some point to reduce the churn rate on the compiler.

wycats: maybe once we have an LTS build

ms2ger: it's to let people build on the older version locally

ato: also don't want to keep churning versions in the CI

Rust/Servo in Gecko

  • See document for current plan: https://docs.google.com/document/d/1uubYE7JXaVY10PoAY9BVx8A-T11ZxP1RYqNOrFJwdcU/edit#

  • High-level overflow of the key servo bits is:

    • The main servo/servo repo is vendored into m-c
    • A single autolander runs the Servo and Gecko tests for all Servo GitHub PRs and m-c autolander submissions
    • On pass, the autolander pushes the commit to m-c (or an autolander branch) and servo/servo GitHub master

  • The broader Cargo ecosystem is handled through the typical Cargo workflow (edit, land upstream, update Cargo version)

    • In the case of a chemspill, can commit to the vendored code, it will land in a mozilla fork & branch of the upstream repo, and manually turned into a PR, which may require manual rebase before it lands and follows the update workflow back to m-c

  • Gecko believes in trusting a try build even if master has moved forward and lands it. Servo always tests against master

    • Would Gecko like to push towards speeding up their try builds & test runs more to be able to test against master?

Questions

  • Where do the forks of upstream repos live? A special mozilla-gecko GitHub Organization?
    • Will local forks be the default, and how to handle upstreaming?
Clone this wiki locally