Move //tools/gs.py into //mojo/public/tools

Move //tools/gs.py into //mojo/public/tools

This move enables download_network_service.py to cleanly use this function.
Since gs.py is now used from the SDK, it takes in the path to find_depot_tools
in order to import it. download_network_service.py correspondingly takes in
that path, similarly to download_shell_binary.py.

R=viettrungluu@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/25540ebe3271805cd3697d5fb7e4d38b721fd49c

[blundell, 2015-02-11 21:44:10]

blundell@chromium.org changed reviewers:
+ jamesr@chromium.org

[vtl, 2015-02-11 21:52:08]

vtl@google.com changed reviewers:
+ vtl@google.com

[vtl, 2015-02-11 21:52:09]

Drive-by comments....

* Also remove the x bit from gs.py.
* It's probably suboptimal to dump non-executables into the root of the tools
directory. But <shrug>.

https://codereview.chromium.org/870143008/diff/20001/mojo/public/tools/gs.py
File mojo/public/tools/gs.py (right):

https://codereview.chromium.org/870143008/diff/20001/mojo/public/tools/gs.py#...
mojo/public/tools/gs.py:1: #!/usr/bin/env python
Delete this.

https://codereview.chromium.org/870143008/diff/20001/mojo/public/tools/gs.py#...
mojo/public/tools/gs.py:11: sys.path.insert(0, find_depot_tools_path)
This function having a permanent side effect on sys.path is suboptimal.

[blundell, 2015-02-11 22:22:01]

I'm happy to move the file under a pylib/ or utils/ subdir if you like that
better.

https://codereview.chromium.org/870143008/diff/20001/mojo/public/tools/gs.py
File mojo/public/tools/gs.py (right):

https://codereview.chromium.org/870143008/diff/20001/mojo/public/tools/gs.py#...
mojo/public/tools/gs.py:1: #!/usr/bin/env python
On 2015/02/11 21:52:09, vtl wrote:
> Delete this.

Done.

https://codereview.chromium.org/870143008/diff/20001/mojo/public/tools/gs.py#...
mojo/public/tools/gs.py:11: sys.path.insert(0, find_depot_tools_path)
On 2015/02/11 21:52:09, vtl wrote:
> This function having a permanent side effect on sys.path is suboptimal.

Sneakily, find_depot_tools.add_depot_tools_to_path() also adds something to
sys.path. :\.

[jamesr, 2015-02-11 23:47:47]

I'm happy with whatever Trung's happy with.

[viettrungluu, 2015-02-11 23:53:57]

On 2015/02/11 22:22:01, blundell wrote:
> I'm happy to move the file under a pylib/ or utils/ subdir if you like that
> better.

Let's put it under a pylib/ subdirectory for foolish consistency.

> 
> https://codereview.chromium.org/870143008/diff/20001/mojo/public/tools/gs.py
> File mojo/public/tools/gs.py (right):
> 
>
https://codereview.chromium.org/870143008/diff/20001/mojo/public/tools/gs.py#...
> mojo/public/tools/gs.py:1: #!/usr/bin/env python
> On 2015/02/11 21:52:09, vtl wrote:
> > Delete this.
> 
> Done.
> 
>
https://codereview.chromium.org/870143008/diff/20001/mojo/public/tools/gs.py#...
> mojo/public/tools/gs.py:11: sys.path.insert(0, find_depot_tools_path)
> On 2015/02/11 21:52:09, vtl wrote:
> > This function having a permanent side effect on sys.path is suboptimal.
> 
> Sneakily, find_depot_tools.add_depot_tools_to_path() also adds something to
> sys.path. :\.

[viettrungluu, 2015-02-11 23:54:49]

viettrungluu@chromium.org changed reviewers:
+ viettrungluu@chromium.org

[viettrungluu, 2015-02-11 23:54:50]

Also, you didn't remove the executable bit from gs.py....

https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
File mojo/public/tools/download_network_service.py (right):

https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
mojo/public/tools/download_network_service.py:67: " find_depot_tools.py,
specified as a relative path"
<bikeshed>On a multiple-line concatenation of strings, should the spaces go at
the end of the  line or at the beginning? Discuss.</bikeshed>

https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
mojo/public/tools/download_network_service.py:71: print "Must specify
--tools-directory; please see help message."
I believe you can just add required=True to add_argument.

https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
File mojo/public/tools/download_shell_binary.py (right):

https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
mojo/public/tools/download_shell_binary.py:79: print "Must specify
--tools-directory; please see help message."
...

[blundell, 2015-02-12 09:06:33]

Thanks!

PS3 set the mode of of gs.py to 644. Had I misunderstood your comment about the
executable bit?

https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
File mojo/public/tools/download_network_service.py (right):

https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
mojo/public/tools/download_network_service.py:67: " find_depot_tools.py,
specified as a relative path"
On 2015/02/11 23:54:49, viettrungluu wrote:
> <bikeshed>On a multiple-line concatenation of strings, should the spaces go at
> the end of the  line or at the beginning? Discuss.</bikeshed>

A quick-and-dirty non-scientific search indicates that we have more of the
former. Changed.

https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
mojo/public/tools/download_network_service.py:71: print "Must specify
--tools-directory; please see help message."
On 2015/02/11 23:54:49, viettrungluu wrote:
> I believe you can just add required=True to add_argument.

Nice! Done.

https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
File mojo/public/tools/download_shell_binary.py (right):

https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
mojo/public/tools/download_shell_binary.py:79: print "Must specify
--tools-directory; please see help message."
On 2015/02/11 23:54:49, viettrungluu wrote:
> ...

Done.

[viettrungluu, 2015-02-12 17:20:25]

https://codereview.chromium.org/870143008/diff/60001/mojo/public/tools/pylib/...
File mojo/public/tools/pylib/gs.py (right):

https://codereview.chromium.org/870143008/diff/60001/mojo/public/tools/pylib/...
mojo/public/tools/pylib/gs.py:9: def download_from_public_bucket(gs_path,
output_path, find_depot_tools_path):
It occurs to me that this is a very strange API. Probably you should just take
depot_tools_path as an argument, and leave it to the caller to figure that out.

[viettrungluu, 2015-02-12 17:23:27]

On 2015/02/12 09:06:33, blundell wrote:
> Thanks!
> 
> PS3 set the mode of of gs.py to 644. Had I misunderstood your comment about
the
> executable bit?

Nope, I'm just bad at reading patches.

> 
>
https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
> File mojo/public/tools/download_network_service.py (right):
> 
>
https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
> mojo/public/tools/download_network_service.py:67: " find_depot_tools.py,
> specified as a relative path"
> On 2015/02/11 23:54:49, viettrungluu wrote:
> > <bikeshed>On a multiple-line concatenation of strings, should the spaces go
at
> > the end of the  line or at the beginning? Discuss.</bikeshed>
> 
> A quick-and-dirty non-scientific search indicates that we have more of the
> former. Changed.
> 
>
https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
> mojo/public/tools/download_network_service.py:71: print "Must specify
> --tools-directory; please see help message."
> On 2015/02/11 23:54:49, viettrungluu wrote:
> > I believe you can just add required=True to add_argument.
> 
> Nice! Done.
> 
>
https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
> File mojo/public/tools/download_shell_binary.py (right):
> 
>
https://codereview.chromium.org/870143008/diff/40001/mojo/public/tools/downlo...
> mojo/public/tools/download_shell_binary.py:79: print "Must specify
> --tools-directory; please see help message."
> On 2015/02/11 23:54:49, viettrungluu wrote:
> > ...
> 
> Done.

[blundell, 2015-02-12 19:31:28]

I think we need a find_find_depot_tools utility.

https://codereview.chromium.org/870143008/diff/60001/mojo/public/tools/pylib/...
File mojo/public/tools/pylib/gs.py (right):

https://codereview.chromium.org/870143008/diff/60001/mojo/public/tools/pylib/...
mojo/public/tools/pylib/gs.py:9: def download_from_public_bucket(gs_path,
output_path, find_depot_tools_path):
On 2015/02/12 17:20:25, viettrungluu wrote:
> It occurs to me that this is a very strange API. Probably you should just take
> depot_tools_path as an argument, and leave it to the caller to figure that
out.

Done.

[viettrungluu, 2015-02-12 20:08:23]

lgtm

[blundell, 2015-02-13 08:47:14]

Committed patchset #5 (id:80001) manually as
25540ebe3271805cd3697d5fb7e4d38b721fd49c (presubmit successful).