Move toolchain update control into src, but keep download logic in depot_tools

Move toolchain update control into src, but keep download logic in depot_tools

Moves the update functionality out of update_depot_tools into src/ in https://codereview.chromium.org/175573004 .

get_toolchain_if_required.py now expects a list of hashes on the
command line, and makes sure that it gets one of those.

toolchain2013.py saves a .json which contains information relevant to the
interests of the caller, so that it can set up the parent environment. This
is returned via the --output-json command line argument to get_...py

R=iannucci@chromium.org
BUG=323300
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252725

[iannucci, 2014-02-21 23:53:08]

lgtm

https://codereview.chromium.org/168603004/diff/1/win_toolchain/get_toolchain_...
File win_toolchain/get_toolchain_if_necessary.py (right):

https://codereview.chromium.org/168603004/diff/1/win_toolchain/get_toolchain_...
win_toolchain/get_toolchain_if_necessary.py:149: print >> sys.stderr, 'Usage:
get_toolchain_if_necessary.py sha1...'
Probably want to note that we'll get the first sha that we have access to? Or is
it always pro/express?

Either way, the halp message should probably say so :)

Not sure if the module docstring needs to change to reflect this.

[scottmg, 2014-02-21 23:53:10]

and the other side.

[scottmg, 2014-02-22 00:02:51]

On 2014/02/21 23:53:08, iannucci wrote:
> lgtm
> 
>
https://codereview.chromium.org/168603004/diff/1/win_toolchain/get_toolchain_...
> File win_toolchain/get_toolchain_if_necessary.py (right):
> 
>
https://codereview.chromium.org/168603004/diff/1/win_toolchain/get_toolchain_...
> win_toolchain/get_toolchain_if_necessary.py:149: print >> sys.stderr, 'Usage:
> get_toolchain_if_necessary.py sha1...'
> Probably want to note that we'll get the first sha that we have access to? Or
is
> it always pro/express?
> 
> Either way, the halp message should probably say so :)
> 
> Not sure if the module docstring needs to change to reflect this.

It uses those to determine whether to do anything (i.e. do they match what's on
disk), but which one to get is still controlled by the access to src-internal
check in this script.

As discussed, the docstring is right still, unfortunately, until we have a way
to store old versions.

[scottmg, 2014-02-22 01:10:02]

PTAL

[iannucci, 2014-02-22 01:23:06]

lgtm

https://codereview.chromium.org/168603004/diff/110001/win_toolchain/get_toolc...
File win_toolchain/get_toolchain_if_necessary.py (right):

https://codereview.chromium.org/168603004/diff/110001/win_toolchain/get_toolc...
win_toolchain/get_toolchain_if_necessary.py:198:
shutil.copyfile(os.path.join(target_dir, 'data.json'), options.output_json)
ooh, fancy...

We're assuming that the caller will be able to correctly absolute-ify the path
data in data.json?

Or does data contain absolute paths?

Probably will need to change when we're not running toolchain2013 all the time

[scottmg, 2014-02-22 01:39:35]

On 2014/02/22 01:23:06, iannucci wrote:
> lgtm
> 
>
https://codereview.chromium.org/168603004/diff/110001/win_toolchain/get_toolc...
> File win_toolchain/get_toolchain_if_necessary.py (right):
> 
>
https://codereview.chromium.org/168603004/diff/110001/win_toolchain/get_toolc...
> win_toolchain/get_toolchain_if_necessary.py:198:
> shutil.copyfile(os.path.join(target_dir, 'data.json'), options.output_json)
> ooh, fancy...
> 
> We're assuming that the caller will be able to correctly absolute-ify the path
> data in data.json?
> 
> Or does data contain absolute paths?

Yes, all the paths in data.json are absolute.

> 
> Probably will need to change when we're not running toolchain2013 all the time

You mean for gs mirroring instead? Yeah, we'll burn that bridge when we get
there.

[iannucci, 2014-02-22 01:39:48]

On 2014/02/22 01:39:35, scottmg wrote:
> On 2014/02/22 01:23:06, iannucci wrote:
> > lgtm
> > 
> >
>
https://codereview.chromium.org/168603004/diff/110001/win_toolchain/get_toolc...
> > File win_toolchain/get_toolchain_if_necessary.py (right):
> > 
> >
>
https://codereview.chromium.org/168603004/diff/110001/win_toolchain/get_toolc...
> > win_toolchain/get_toolchain_if_necessary.py:198:
> > shutil.copyfile(os.path.join(target_dir, 'data.json'), options.output_json)
> > ooh, fancy...
> > 
> > We're assuming that the caller will be able to correctly absolute-ify the
path
> > data in data.json?
> > 
> > Or does data contain absolute paths?
> 
> Yes, all the paths in data.json are absolute.
> 
> > 
> > Probably will need to change when we're not running toolchain2013 all the
time
> 
> You mean for gs mirroring instead? Yeah, we'll burn that bridge when we get
> there.

sg, ship it

[scottmg, 2014-02-22 01:40:40]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2014-02-22 01:40:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/168603004/110001

[commit-bot: I haz the power, 2014-02-22 01:40:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-22 01:40:45]

Failed to apply patch for depot_tools/update_depot_tools.bat:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file depot_tools/update_depot_tools.bat
  Hunk #1 FAILED at 30.
  1 out of 1 hunk FAILED -- saving rejects to file
depot_tools/update_depot_tools.bat.rej

Patch:       depot_tools/update_depot_tools.bat
Index: update_depot_tools.bat
diff --git depot_tools/update_depot_tools.bat depot_tools/update_depot_tools.bat
index
0297d3d8826ac53f66b96f241ef7ad3c5ba6bb45..58d3d849738edef1d9c4280add83ab79c7a872f0
100644
--- a/depot_tools/update_depot_tools.bat
+++ b/depot_tools/update_depot_tools.bat
@@ -30,12 +30,6 @@ if errorlevel 1 goto :EOF
 :: Now clear errorlevel so it can be set by other programs later.
 set errorlevel=
 
-IF "%DEPOT_TOOLS_WIN_TOOLCHAIN%" == "0" GOTO :NOTOOLCHAIN
-call python %~dp0win_toolchain\get_toolchain_if_necessary.py
-if errorlevel 1 goto :EOF
-set errorlevel=
-:NOTOOLCHAIN
-
 :: Shall skip automatic update?
 IF "%DEPOT_TOOLS_UPDATE%" == "0" GOTO :EOF

[scottmg, 2014-02-22 01:41:12]

On 2014/02/22 01:40:45, I haz the power (commit-bot) wrote:
> Failed to apply patch for depot_tools/update_depot_tools.bat:
> While running patch -p1 --forward --force --no-backup-if-mismatch;
>   patching file depot_tools/update_depot_tools.bat
>   Hunk #1 FAILED at 30.
>   1 out of 1 hunk FAILED -- saving rejects to file
> depot_tools/update_depot_tools.bat.rej
> 
> Patch:       depot_tools/update_depot_tools.bat
> Index: update_depot_tools.bat
> diff --git depot_tools/update_depot_tools.bat
depot_tools/update_depot_tools.bat
> index
>
0297d3d8826ac53f66b96f241ef7ad3c5ba6bb45..58d3d849738edef1d9c4280add83ab79c7a872f0
> 100644
> --- a/depot_tools/update_depot_tools.bat
> +++ b/depot_tools/update_depot_tools.bat
> @@ -30,12 +30,6 @@ if errorlevel 1 goto :EOF
>  :: Now clear errorlevel so it can be set by other programs later.
>  set errorlevel=
>  
> -IF "%DEPOT_TOOLS_WIN_TOOLCHAIN%" == "0" GOTO :NOTOOLCHAIN
> -call python %~dp0win_toolchain\get_toolchain_if_necessary.py
> -if errorlevel 1 goto :EOF
> -set errorlevel=
> -:NOTOOLCHAIN
> -
>  :: Shall skip automatic update?
>  IF "%DEPOT_TOOLS_UPDATE%" == "0" GOTO :EOF

rarrr crlf batch files

[scottmg, 2014-02-22 01:46:30]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2014-02-22 01:46:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/168603004/320001

[commit-bot: I haz the power, 2014-02-22 01:46:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-22 01:46:43]

Failed to apply patch for depot_tools/update_depot_tools.bat:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file depot_tools/update_depot_tools.bat
  Hunk #1 FAILED at 30.
  Hunk #2 FAILED at 74.
  2 out of 2 hunks FAILED -- saving rejects to file
depot_tools/update_depot_tools.bat.rej

Patch:       depot_tools/update_depot_tools.bat
Index: update_depot_tools.bat
diff --git depot_tools/update_depot_tools.bat depot_tools/update_depot_tools.bat
index
e4981f0182c2fab9215ebab2f7c25bf8c069c413..fd3e9a22e8e54130ba0d11b567b7863025bbda46
100644
--- a/depot_tools/update_depot_tools.bat
+++ b/depot_tools/update_depot_tools.bat
@@ -30,13 +30,6 @@ if errorlevel 1 goto :EOF
 :: Now clear errorlevel so it can be set by other programs later.
 set errorlevel=
 
-:: Opt-in for now.
-IF "%DEPOT_TOOLS_WIN_TOOLCHAIN%" == "" GOTO :NOTOOLCHAIN
-call python %~dp0win_toolchain\get_toolchain_if_necessary.py
-if errorlevel 1 goto :EOF
-set errorlevel=
-:NOTOOLCHAIN
-
 :: Shall skip automatic update?
 IF "%DEPOT_TOOLS_UPDATE%" == "0" GOTO :EOF
 
@@ -74,4 +67,4 @@ goto :EOF
 :GIT_SVN_UPDATE
 cd /d "%DEPOT_TOOLS_DIR%."
 call git svn rebase -q -q
-goto :EOF
\ No newline at end of file
+goto :EOF

[scottmg, 2014-02-22 01:47:58]

Committed patchset #6 manually as r252725 (presubmit successful).

[scottmg, 2014-02-22 03:00:48]

On 2014/02/22 01:39:48, iannucci wrote:
> On 2014/02/22 01:39:35, scottmg wrote:
> > On 2014/02/22 01:23:06, iannucci wrote:
> > > lgtm
> > > 
> > >
> >
>
https://codereview.chromium.org/168603004/diff/110001/win_toolchain/get_toolc...
> > > File win_toolchain/get_toolchain_if_necessary.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/168603004/diff/110001/win_toolchain/get_toolc...
> > > win_toolchain/get_toolchain_if_necessary.py:198:
> > > shutil.copyfile(os.path.join(target_dir, 'data.json'),
options.output_json)
> > > ooh, fancy...
> > > 
> > > We're assuming that the caller will be able to correctly absolute-ify the
> path
> > > data in data.json?
> > > 
> > > Or does data contain absolute paths?
> > 
> > Yes, all the paths in data.json are absolute.
> > 

Duh! Obviously that isn't going to work when it's not the same path as my
machine because that part's hashed. Some days, I should just stay in bed. $#@!$@

> > > 
> > > Probably will need to change when we're not running toolchain2013 all the
> time
> > 
> > You mean for gs mirroring instead? Yeah, we'll burn that bridge when we get
> > there.
> 
> sg, ship it

[scottmg, 2014-02-22 03:11:25]

In the interest of expedience and fixing this tonight, I'm going to move
data.json out of the hashed part for now, but otherwise keep things the
same. We'll have to figure out the correct location for these things when
we work out supporting multiple sha1s simultaneously.


On Fri, Feb 21, 2014 at 7:00 PM, <scottmg@chromium.org> wrote:

> On 2014/02/22 01:39:48, iannucci wrote:
>
>> On 2014/02/22 01:39:35, scottmg wrote:
>> > On 2014/02/22 01:23:06, iannucci wrote:
>> > > lgtm
>> > >
>> > >
>> >
>>
>
> https://codereview.chromium.org/168603004/diff/110001/win_
> toolchain/get_toolchain_if_necessary.py
>
>> > > File win_toolchain/get_toolchain_if_necessary.py (right):
>> > >
>> > >
>> >
>>
>
> https://codereview.chromium.org/168603004/diff/110001/win_
> toolchain/get_toolchain_if_necessary.py#newcode198
>
>> > > win_toolchain/get_toolchain_if_necessary.py:198:
>> > > shutil.copyfile(os.path.join(target_dir, 'data.json'),
>>
> options.output_json)
>
>> > > ooh, fancy...
>> > >
>> > > We're assuming that the caller will be able to correctly absolute-ify
>> the
>> path
>> > > data in data.json?
>> > >
>> > > Or does data contain absolute paths?
>> >
>> > Yes, all the paths in data.json are absolute.
>> >
>>
>
> Duh! Obviously that isn't going to work when it's not the same path as my
> machine because that part's hashed. Some days, I should just stay in bed.
> $#@!$@
>
>
>  > >
>> > > Probably will need to change when we're not running toolchain2013 all
>> the
>> time
>> >
>> > You mean for gs mirroring instead? Yeah, we'll burn that bridge when we
>> get
>> > there.
>>
>
>  sg, ship it
>>
>
>
>
> https://codereview.chromium.org/168603004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[iannucci, 2014-02-22 08:35:29]

On 2014/02/22 03:11:25, scottmg wrote:
> In the interest of expedience and fixing this tonight, I'm going to move
> data.json out of the hashed part for now, but otherwise keep things the
> same. We'll have to figure out the correct location for these things when
> we work out supporting multiple sha1s simultaneously.
> 
> 
> On Fri, Feb 21, 2014 at 7:00 PM, <mailto:scottmg@chromium.org> wrote:
> 
> > On 2014/02/22 01:39:48, iannucci wrote:
> >
> >> On 2014/02/22 01:39:35, scottmg wrote:
> >> > On 2014/02/22 01:23:06, iannucci wrote:
> >> > > lgtm
> >> > >
> >> > >
> >> >
> >>
> >
> > https://codereview.chromium.org/168603004/diff/110001/win_
> > toolchain/get_toolchain_if_necessary.py
> >
> >> > > File win_toolchain/get_toolchain_if_necessary.py (right):
> >> > >
> >> > >
> >> >
> >>
> >
> > https://codereview.chromium.org/168603004/diff/110001/win_
> > toolchain/get_toolchain_if_necessary.py#newcode198
> >
> >> > > win_toolchain/get_toolchain_if_necessary.py:198:
> >> > > shutil.copyfile(os.path.join(target_dir, 'data.json'),
> >>
> > options.output_json)
> >
> >> > > ooh, fancy...
> >> > >
> >> > > We're assuming that the caller will be able to correctly absolute-ify
> >> the
> >> path
> >> > > data in data.json?
> >> > >
> >> > > Or does data contain absolute paths?
> >> >
> >> > Yes, all the paths in data.json are absolute.
> >> >
> >>
> >
> > Duh! Obviously that isn't going to work when it's not the same path as my
> > machine because that part's hashed. Some days, I should just stay in bed.
> > $#@!$@
> >

Hehe, yeah that was kinda what I was getting at :P.

> >
> >  > >
> >> > > Probably will need to change when we're not running toolchain2013 all
> >> the
> >> time
> >> >
> >> > You mean for gs mirroring instead? Yeah, we'll burn that bridge when we
> >> get
> >> > there.
> >>
> >
> >  sg, ship it
> >>
> >
> >
> >
> > https://codereview.chromium.org/168603004/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.