libbrillo has forked. Expected? |
||||||
Issue descriptionI noticed libbrillo has forked between Android and Chromeos around 7/16. 26 android changes are not in chromeos 39 chromeos changes are not in aosp. Is it intentional? Trying to merge, few ASOP CLs have conflicts: 3b926ae: Use registered users in libbrillo/OWNERS. (migrate from @chromium to @google) 7a4643f: Convert most libbrillo targets to soong. f85de06: Convert libbrillo-minijail target to Soong also some conflicts in: 826000b libbrillo: Update libchrome APIS to r456626. 0cd09dc libbrillo: policy: Increase resilience for policy read Gwendal.
,
Jun 11 2018
These are some of the things that UE uses in brillo: bind_lambda.h data_encoding.h error_codes.h message_loop.h -> this one might be tricky since it is very basic in UE. key_store_value.h -> Another important one. string_utils.h deamons.h flag_helper.h bind_watcher.h for libchrome we use mainly use dbus. We do syncing between UE in CrOS and AOSP a lot. We try to keep them in sync as much as possible since good features that land in one gets merge into another one and we would like to keep it this way unless there is opposition. If we move back these libs to chromium, does that mean that it will be a permanent fork and we won't uprev to AOSP changes anymore?
,
Jun 11 2018
Adding UE owner in AOSP: senj@
,
Jun 11 2018
we're just bouncing around ideas atm if we move the main libbrillo/libchrome dev back to Chromium, the changes can be merged back to AOSP as needed on the UE side
,
Jun 11 2018
update_engine is not the only one using libbrillo/libchrome http://cs/search/?q=lib(chrome%7Cbrillo)+f:android%5C.(bp%7Cmk)&sq=package:%5Eandroid$&type=cs There's metricsd, peripheralmanager, bluetooth, nfc and some others using them. So I don't think it's feasible to remove libchrome from aosp at this point, but I know that there aren't much development in libchrome/libbrillo going on from android side, so if it make you guys happier I'm fine with treating the repo in chromium as upstream. Adding other android owners.
,
Jun 11 2018
I remember, the decision to separate libchrome in aosp and chromeos has been made a couple years ago. At the time, it was a constant source of problems for anybody doing uprevving (need to make sure both Android and Chrome OS code is compiling correctly, at the same time and with the branch layout [aosp, vs internal googleplex repos] it became even a bigger problem of trying to keep automerger happy and working, since some AOSP changes are tried by TreeHugger on top of googleplex changes). In the end, the Brillo team at the time decided to maintain AOSP copy and Chrome OS team dealt with its own version. AT the same time "shared" engines such as Update Engines have parted their way and were no longer single-sourced. So, I'd say the fork is expected and has happened like two years ago.
,
Jun 12 2018
i'm not talking about removing libs from AOSP. they can stay there. the question is how to handle the projects that are still being actively used in both CrOS & AOSP wrt these libs. clearly there's a lot more development happening on the CrOS side with these projects while the AOSP side tends to be quiet. it's unreasonable to have CrOS devs doing libchrome uprevs on the CrOS side to be responsible for all the AOSP projects. if UE is the only thing we care about keeping in sync, we can probably just do what we had in the past ... when the code diverges, uses #if BASE_VER CPP logic to pick between the two. libchrome today is at 395517 in AOSP and CrOS. so when we move to the next version (e.g. 413419), when do it on the CrOS side w/out checking anything on the AOSP side. but when Amin goes to do an uprev of AOSP UE into CrOS, he'll have to make sure it still works with the newer version of libchrome. if it doesn't, he can make a change on the AOSP side like: #if BASE_CPP <= 395517 // Do older libchrome stuff that AOSP needs #else // Do newer libchrome stuff that CrOS needs #endif then if/when AOSP ever merges libchrome/libbrillo changes back, the code in UE should still work, and get pruned. UE & minijail are still single sourced. everything else has gone back to platform2/ or is in process.
,
Aug 13
so what i'm thinking is: - move libbrillo back into platform2/ - create a gsubtree copy on GoB at chromiumos/platform2/libbrillo - leave the AOSP copy alone (and thus nothing changes on the Android side) - if people want to merge changes, the gsubtree copy can be used - all projects in CrOS will use the new libbrillo that we maintain - any changes in libbrillo will run through the CrOS CQ like normal and makes sure everything continues to build/work - AOSP is a fork that does not get auto-updated - if any changes to libbrillo are made that break UE, the person making that change will have to update UE to include some #if logic so it works with the new code on the CrOS side - this change will go through AOSP/treehugger to make sure Android doesn't break - a manual uprev of UE happens on the CrOS side like it does today - the CrOS libbrillo change lands via the CrOS CQ this basically means libbrillo on the AOSP side won't change until someone on the Android side cares. if the #if logic in UE gets too onerous, then someone on the UE team will have to take care of rolling things forward (as it cascades into all the other packages that Android has that might be using libbrillo still). but this should allow the CrOS side to move forward w/out being beholden to the Android side, and Android doesn't have to worry about changes rolling in automatically that breaks the Android tree. to keep things simple, we can keep the Android.bp and related files in the platform2/libbrillo/ source tree. or if it makes no difference to Android, they can keep those files in the Android libbrillo fork and just do normal git merges. i guess they're going to anyways right?
,
Aug 14
#9: Thanks for writing that up. No objections here.
,
Aug 14
Thanks :) So regarding libbrillo, we are only moving it to platform2 because AOSP and CrOS where technically on different repos. right? That doesn't fundamentally change anything much. Did I get it wrong? LGTM with me although, upreving the AOSP version of libbrillo may not be an easy task as it might need cherry picking hundreds of CLs among platform2 repo (instead of a possible git merge). How about libchrome?
,
Aug 14
#9: sgtm. Should the same logic apply to libchrome as well?
,
Aug 14
we want to move libbrillo back into platform2 because having a single repo for all our platform projects increases visibility and makes it easier to make changes together (especially with breaking changes). the fact that it's hosted in AOSP makes this even more painful to update -- it's a sep CQ and the ebuild is manually uprevved. by setting up a gsubtree of libbrillo, Chrome infra will create a git repo of just libbrillo CLs as if they were rooted at libbrillo/. that should make it easier to cherry pick or even do a `git merge` directly. you can see an example of this here: https://chromium.googlesource.com/chromium/src/base/ those are commits made to src.git but only to files under base/.
,
Aug 14
actually, the AOSP/blacklist note doesn't apply to libbrillo, so nm that. wrt libchrome, i'm not sure we'd ever merge it into platform2 unrelated to AOSP. i'd like to brainstrom that in a sep bug.
,
Aug 14
Sounds good to me, I think I just need to change aosp/upstream-master branch to replicate the new gsubtree repo, and merge should still work.
,
Aug 14
Cool, That explains it better. But just a question, I see that libbrillo has a 9999 ebuild and is not blacklisted, so why does it have a separate CQ? Changes in CrOS do not land in AOSP automatically and neither the otherwise. And AFAIK the changes to libbrillo goes through Chromium gerrit not Android gerrit. I'm just a bit confused. SGTM overall.
,
Aug 14
+1 to #9. One question. What should we do if we/UE team want to change libbrillo in AOSP? Change in CrOS and then uprev in AOSP? Or, just making local change (so that libbrillo is (more) diverged)? As for libchrome, filed a separate bug. crbug.com/873906 tl;dr; I'd propose to move libchrome to CrOS, too, and make AOSP's mirror. Let's discuss it in that bug.
,
Aug 14
ignoring Android-specific files (e.g. Android.bp), ideally there should be no differences between any source files between Android and CrOS. however, i think that'd be up to Android devs to make sure.
,
Aug 21
i've rewritten & merged the history here: https://chromium.googlesource.com/chromiumos/platform2/+log/sandbox/vapier/libbrillo i've opted to drop the Android-specific files on the floor: .clang-format | 1 Android.mk | 389 MODULE_LICENSE_BSD | 0 NOTICE | 27 testrunner_android.cc | 26 astute reviewers might notice that AOSP has backoff_entry files but CrOS does not. this is because we deleted/renamed them in CrOS after AOSP had forked, but those changes weren't merged into AOSP. nothing uses them there though afaik so i've opted to keep that drop in CrOS. i expect AOSP will also drop them when they sync up with CrOS again. https://chromium-review.googlesource.com/293421 since AOSP will want to take a merge style to the repo anyways like it has in the past, it should be easy to add those files on the AOSP side. this also allows AOSP to merge any changes they want independently of platform2 (although conflicts will obviously make it harder to pull things back into sync). i'll add a README.md describing the process for libbrillo contributors and the relationship with AOSP. i'm going to wait for Ben's CL to land, and once that does, i'll pull the trigger on this stuff and nerf all open libbrillo CL's on our side. no other libbrillo CL looks like it'll be landing any time soon. https://chromium-review.googlesource.com/1181529
,
Aug 23
now that Ben's CL has landed, i've pulled that in, rebased onto the latest, and pushed it into platform2. i made sure that the diff between platform2.git:a06bc4708453 and libbrillo.git:84a40fb6c08e was just the Android content i noted in comment #20. ebuild/manifest CLs are in flight now.
,
Aug 23
,
Aug 23
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/e825fcfb0bc08f40941bba1d281d3285c7a002f1 commit e825fcfb0bc08f40941bba1d281d3285c7a002f1 Author: Mike Frysinger <vapier@chromium.org> Date: Thu Aug 23 20:47:38 2018 libbrillo: restore backoff_entry While nothing in CrOS has used this, some Jetstream code has started to. Restore the code until we can update those codebases. BUG= chromium:830940 TEST=build still passes Change-Id: I3488f9293c38640577f75ad1c134dd494aafd0f3 Reviewed-on: https://chromium-review.googlesource.com/1187383 Reviewed-by: Mike Frysinger <vapier@chromium.org> Tested-by: Mike Frysinger <vapier@chromium.org> [add] https://crrev.com/e825fcfb0bc08f40941bba1d281d3285c7a002f1/libbrillo/brillo/backoff_entry_unittest.cc [modify] https://crrev.com/e825fcfb0bc08f40941bba1d281d3285c7a002f1/libbrillo/libbrillo.gypi [add] https://crrev.com/e825fcfb0bc08f40941bba1d281d3285c7a002f1/libbrillo/brillo/backoff_entry.cc [add] https://crrev.com/e825fcfb0bc08f40941bba1d281d3285c7a002f1/libbrillo/brillo/backoff_entry.h
,
Aug 23
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/b3b2cd035802fb3346ce051b09a0810abdcbf048 commit b3b2cd035802fb3346ce051b09a0810abdcbf048 Author: Mike Frysinger <vapier@chromium.org> Date: Thu Aug 23 20:47:58 2018 libbrillo: build out of platform2 BUG= chromium:830940 TEST=precq passes Change-Id: I9af0cc629a6912fd8dd0188137bd8d0b815f7e00 Reviewed-on: https://chromium-review.googlesource.com/1173694 Reviewed-by: Chirantan Ekbote <chirantan@chromium.org> Tested-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/b3b2cd035802fb3346ce051b09a0810abdcbf048/chromeos-base/libbrillo/libbrillo-9999.ebuild
,
Aug 29
The following revision refers to this bug: https://chrome-internal.googlesource.com/chromeos/manifest-internal/+/8a3465b6049b83211712229930892f9b416b12a7 commit 8a3465b6049b83211712229930892f9b416b12a7 Author: Mike Frysinger <vapier@chromium.org> Date: Wed Aug 29 18:15:45 2018
,
Aug 29
i've got one more CL in the CQ (update public manifest), so just going to close this out now
,
Aug 29
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/manifest/+/c61929d8263fa9f07fe2f13272a603e04d5d79f8 commit c61929d8263fa9f07fe2f13272a603e04d5d79f8 Author: Mike Frysinger <vapier@chromium.org> Date: Wed Aug 29 23:09:10 2018 libbrillo: move back to platform2 BUG= chromium:830940 TEST=precq passes Change-Id: I15ef9697c324fa69cf89ca2cfc92ff557cc1fef3 Reviewed-on: https://chromium-review.googlesource.com/1183997 Commit-Ready: Mike Frysinger <vapier@chromium.org> Tested-by: Mike Frysinger <vapier@chromium.org> Reviewed-by: Hidehiko Abe <hidehiko@chromium.org> [modify] https://crrev.com/c61929d8263fa9f07fe2f13272a603e04d5d79f8/full.xml |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by vapier@chromium.org
, Jun 9 2018