Make clang tot bots not depend on depot_tools's svn.exe |
||||||||
Issue descriptionIt looks like the tip-of-tree clang bots are checking out the llvm repo directly from llvm.org using svn: https://cs.chromium.org/chromium/src/tools/clang/scripts/update.py?rcl=0&l=82 https://cs.chromium.org/chromium/src/tools/clang/scripts/update.py?rcl=0&l=254 https://cs.chromium.org/chromium/src/tools/clang/scripts/update.py?rcl=0&l=168 I think there may be two problems with this. First, we normally don't want our buildbots pulling code from repos we don't control that aren't on our networks; in other words, we normally want to be using mirrors on google-hosted machines. Second, we don't really want to be using SVN at all, and so there's a risk that at some point our machine images won't have SVN installed on them, at which point this update flow would break. Does it make sense to set up a git+svn mirror and switch to pulling the code via git instead? The Clang ToT bots are only FYI, but still this seems less than good.
,
Oct 13 2016
In fact, I don't understand why update.py isn't just "write a DEPS file, call gclient sync, compile some things". Why has it completely reinvented the wheel?
,
Oct 13 2016
There are discussions in llvm land about moving to git. Until then, unless there are concrete problems, I don't want to change anything here since LLVM deals in svn revision numbers and git hashes are much more annoying to deal with. (we used to point to the mirrors for a while, but we accidentally lost that, and since it really hasn't been an issue we never bothered to change that.) So unless there's concrete cause to be alarmed, this is WontFix until the git situation is sorted out upstream (discussions are ongoing)
,
Oct 13 2016
most of update.py is about compiling, not syncing
,
Oct 13 2016
Can't you keep using svn revisions by just use the git-svn-id footers that the mirror contains?
,
Oct 13 2016
If i want the script to say "check out rFoo" then that'd be something like "parse git log, get hash, do thing", which is possible, but annoying to do, and I don't see why it needs doing now given this has been working fine for years and upstream is talking about switching VCSs.
,
Oct 13 2016
The point is less "we shouldn't be directly using SVN" (although we shouldn't -- I almost deleted the SVN executable from depot_tools this week) and more "we shouldn't be downloading code from sources we don't control". We can't make any guarantees about the uptime or service levels of someone else's SVN server. Just like you don't want tests making network requests because they introduce flake, we don't want compile bots downloading from external networks. I can guarantee that llvm.googlesource.com will be around for a long time. If upstream switches to Git, it will still work. Please work on switching update.py to use Git (or even better, gclient, to avoid duplicating work) sooner rather than later.
,
Oct 13 2016
Because we've been working hard to not have any uses of SVN (i.e., we actually thought we had found them all) and because we try not to pull code from non-google-managed machines? I.e., because of the reasons I gave in the first place? Unless there's an actual decision in llvm-land and a date for a transition, "discussions are ongoing" doesn't hold any value with me, at least. We had "ongoing discussions" for what, 5+ years?
,
Oct 13 2016
Yeah, but those are FYI bots and it's ok if the llvm svn server is down every now and then (it rarely is, but it happens). These bots build clang from trunk svn and then chrome with that. When llvm svn is down, there's nothing useful these bots can do anyways. We can put a non-depot_tools svn binary on there if that's the concern.
,
Oct 13 2016
The only way to keep an SVN binary on those machines will be to have update.py download it. Infra team will not support having an SVN binary installed, whether or not it is part of depot_tools by default. I've already identified the repos that should be used. It seems like the meat of the matter here is just a) mapping the currently pinned SVN revision to 5 different git hashes b) changing 'svn checkout --force' to 'if dir: git pull && git checkout; else: git clone' Neither of these seems particularly burdensome to me? Maybe I'm wrong.
,
Oct 13 2016
Yes, update.py would download an svn binary. It downloads a bunch of other binaries too. Is the motivation here the deletion of svn from depot_tools?
,
Oct 13 2016
The deletion of SVN from depot_tools *and* the desire to not download from servers we don't control. If you were checking out from git.llvm.org I'd be asking you to switch to the mirrors too.
,
Oct 13 2016
As mentioned above, this generally makes sense, but these bots can't do anything anyway if the outside servers are down. depot_tools only has an svn binary on windows, yes? i'll change the windows bots to not rely on depot_tools svn.
,
Oct 14 2016
(I'm waiting on someone to confirm that not relying on svn.exe from depot_tools is what this bug is meant to be about.)
,
Oct 14 2016
See comment 0, and comment 7, and comment 12: that is *not* the only thing this bug is meant to be about. We would much prefer that everyone and everything on our bots only use Git to download repos from *.googlesource.com, or wget (essentially) to download binaries from storage.googleapis.com. In particular, when we're auditing logs to ensure we can do things like actually remove the svn executable from depot_tools, these svn invocations will be red herrings. If you switch update.py to download its own SVN executable and get someone to LGTM the change, I can't/won't stop you, but I will be sad. And if the SVN download ever flakily fails, infra team won't be able to provide support.
,
Oct 14 2016
I'm trying to tease appart if anything the bots are doing is an actual problem, and what is your folks's opinion on how the world should be. If these bots get in the way of you doing your job, that's important to me and I want to fix this. So far, I'm hearing "don't depend on svn.exe in depot_tools, and don't mention svn in logs so grepping logs for the absence of svn is easier". This both makes sense to me. I agree with the general statement that we don't want bots to turn red just cause some infrastructure we don't control fails. As mentioned above, that doesn't apply for these bots since they're not doing anything if that particular external infra is broken. I don't want to spend time on this since a) it's not fixing an actual problem as far as I can tell B) whatever I'd do would be undone if upstream switches to git c) it makes my team's daily life harder cause llvm (for now) deals in svn revisions. So once more: it'd be good if someone could say what (if anything) is actually supposed to happen on this bug.
,
Oct 14 2016
It's Chrome's policy to not have builders using SVN or talking to machines that aren't google-maintained, just like it's our policy that builders should be using recipes, GN, Ninja, and all of the other common infrastructure. You might not call that an actual problem, but we do. I believe agable@ and I have been quite clear about what needs to happen: the builders need to be switched to talk to a git repo on a machine hosted in our data center, like every other machine in our fleet.
,
Oct 14 2016
> It's Chrome's policy ... As said above, that does make a lot of sense because we don't want bots to go down when some infra we don't control breaks down. !!!This does not apply here!!! (Similarly, the bot also uses cmake to build llvm, which we also normally don't do.) Please separate the dogmatic parts of this bug from the actual request.
,
Oct 14 2016
The actual request is to comply with the dogma.
,
Oct 14 2016
¯\_(ツ)_/¯ Feel free to escalate. As said above, if anything the tot bots do block any effort or cause anyone any headaches, I'm very interested in fixing them (if something like that is happening here -- I'm still not sure 20 comments in -- please file a new bug for that). I don't want anyone to be slowed down by them. I'm not interested in applying an engineering guideline that generally makes sense but doesn't here (as far as I can tell) dogmatically.
,
Oct 14 2016
I'm not sure why you aren't understanding that having builders be inconsistent, using SVN, and talking to non-google hosts isn't introducing some unnecessary operational complexity and causing headaches for infra folks who now have to support yet another special snowflake (and yes, they have to support it regardless of whether it is an FYI bot or not). It is true that making the changes will add maybe a dozen or two lines of code to your script and make things slightly harder to track, but I don't think that cost is enough of a justification for you to leave things alone. But, I'm going to suggest a compromise and de-assign you from this. Infra can figure out if this is a big enough problem that we should find someone else to change things before LLVM switches to git; maybe you'll get lucky and they're be willing to let it ride. However, if you actually want to stop anyone else from changing this, then we probably need to have a further conversation and/or escalate it.
,
Oct 14 2016
If there are any examples of this causing issues, please point me to them. Infra doesn't do anything operational with these bots as far as I know, and doesn't support the builds on them, so I don't understand (yet) where headaches would come from. (Infra does help with restarting them when they're down, but that part of the bot setup is 100% standard.) I do want to stop people from moving the script from svn revs to git hashes, yes. So far, I've seen no evidence for a > 0 benefit on this bug, and it has > 0 cost. If there's a good reason for changing things, I'm open to that (as I pointed out on every one of my comments). I guessed at a few things that might have > 0 benefit, but my questions if these guesses were right have so far been ignored.
,
Oct 14 2016
Aaron - I remember you looking into this a few months ago. Couldn't we just use: https://chromium.googlesource.com/chromium/src/tools/clang/ ?? Our builders should be pulling only from googlesource.com, so if we need to set up another project in third_party and sync all changes from upstream llvm, we should do that. But I was under the impression that we were doing this already with most (or all) of llvm.org code.
,
Oct 14 2016
benhenry: please see comments above
,
Oct 14 2016
Having all of the builders using git and only talking to machines on our networks has > 0 benefit for infra. It makes security audits easier, it helps w/ network configuration, and it makes our infrastructure more consistent and understandable, at least. It means that we can use existing monitoring infrastructure to detect outages rather than have bots randomly turn red and have people wondering at what broke and why. And, yes, it allows us to do things like delete svn support from depot_tools. I don't know which questions or guesses you've made that you think have been ignored.
,
Oct 14 2016
/me should have read the entire bug. As per Dirk's request, I'm going to leave this up to Aaron. Assuming the cost is minimal, we should do this. The dogma in this case exists for a good reason - which is to limit the amount of knowledge troopers and sheriffs have to retain in order to be effective as those roles. By unifying technologies, we limit the scope of what it is we're asking people to learn/know. Is there an expected timeline for when llvm will switch (or won't) to Git? Is there a chance they won't?
,
Oct 14 2016
Re timeline: http://llvm.org/docs/Proposals/GitHubMove.html has the current proposal, it will be discussed at the upcoming LLVM conf Nov 3/4 2016. Depending on what happens there, it might move quickly, slowly, or it'll stop moving :-)
,
Oct 14 2016
That is a better estimate than CIT provided for a Git migration, so thank you for that. Aaron - wdyt?
,
Oct 14 2016
Based on my reading of the GitHubMove doc, even if the decision on Nov 3/4 is "let's do it and let's do it fast!" it'll take until Q2 2017 before the move is finalized, at least. I hope I'm wrong, for their sake, since I remember exactly how hard our migration was ;) Anyway, I don't believe that the llvm team's decision of whether or not to move to Git is particularly relevant to this discussion. * If LLVM doesn't move to Git, we'll still want people checking those repositories out to get them from Git mirrors anyway * If LLVM does move to Git, then this script will be ahead of the game. Since we want update.py to be using Git no matter what, investing in the effort to have it download its own SVN executable seems like progress in the wrong direction. I'm happy to update the svn-using code in update.py to use git instead. However, I'm afraid that I don't fully understand the use-case. Do devs run it? Only bots? What about package.py? How often is the revision number updated? How important is it that all 5 repos be pinned to the same revision? (I note that in the GitHubMove doc, the llvm team stance on that issue is mostly "you'll deal with it".) Left to my own devices, 5 hashes would be separately defined at the top of the file. I have no clue how much of a hardship that would or wouldn't be to the team.
,
Oct 17 2016
It seems we're talking past each other here, and maybe there is some confusion as to how update.py is used and what these FYI bots are doing. Replying to the original post: > First, we normally don't want our buildbots pulling code from repos we don't control that aren't on our networks; in other words, we normally want to be using mirrors on google-hosted machines. Not having external dependencies makes perfect sense for building Chromium in general. However, the purpose of these bots is to track the status of building tip-of-tree Chromium with tip-of-tree LLVM. That is, the external dependency is the point of the bots. If LLVM's SVN server goes down, I *want* these bots to go red. > Second, we don't really want to be using SVN at all, and so there's a risk that at some point our machine images won't have SVN installed on them, at which point this update flow would break. I can certainly see that infra don't want to be maintaining svn now that we've transitioned to git. That's fine. As Nico pointed out, we can treat svn as we do other dependencies (gcc, cmake) in update.py.
,
Oct 17 2016
Ok, that's a piece of the puzzle that I was definitely missing. I understand wanting the bots to go red when tip-of-tree LLVM is broken; wanting the bots to go red when LLVM servers are down never even crossed my mind. In that case, please go ahead and download an svn executable along with the other binaries you rely on. I'll follow progress on this bug so I know when its safe(er) to remove depot_tools' svn.
,
Oct 17 2016
Thanks for explaining this much better than me, Hans. I'm making a CL for svn (currently uploading).
,
Oct 17 2016
For what it's worth, I understood the original request (that you want the builder to turn red if llvm.org is unavailable), but I still don't think that is something we should necessarily be supporting directly on the builders. I think it's better that we rely on our existing mirroring and monitoring instead. But, it's not my call. If agable@ is okay with this, then I defer to him.
,
Oct 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/78dbc8025d886c77c1d942fcbb31716933f756b3 commit 78dbc8025d886c77c1d942fcbb31716933f756b3 Author: thakis <thakis@chromium.org> Date: Tue Oct 18 01:22:24 2016 clang tot bots: Don't depend on svn.exe from depot_tools. BUG= 655790 NOTRY=true Review-Url: https://codereview.chromium.org/2429613002 Cr-Commit-Position: refs/heads/master@{#425846} [modify] https://crrev.com/78dbc8025d886c77c1d942fcbb31716933f756b3/tools/clang/scripts/update.py
,
Oct 18 2016
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by aga...@chromium.org
, Oct 13 2016