Issue metadata
Sign in to add a comment
|
Move shill from aosp back to chromiumos platform2 |
||||||||||||||||||||||
Issue descriptionshill is no longer used in any aosp or brillo context, we should consider moving it back to platform2
,
Nov 15 2017
,
May 30 2018
FYI: It no longer exists in aosp. https://android-review.googlesource.com/c/platform/manifest/+/272476
,
Jul 24
i've rewritten & posted the branch for review here: https://chromium.googlesource.com/chromiumos/platform2/+/sandbox/vapier/shill i didn't do as much editing on this series as others because of the sheer volume ... over 850 CLs. what i did edit out of the history though was: - drop Android specific files (e.g. NOTICE and shill.rc) - drop gyp changes that changed to ../../platform2/common-mk/ paths from ../common-mk/ - add "shill:" prefix to all commit messages that didn't already have it that means the code base is still licensed as AOSP. editing that out was more effort than worth i think.
,
Aug 1
Brian said he thought it looked sane, so once the current shill fire settles down, i'll push through the migration
,
Aug 29
Ben asked i wait until after M70 branches, so i'm going to aim for the afternoon of Sep 7th (Fri). that'll give me the weekend to chump fixes in case things burn ;).
,
Aug 29
I'm curious: is it better to do this just before or after a branch? Just before: means that if you have to pick fixes (e.g., for other features) back to M70, you don't have to pick them to different repositories. Just after: means you have the highest chance of having to cherry-pick between repositories. Or maybe there's other factors I'm not considering. e.g., maybe there's a higher chance of accidentally missing something in the transition, and we'd rather not have to fix that up on 2 branches?
,
Aug 29
IMHO, what's more important is that we need some bake time before beta promotion, so it seems less ideal to do the transition right before the branch point, which will give us only 2 weeks to sort things out. Between fighting for beta blockers + reverting CLs (worst case) in a branch and cherry-picking/backporting CLs to a branch, the latter seems more tolerable.
,
Aug 29
Another factor is that we want the sandboxing by itself in R70, then do the path move. If stuff breaks with sandboxing, at least we are fixing on familiar paths.
,
Aug 29
M70 branch is supposed to be tomorrow (Aug 30) which is why i was picking a week later (Sep 7). that'll get the "quick" cherry picks out of the way, but you're right that after we move you'll have to do a bit more effort to backport any changes. that said, i don't think it's too big a deal. after we move the code, backporting looks like: - land CL in platform2 repo - use `git format-patch -1` on the commit to create a patch - apply the patch by hand to the standalone shill repo using `git am -p1` - upload the CL to shill repo for that branch if that's the only downside, imo having longer bake time in M71 is more important than being able to smoothly backport things to M70/M69. i strongly don't think we should fork just before branching because of possible unforeseen problems/breakage. some of the platform2 migrations have gone smoothly (no one noticed and CLs went through the CQ like normal), but others (like libbrillo) caused immediate CQ breakage at build time and required some chumping :/. the TPMs already have enough to deal with to trying to get a new branch stable to not also have to deal with this.
,
Aug 29
SGTM, thanks
,
Sep 5
i'm still planning on doing this migration this coming weekend
,
Sep 5
thanks for the heads up. ill need to cherry pick these https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/1176166 https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1176106 by hand back into M70 but that shouldn't be too bad per c#10
,
Sep 5
^that is if I can't get the CLs into ToT and cherry picked before then
,
Sep 7
The NextAction date has arrived: 2018-09-07
,
Sep 7
+Harpreet FYI/for testing. *In theory*, this shouldn't cause breakage. In practice, I don't know what is an appropriate test other than "all builders should be green". Adding some folks as a heads-up to sister teams: adityakali@ for lakitu, caiz@ for Jetstream.
,
Sep 7
Thanks for looping us in. I don't think we use shill anymore though.
,
Sep 7
Yeah, we've moved away from shill long ago, and `equery-lakitu d chromeos-base/shill` doesn't return anything.
,
Sep 8
i've pushed this out now. over 800 CLs! i'll watch the tree, and if it doesn't burn, run the CLs to update the ebuild/manifests through the CQ.
,
Sep 8
For the naive and uninitiated among us, could you document here the steps you've taken for the migration? 1. https://chromium.googlesource.com/chromiumos/platform2/+/sandbox/vapier/shill Did you do a `git push cros ${VAPIER_BRANCH}:refs/for/master` on this branch? FWIW, this branch is no longer accessible for me (404). 2. https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1147713 https://chromium-review.googlesource.com/c/chromiumos/manifest/+/1149101 I only see these 2 CLs with the search "owner:vapier@chromium.org status:open" that look relevant. Are those the only CLs? Are there any steps between 1 and 2?
,
Sep 8
> For the naive and uninitiated among us, could you document here the steps you've taken for the migration?
it was pretty all ad-hoc
(1) rewrite aosp git history
git filter-branch -f --prune-empty --tree-filter 'mkdir -p shill; git ls-tree --name-only ${GIT_COMMIT} | xargs -I %files% mv %files% shill/'
(2) flatten aosp git history with git-rebase until all the merge commits are gone
(3) find the commit that deleted shill in platform2 and revert it (see db66d0fab31649e3d24797648dbc4c0a99494726)
(4) manually stitch/reorder aosp shill history with git-rebase until we can find a commit that matches the tree state in platform2's (now restored but still ancient) shill/
(5) run `git filter-branch` to delete Android-specific files (MODULE_LICENSE_APACHE2/NOTICE/etc...) in the aosp tree
(6) run git rebase and manually rewrite all new commits to have a "shill:" prefix on it
(7) run `git format-patch` in aosp to get all the new CLs
(8) run `git am` in platform2 to apply all those new CLs
(9) use `git log` on the original aosp history to get original commit email/dates into a flat file
(10) write a script (like www/~vapier/rewrite-commit-date.sh) to use with `git rebase --exec` to read that log and reset the commit author/date details
(11) write a CL to update the platform2/README.md and check-readme.py files to make sure presubmit doesn't break
(12) `git push` the new history directly to master
> 1. https://chromium.googlesource.com/chromiumos/platform2/+/sandbox/vapier/shill
no, that's the sandbox namespace. anyone can run `git push cros HEAD:refs/sandbox/$USER/foo` to create things like this.
since i've merged shill into master, i've cleaned up my sandbox branch as i no longer need it.
> I only see these 2 CLs with the search "owner:vapier@chromium.org status:open"
yes, there are only 3 CLs needed: update the ebuild, update the public manifest, update the internal manifest
,
Sep 9
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/abf6a4071fae84aa2fd5b9f5c76999f2f7193f66 commit abf6a4071fae84aa2fd5b9f5c76999f2f7193f66 Author: Mike Frysinger <vapier@chromium.org> Date: Sun Sep 09 17:33:47 2018 shill: build out of platform2 BUG= chromium:785137 TEST=precq passes Change-Id: I2f68d0f59f5159e629281c19c23286da1f1a75ca Reviewed-on: https://chromium-review.googlesource.com/1147713 Commit-Ready: Mike Frysinger <vapier@chromium.org> Tested-by: Mike Frysinger <vapier@chromium.org> Reviewed-by: Ben Chan <benchan@chromium.org> [modify] https://crrev.com/abf6a4071fae84aa2fd5b9f5c76999f2f7193f66/chromeos-base/shill-client/shill-client-9999.ebuild [modify] https://crrev.com/abf6a4071fae84aa2fd5b9f5c76999f2f7193f66/chromeos-base/shill-test-scripts/shill-test-scripts-9999.ebuild [modify] https://crrev.com/abf6a4071fae84aa2fd5b9f5c76999f2f7193f66/chromeos-base/shill/shill-9999.ebuild
,
Sep 10
The following revision refers to this bug: https://chrome-internal.googlesource.com/chromeos/manifest-internal/+/9bb00e88895d92c9b02367f7e3b55192d11cb870 commit 9bb00e88895d92c9b02367f7e3b55192d11cb870 Author: Mike Frysinger <vapier@chromium.org> Date: Mon Sep 10 05:09:07 2018
,
Sep 10
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/manifest/+/c744e153151beab3abcc45af7977e0d70a29ad5e commit c744e153151beab3abcc45af7977e0d70a29ad5e Author: Mike Frysinger <vapier@chromium.org> Date: Mon Sep 10 05:09:08 2018 shill: move back to platform2 BUG= chromium:785137 TEST=precq passes Change-Id: Ife0a6dffc1675fda27e86a7d6dc12d0ae1d125aa Reviewed-on: https://chromium-review.googlesource.com/1149101 Commit-Ready: Mike Frysinger <vapier@chromium.org> Tested-by: Mike Frysinger <vapier@chromium.org> Reviewed-by: Ben Chan <benchan@chromium.org> Reviewed-by: Brian Norris <briannorris@chromium.org> [modify] https://crrev.com/c744e153151beab3abcc45af7977e0d70a29ad5e/full.xml
,
Sep 10
,
Sep 10
Seems that a repo sync after this change will blow away local shill branches for people, without a warning. This just happened to me, although I don't care since my branches were uploaded. Regardless, do you think this is the behavior CrOS developers expect? Just dont want somebody to lose local changes. That being said, I imagine there's probably some way of undoing a repo sync command.
,
Sep 10
usually repo is supposed to not prune checkouts that have branches/work in them you should be able to find your old git repo under: .repo/project-objects/aosp/platform/system/connectivity/shill.git/ .repo/projects/src/aosp/system/connectivity/shill.git/ if you `cd` into the latter, you can run `git branch -a` and such to extract your work.
,
Sep 10
Good to know, thanks!
,
Sep 13
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/aff7547c93d70a4f0085b26a16e6ba934df9bd39 commit aff7547c93d70a4f0085b26a16e6ba934df9bd39 Author: Mike Frysinger <vapier@chromium.org> Date: Thu Sep 13 06:46:00 2018 shill: add vpn-manager to the subree list This avoids sandbox errors related to vpn-manager/service_error.h use. BUG= chromium:785137 TEST=precq passes Change-Id: Iee333ded1d0f49fc9b4e61928a0072a9cd967540 Reviewed-on: https://chromium-review.googlesource.com/1223570 Reviewed-by: Mike Frysinger <vapier@chromium.org> Tested-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/aff7547c93d70a4f0085b26a16e6ba934df9bd39/chromeos-base/shill/shill-9999.ebuild
,
Sep 18
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ejcaruso@chromium.org
, Nov 15 2017