1563531 - Settle the megazording discussion

Status: RESOLVED FIXED

(Data Platform and Tools :: Glean: SDK, task, P3)

Description

[Alessio Placitelli [:Dexter]]

We need to figure out how to megazord with the other Rust libraries, otherwise the plan discussed in bug 1563527 can't be executed. Moreover, without megazording, iOS won't be possible.

Comment 1

[Alessio Placitelli [:Dexter]]

Hey Thom!

Were you able to collect a few of the problems about megazording / different repos in a document?

Comment 2

[Thom Chiovoloni [:tcsc] (ex-moco)]

I've been a bit busier than expected, but wrote the following shortly after all hands, and just cleaned it up. It's probably not 100% complete, but I think it hits many of the technical challenges. I can move it to a google doc if necessary, but at least having it on bugzilla means it's easier to find for people in the future (not to mention, openness, etc).

I also have abstained from listing any of the things I think we'd get from having a shared repo. I've tried to hide my biased opinion here, but probably failed. I've also generally not gone in depth on ergonomic issues, since it's hard to know how bad they'd be in practice, but I suspect rather bad.

Note: This is all assuming the changes happen in application-services to remove non-megazord builds. That's tracked by https://github.com/mozilla/application-services/pull/1103, and should land somewhat soon. Some WIP docs on this are available here: https://github.com/mozilla/application-services/blob/558d8f0ccb91770c14221943e890bd9ab1ecdc16/docs/design/megazords.md

Anyway, the problems:

1. Dependency order issues:

   • The Rust crate for each megazord needs to depends on all the smaller FFI crates, in order to re-export those symbols.

     Conversely, the android package containing the bindings for each component must depend on the depend on the megazord package.

     (To be clear, the dependencies go in opposite directions for Rust and Java/Kotlin. This may seem unintuitive, but solves many problems and more accurately represents the actual dependency graph).

   • This gets more complex when you consider the code (outside of pure-rust things which can probably be done via crates.io) we'd want to share:

     • Our viaduct network layer: a rust crate exposing a fetch-like API that can be configured by android and use geckoview to perform the fetch.

       • To be clear, this includes FFI and an android component and is part of the megazord, in addition to being a plain rust crate.
       • Note that if we end up with multiple copies of that crate (due to cargo resolving the semver mismatches by duplication), we'd have... problems.

     • The actual megazord setup android library, needed by every android component binding (native_support).

     • Probably our rust NSS bindings in the future...

2. It's unclear how repositories that don't contain the megazord will be able to unit test their android code that calls rust, especially given the dependency issues in #1.

   • Additionally, unit testing android rust bindings when using gradle substitutions to depend on the megazord is very difficult, if it's possible at all.

3. We lose the ability to test that the versions of the android bindings match the version of the rust code inside the megazord. In practice this has bitten us before, and is a memory safety issue.

4. Bunch of smaller issues:

   • Lots of duplicated infrastructure, even in the best case.

   • Our swift setup is currently a single xcodeproj, which makes life easy for our consumers, but seems incompatible here (not sure how many care about firefox-ios)

   • Harder to audit for bugs that could occur in all crates -- We've had times where we've wanted to do a sweep of the FFI code to look for a potential problem.

   • In all likelihood, a lot more ./gradlew publishToMavenLocal will be needed, as gradle substitutions are not always viable, unfortunately.

Comment 3

[Alessio Placitelli [:Dexter]]

(In reply to Thom Chiovoloni [:tcsc] from comment #2)

I've been a bit busier than expected, but wrote the following shortly after all hands, and just cleaned it up. It's probably not 100% complete, but I think it hits many of the technical challenges. I can move it to a google doc if necessary, but at least having it on bugzilla means it's easier to find for people in the future (not to mention, openness, etc).

Thanks for working on this and listing them out! I've added them to a document for easier commenting and discussion, the doc itself us publicly accessible: https://docs.google.com/document/d/1ky7IXHAc3EOBkzyd8yszbX4Rt8d_lDvjDj03BEX1o2o

I also have abstained from listing any of the things I think we'd get from having a shared repo. I've tried to hide my biased opinion here, but probably failed. I've also generally not gone in depth on ergonomic issues, since it's hard to know how bad they'd be in practice, but I suspect rather bad.

Sure, we can get to that after we better document the issues! Thanks for kicking this off!

Note: This is all assuming the changes happen in application-services to remove non-megazord builds. That's tracked by https://github.com/mozilla/application-services/pull/1103, and should land somewhat soon. Some WIP docs on this are available here: https://github.com/mozilla/application-services/blob/558d8f0ccb91770c14221943e890bd9ab1ecdc16/docs/design/megazords.md

Does this mean the problems will be different? Is there any chance that the mentioned changes won't stick?

Comment 4

[Thom Chiovoloni [:tcsc] (ex-moco)]

Does this mean the problems will be different?

The issues we'd face without that work are both different and larger. I didn't bother listing them since I haven't thought as much about it and it would be a distraction, since we expect that branch to land soon (I had expected that branch to have landed by now when I wrote that, but it got delayed a little by both a desire to not make massive changes that might complicate uplifting hotfixes for the Fenix release, and because some higher priority items came up unexpectedly)

Is there any chance that the mentioned changes won't stick?

At this point, no. I expect it to land early next week. Other features we need (choose what to sync) require removing support for the non-megazorded builds, so it's not just a cleanup / refactoring.

Comment 5

[Jan-Erik Rediger [:janerik]]

We created an overview document with the problem statement and possible solutions: https://docs.google.com/document/d/1LQhgXR9IcXQkovWDJrq-KQOrLSiQMOyb8XRrccGOTuc/view

We had a call with Thom and we will move forward with:

• Investigating the code size of Glean
• Write a proposal on how we move forward.

Closing this bug and tracking it in the follow-ups.