Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
144823003:
Automatic toolchain: Use src-internal as signal for Pro (Closed)
DescriptionAutomatic toolchain: Use src-internal as signal for Pro
R=iannucci@chromium.org
BUG=323300
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=248786
Patch Set 1 #Patch Set 2 : . #Patch Set 3 : . #Patch Set 4 : use access instead of .gclient #Patch Set 5 : check svn access too #
Total comments: 5
Patch Set 6 : svn first, and early out on success #
Total comments: 1
Patch Set 7 : . #Messages
Total messages: 20 (0 generated)
Deferring to Robbie since he'll be the one having to maintain this.
huge barf :( Wouldn't a more reliable check just be to `git remote show https://chrome-internal.googlesource.com/chrome/src-internal/` and see if it exits with 0?
On 2014/02/04 02:55:14, iannucci wrote: > huge barf :( > > Wouldn't a more reliable check just be to `git remote show > https://chrome-internal.googlesource.com/chrome/src-internal/%60 and see if it > exits with 0? Sounds good once we switch to git. :)
On 2014/02/04 04:34:04, scottmg wrote: > On 2014/02/04 02:55:14, iannucci wrote: > > huge barf :( > > > > Wouldn't a more reliable check just be to `git remote show > > https://chrome-internal.googlesource.com/chrome/src-internal/%2560 and see if it > > exits with 0? > > Sounds good once we switch to git. :) svn ls svn://svn.chromium.org/chrome-internal/trunk/src-internal I guess? I'll switch to that.
Actually, it's kind of slow to check those :( c:\src\cr\src>tim cmd /c git remote show https://chrome-internal.googlesource.com/chrome/src-internal/ * remote https://chrome-internal.googlesource.com/chrome/src-internal/ Fetch URL: https://chrome-internal.googlesource.com/chrome/src-internal/ Push URL: https://chrome-internal.googlesource.com/chrome/src-internal/ HEAD branch: master Local ref configured for 'git push': master pushes to master (local out of date) real: 0m3.198s c:\src\cr\src>tim cmd /c svn ls svn:// svn.chromium.org/chrome-internal/trunk/src-internal/ .DEPS.git DEPS PRESUBMIT.py build/ codereview.settings update_src_limited.py update_src_limited_test.py real: 0m0.936s This runs on update_depot_tools, so seems kind of bad to add 4+s to that frequently executed path. On Mon, Feb 3, 2014 at 8:35 PM, <scottmg@chromium.org> wrote: > On 2014/02/04 04:34:04, scottmg wrote: > >> On 2014/02/04 02:55:14, iannucci wrote: >> > huge barf :( >> > >> > Wouldn't a more reliable check just be to `git remote show >> > https://chrome-internal.googlesource.com/chrome/src-internal/%2560 and >> see >> > if it > >> > exits with 0? >> > > Sounds good once we switch to git. :) >> > > svn ls svn://svn.chromium.org/chrome-internal/trunk/src-internal I guess? > I'll > switch to that. > > https://codereview.chromium.org/144823003/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2014/02/04 04:45:44, scottmg wrote: > Actually, it's kind of slow to check those :( > > c:\src\cr\src>tim cmd /c git remote show > https://chrome-internal.googlesource.com/chrome/src-internal/ > * remote https://chrome-internal.googlesource.com/chrome/src-internal/ > Fetch URL: https://chrome-internal.googlesource.com/chrome/src-internal/ > Push URL: https://chrome-internal.googlesource.com/chrome/src-internal/ > HEAD branch: master > Local ref configured for 'git push': > master pushes to master (local out of date) > > real: 0m3.198s > > c:\src\cr\src>tim cmd /c svn ls svn:// > svn.chromium.org/chrome-internal/trunk/src-internal/ > .DEPS.git > DEPS > PRESUBMIT.py > build/ > codereview.settings > update_src_limited.py > update_src_limited_test.py > > real: 0m0.936s > > This runs on update_depot_tools, so seems kind of bad to add 4+s to that > frequently executed path. > Can't we do the check iff there's a new version to pull? Then it only happens 1/<long_time>... > > On Mon, Feb 3, 2014 at 8:35 PM, <mailto:scottmg@chromium.org> wrote: > > > On 2014/02/04 04:34:04, scottmg wrote: > > > >> On 2014/02/04 02:55:14, iannucci wrote: > >> > huge barf :( > >> > > >> > Wouldn't a more reliable check just be to `git remote show > >> > https://chrome-internal.googlesource.com/chrome/src-internal/%252560 and > >> see > >> > > if it > > > >> > exits with 0? > >> > > > > Sounds good once we switch to git. :) > >> > > > > svn ls svn://svn.chromium.org/chrome-internal/trunk/src-internal I guess? > > I'll > > switch to that. > > > > https://codereview.chromium.org/144823003/ > > > > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org.
Oh, also, you should use the git version. Otherwise this will just break on flag day, and I'll laugh at you. You've been warned.
On 2014/02/04 05:11:02, iannucci wrote: > On 2014/02/04 04:45:44, scottmg wrote: > > Actually, it's kind of slow to check those :( > > > > c:\src\cr\src>tim cmd /c git remote show > > https://chrome-internal.googlesource.com/chrome/src-internal/ > > * remote https://chrome-internal.googlesource.com/chrome/src-internal/ > > Fetch URL: https://chrome-internal.googlesource.com/chrome/src-internal/ > > Push URL: https://chrome-internal.googlesource.com/chrome/src-internal/ > > HEAD branch: master > > Local ref configured for 'git push': > > master pushes to master (local out of date) > > > > real: 0m3.198s > > > > c:\src\cr\src>tim cmd /c svn ls svn:// > > svn.chromium.org/chrome-internal/trunk/src-internal/ > > .DEPS.git > > DEPS > > PRESUBMIT.py > > build/ > > codereview.settings > > update_src_limited.py > > update_src_limited_test.py > > > > real: 0m0.936s > > > > This runs on update_depot_tools, so seems kind of bad to add 4+s to that > > frequently executed path. > > > > Can't we do the check iff there's a new version to pull? Then it only happens > 1/<long_time>... Thanks, much better. Done. > > > > > On Mon, Feb 3, 2014 at 8:35 PM, <mailto:scottmg@chromium.org> wrote: > > > > > On 2014/02/04 04:34:04, scottmg wrote: > > > > > >> On 2014/02/04 02:55:14, iannucci wrote: > > >> > huge barf :( > > >> > > > >> > Wouldn't a more reliable check just be to `git remote show > > >> > https://chrome-internal.googlesource.com/chrome/src-internal/%25252560 and > > >> see > > >> > > > if it > > > > > >> > exits with 0? > > >> > > > > > > Sounds good once we switch to git. :) > > >> > > > > > > svn ls svn://svn.chromium.org/chrome-internal/trunk/src-internal I guess? > > > I'll > > > switch to that. > > > > > > https://codereview.chromium.org/144823003/ > > > > > > > To unsubscribe from this group and stop receiving emails from it, send an > email > > to mailto:chromium-reviews+unsubscribe@chromium.org.
On 2014/02/04 05:11:48, iannucci wrote: > Oh, also, you should use the git version. Otherwise this will just break on flag > day, and I'll laugh at you. You've been warned. How long will this be? Should I add both or is it not possible to pull any more without git auth?
Per IM, now checking git or svn.
https://codereview.chromium.org/144823003/diff/180001/win_toolchain/get_toolc... File win_toolchain/get_toolchain_if_necessary.py (right): https://codereview.chromium.org/144823003/diff/180001/win_toolchain/get_toolc... win_toolchain/get_toolchain_if_necessary.py:127: shell=True, stdout=nul, stderr=nul) maybe stdin=nul as well? Why shell=True? https://codereview.chromium.org/144823003/diff/180001/win_toolchain/get_toolc... win_toolchain/get_toolchain_if_necessary.py:128: return git_rc == 0 or svn_rc == 0 Probably prefer svn for now, since we know it's faster on windows... only need to do the second check if the first one fails, too.
https://codereview.chromium.org/144823003/diff/180001/win_toolchain/get_toolc... File win_toolchain/get_toolchain_if_necessary.py (right): https://codereview.chromium.org/144823003/diff/180001/win_toolchain/get_toolc... win_toolchain/get_toolchain_if_necessary.py:127: shell=True, stdout=nul, stderr=nul) On 2014/02/04 21:28:29, iannucci wrote: > maybe stdin=nul as well? Done. > Why shell=True? https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/.git... https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/.git... https://codereview.chromium.org/144823003/diff/180001/win_toolchain/get_toolc... win_toolchain/get_toolchain_if_necessary.py:128: return git_rc == 0 or svn_rc == 0 On 2014/02/04 21:28:29, iannucci wrote: > Probably prefer svn for now, since we know it's faster on windows... only need > to do the second check if the first one fails, too. Done.
lgtm https://codereview.chromium.org/144823003/diff/180001/win_toolchain/get_toolc... File win_toolchain/get_toolchain_if_necessary.py (right): https://codereview.chromium.org/144823003/diff/180001/win_toolchain/get_toolc... win_toolchain/get_toolchain_if_necessary.py:127: shell=True, stdout=nul, stderr=nul) On 2014/02/04 21:37:04, scottmg wrote: > On 2014/02/04 21:28:29, iannucci wrote: > > maybe stdin=nul as well? > > Done. > > > Why shell=True? > > https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/.git... > > https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/.git... #!@*#!)$!@( https://codereview.chromium.org/144823003/diff/220001/win_toolchain/get_toolc... File win_toolchain/get_toolchain_if_necessary.py (right): https://codereview.chromium.org/144823003/diff/220001/win_toolchain/get_toolc... win_toolchain/get_toolchain_if_necessary.py:129: return True nit: this should actually return False. Otherwise it'll return None, which while Falseish, is confusing :p
On 2014/02/04 21:43:58, iannucci wrote: > lgtm > > https://codereview.chromium.org/144823003/diff/180001/win_toolchain/get_toolc... > File win_toolchain/get_toolchain_if_necessary.py (right): > > https://codereview.chromium.org/144823003/diff/180001/win_toolchain/get_toolc... > win_toolchain/get_toolchain_if_necessary.py:127: shell=True, stdout=nul, > stderr=nul) > On 2014/02/04 21:37:04, scottmg wrote: > > On 2014/02/04 21:28:29, iannucci wrote: > > > maybe stdin=nul as well? > > > > Done. > > > > > Why shell=True? > > > > > https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/.git... > > > > > https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/.git... > > #!@*#!)$!@( > > https://codereview.chromium.org/144823003/diff/220001/win_toolchain/get_toolc... > File win_toolchain/get_toolchain_if_necessary.py (right): > > https://codereview.chromium.org/144823003/diff/220001/win_toolchain/get_toolc... > win_toolchain/get_toolchain_if_necessary.py:129: return True > nit: this should actually return False. Otherwise it'll return None, which while > Falseish, is confusing :p Doneish.
The CQ bit was checked by scottmg@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/scottmg@chromium.org/144823003/270001
Message was sent while issue was closed.
Change committed as 248786
Message was sent while issue was closed.
svn apparently decides hanging at a password prompt here is the answer, even when stdin in NUL. :P Any idea how to avoid that? I think the git one will fail too if it's not run inside a git repo. The cwd is depot_tools/win_toolchain, so I _think_ that should OK, though a bit weird.
Message was sent while issue was closed.
Ah, --non-interactive apparently. |
