Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
238063010:
Bot Update support on official builders (Closed)
|
Created:
6 years, 8 months ago by Ryan Tseng Modified:
6 years, 8 months ago Reviewers:
agable CC:
chromium-reviews, kjellander-cc_chromium.org, cmp-cc_chromium.org, ilevy-cc_chromium.org, stip+watch_chromium.org, mmoss Base URL:
https://chromium.googlesource.com/chromium/tools/build.git@master Visibility:
Public. |
DescriptionBot Update support on official builders
BUG=
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=265371
Patch Set 1 #Patch Set 2 : Rebase #
Total comments: 2
Patch Set 3 : Added other dirs #
Total comments: 21
Patch Set 4 : Review Fixes #
Total comments: 2
Messages
Total messages: 12 (0 generated)
agable review, mmoss fyi. This is probably one of the more ugly hacks in bot_update, but it works on my local machine running an official master. It checks to see if a url matches a buildspec url, turns on buildspec mode, which extracts the release number out of the commit messages for the DEPS file, fetches the DEPS file in the git_buildspecs repo, and tells gclient to use that DEPS file instead.
https://codereview.chromium.org/238063010/diff/20001/scripts/slave/bot_update.py File scripts/slave/bot_update.py (right): https://codereview.chromium.org/238063010/diff/20001/scripts/slave/bot_update... scripts/slave/bot_update.py:48: 'chrome-official', I think this needs to be more than chrome-official: there's also chrome-official-win, chrome-official-win-asan, etc. Every subdirectory of chrome/tools/buildspec/build is a possible value.
https://codereview.chromium.org/238063010/diff/20001/scripts/slave/bot_update.py File scripts/slave/bot_update.py (right): https://codereview.chromium.org/238063010/diff/20001/scripts/slave/bot_update... scripts/slave/bot_update.py:48: 'chrome-official', On 2014/04/18 03:45:07, agable wrote: > I think this needs to be more than chrome-official: there's also > chrome-official-win, chrome-official-win-asan, etc. Every subdirectory of > chrome/tools/buildspec/build is a possible value. Done.
https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update.py File scripts/slave/bot_update.py (right): https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:33: 'chrome-official', Is there any way these could be regex'd? 'chrome-official*'? https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:74: # buildspec -> git cron job to produce the git version of the buildspec. buildspec2git https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:148: ENABLED_MASTERS = ['chromium.git', 'chrome_git'] Have we made sure that the chrome_git waterfall is now running as close to a clone of the chrome.official as we can? It needs to be not doing all its other crazy git stuff. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:367: solution['deps_file'].rsplit('DEPS', 1)) wat. '.DEPS.git'.join()!? https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:556: repository into a temp directory and confine the cleanup logic here.""" nit: close docstring quotes on next line https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:561: git('clone', mirror_dir, tempdir) Shouldn't need the tempdir or the clone. Once you have the cache, you can just run 'git show' against the right path to get the file contents without ever having to check them out. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:569: time.sleep(5) Would be wise to have an upper limit on this, so it doesn't sleep forever (and buildbot will never kill it because it prints output every 5 seconds). https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:586: committed into the git_buildspecs repository.""" nit: close docstring quotes on next line. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:587: deps_log = git('log', '-1', deps_file, cwd=repo_base) you could pass a format argument to git-log so that it only prints the commit message, and not any of the other metadata. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:615: if sln_dir in ['src-internal', 'chrome-official']: hard-coding chrome-official again
comments addressed, ptal https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update.py File scripts/slave/bot_update.py (right): https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:33: 'chrome-official', On 2014/04/18 19:09:33, agable wrote: > Is there any way these could be regex'd? 'chrome-official*'? Hm, maybe we can regex "/chrome-internal/trunk/tools/buildspec/build/(.*)" and turn on buildspec mode. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:33: 'chrome-official', On 2014/04/18 19:09:33, agable wrote: > Is there any way these could be regex'd? 'chrome-official*'? Done. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:74: # buildspec -> git cron job to produce the git version of the buildspec. On 2014/04/18 19:09:33, agable wrote: > buildspec2git Done. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:148: ENABLED_MASTERS = ['chromium.git', 'chrome_git'] On 2014/04/18 19:09:33, agable wrote: > Have we made sure that the chrome_git waterfall is now running as close to a > clone of the chrome.official as we can? It needs to be not doing all its other > crazy git stuff. Yep https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:367: solution['deps_file'].rsplit('DEPS', 1)) On 2014/04/18 19:09:33, agable wrote: > wat. '.DEPS.git'.join()!? rsplit('DEPS') will remove the rightmost 'DEPS', and turn it into a ['asdf/', ''], and running '.DEPS.git'.join() on that list will produce 'asdf/.DEPS.git'. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:556: repository into a temp directory and confine the cleanup logic here.""" On 2014/04/18 19:09:33, agable wrote: > nit: close docstring quotes on next line Done. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:561: git('clone', mirror_dir, tempdir) On 2014/04/18 19:09:33, agable wrote: > Shouldn't need the tempdir or the clone. Once you have the cache, you can just > run 'git show' against the right path to get the file contents without ever > having to check them out. Ohh interesting, done. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:569: time.sleep(5) On 2014/04/18 19:09:33, agable wrote: > Would be wise to have an upper limit on this, so it doesn't sleep forever (and > buildbot will never kill it because it prints output every 5 seconds). Done. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:586: committed into the git_buildspecs repository.""" On 2014/04/18 19:09:33, agable wrote: > nit: close docstring quotes on next line. Done. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:587: deps_log = git('log', '-1', deps_file, cwd=repo_base) On 2014/04/18 19:09:33, agable wrote: > you could pass a format argument to git-log so that it only prints the commit > message, and not any of the other metadata. Done. https://codereview.chromium.org/238063010/diff/40001/scripts/slave/bot_update... scripts/slave/bot_update.py:615: if sln_dir in ['src-internal', 'chrome-official']: On 2014/04/18 19:09:33, agable wrote: > hard-coding chrome-official again Removed, this actually isn't needed
ping
lgtm https://codereview.chromium.org/238063010/diff/50001/scripts/slave/bot_update.py File scripts/slave/bot_update.py (right): https://codereview.chromium.org/238063010/diff/50001/scripts/slave/bot_update... scripts/slave/bot_update.py:528: TOTAL_TRIES = 30 How'd you arrive at this number? 2.5 minutes?
https://codereview.chromium.org/238063010/diff/50001/scripts/slave/bot_update.py File scripts/slave/bot_update.py (right): https://codereview.chromium.org/238063010/diff/50001/scripts/slave/bot_update... scripts/slave/bot_update.py:528: TOTAL_TRIES = 30 On 2014/04/22 21:38:03, agable wrote: > How'd you arrive at this number? 2.5 minutes? Yep, its pretty arbitrary. afaik the cron job is pretty fast, and if its not there by 2 minutes somethings probably wrong.
The CQ bit was checked by hinoka@google.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hinoka@google.com/238063010/50001
Message was sent while issue was closed.
Change committed as 265371
Message was sent while issue was closed.
A revert of this CL has been created in https://codereview.chromium.org/248303002/ by hinoka@google.com. The reason for reverting is: This might be causing a failure here: http://build.chromium.org/p/chromium.linux/builders/Android%20Webview%20AOSP%... Doing a speculative revert.. |
