Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2256183003:
Ensure gclient solutions are always managed. (Closed)
DescriptionEnsure gclient solutions are always managed.
Otherwise slave.DEPS repos are never updated.
BUG=638980
Committed: https://chromium.googlesource.com/chromium/tools/build/+/d608a826f6c99e8548774252a7653b50d14fd7bb
Patch Set 1 #
Messages
Total messages: 13 (3 generated)
dsansome@chromium.org changed reviewers: + agable@chromium.org, phajdan.jr@chromium.org, sergeyberezin@chromium.org
Whoa, we were trying to get *rid* of the managed mode. Is it possible there may be better solutions to this? My point is managed mode implies much more than just "update all the repos". It guesses a lot of things, and introduces complexity. Maybe there's something simpler we could do, i.e. tell gclient to "update all the repos". That update would _fail_ if they're in unexpected state (non fast forwardable), as opposed to doing managed mode's guessing. Consider starting a discussion about this on one of infra's MLs.
On 2016/08/19 at 09:34:13, phajdan.jr wrote: > Whoa, we were trying to get *rid* of the managed mode. > > Is it possible there may be better solutions to this? > > My point is managed mode implies much more than just "update all the repos". It guesses a lot of things, and introduces complexity. Maybe there's something simpler we could do, i.e. tell gclient to "update all the repos". That update would _fail_ if they're in unexpected state (non fast forwardable), as opposed to doing managed mode's guessing. > > Consider starting a discussion about this on one of infra's MLs. Yeah +1 to what Pawel said; managed mode has been on the way out (much too slowly) for a long time. It had all sorts of really dumb consequences, and introduces tons of complexity to gclient itself. I'd rather not depend on it in any new places.
On 2016/08/19 17:16:46, agable wrote: > On 2016/08/19 at 09:34:13, phajdan.jr wrote: > > Whoa, we were trying to get *rid* of the managed mode. > > > > Is it possible there may be better solutions to this? > > > > My point is managed mode implies much more than just "update all the repos". > It guesses a lot of things, and introduces complexity. Maybe there's something > simpler we could do, i.e. tell gclient to "update all the repos". That update > would _fail_ if they're in unexpected state (non fast forwardable), as opposed > to doing managed mode's guessing. > > > > Consider starting a discussion about this on one of infra's MLs. > > Yeah +1 to what Pawel said; managed mode has been on the way out (much too > slowly) for a long time. It had all sorts of really dumb consequences, and > introduces tons of complexity to gclient itself. I'd rather not depend on it in > any new places. Ok, let's move that discussion to crbug/339055. This CL is about fixing an inconsistency in /b checkouts in production. Some bots have unmanaged /b checkouts and therefore changes to slave.DEPS or internal.DEPS will *never* make it to their checkouts. See crbug/638980. I don't know how they got unmanaged /b checkouts in the first place though, but there have been a number of CLs switching the defaults back and forward again, and switching the --managed flag on and off in the linux golo setup script. Maybe there was a period of few days when the stars aligned and the default was unmanaged and --managed wasn't set in services/slave/linux/setup.sh? I want to submit this CL now to fix the immediate problem in production (unless you have a better idea that will allow slave.DEPS and internal.DEPS to be updated so I can roll forward my changes that depend on infra_libs ... and don't say "ssh"). You can revert this update_scripts.py change later when things are consistent.
Since we're already landing changes such as https://codereview.chromium.org/2099333002 , and this change is in build repo and not depot_tools (it won't affect developer chromium/src checkouts), I could be OK with this. I also like the long term plan in https://bugs.chromium.org/p/chromium/issues/detail?id=339055#c27 .
On 2016/08/22 07:09:54, Paweł Hajdan Jr. wrote: > I could be OK with this. Is that an LGTM? :p
I guess the thing that I would prefer most is that update_scripts do a "git pull" in the checkout root, rather than forcing managed mode, so that we actually move forward instead of backwards. But whatever, this can LGTM if necessary.
LGTM to move forward - I'm sad to see the same change being reverted 4 times for silly reasons like inconsistent checkouts. At this point I prefer ugly but consistent. We can fix it later to less ugly while it stays consistent. And +1 to plain "git pull" or "gclient --managed" if it's easy to do. But really it can be done later, together with "ensure_unmanaged" :-)
The CQ bit was checked by dsansome@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Ensure gclient solutions are always managed. Otherwise slave.DEPS repos are never updated. BUG=638980 ========== to ========== Ensure gclient solutions are always managed. Otherwise slave.DEPS repos are never updated. BUG=638980 Committed: https://chromium.googlesource.com/chromium/tools/build/+/d608a826f6c99e854877... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/tools/build/+/d608a826f6c99e854877... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
