Create a tool to save a pinned dep file for the current sync.

Create a tool to save a pinned dep file for the current sync.

[piman, 2010-05-15 01:45:42]

A general comment, I don't know if you thought about it. I believe the buildbots
use a different internal url to access the git repos, so the generated DEPS file
may need further processing for general consumption.
From the buildbot logs:
argv: ['gclient', 'config',
'--spec=solutions=[{"name":"chromeos","url":"git://codf17.jail.google.com/git/repos/chromeos","custom_deps":{},"custom_vars":{},},]']

http://codereview.chromium.org/2112003/diff/1/4
File src/scripts/solution_to_deps.py (right):

http://codereview.chromium.org/2112003/diff/1/4#newcode10
src/scripts/solution_to_deps.py:10: #
How about reading the file in a contained environment ?

env = {}
execfile(file, env)
solutions = env['solutions']

It feels less of a hack, and you can add catch exceptions around execfile for
more resilience.

http://codereview.chromium.org/2112003/diff/1/4#newcode14
src/scripts/solution_to_deps.py:14: print ("#\n"
use multi-line strings ?
print """#
# Autogenerated pinned DEPS file.  This file was created by running
# save_pinned_deps.
#

use_relative_paths = True

deps = {"""


It's a lot more readable.

http://codereview.chromium.org/2112003/diff/1/4#newcode23
src/scripts/solution_to_deps.py:23: for (path, repository) in
solutions[0]["custom_deps"].items() :
no space before :

http://codereview.chromium.org/2112003/diff/1/4#newcode33
src/scripts/solution_to_deps.py:33: if (remainder != "deps") :
no space before :, no need for ()

http://codereview.chromium.org/2112003/diff/1/4#newcode37
src/scripts/solution_to_deps.py:37: print ("}\n")
no need for ()

[robotboy, 2010-05-18 23:13:32]

Comments addressed and a whole lot more.  I'm now saving a pinned solution file
because the pinned dep file can not pin the top level repository.  Unfortunately
this means that the URL that was used to check out the particular repository
leaks into the solution file.  That means that to use these auto-generated
gclient files they will need to be hand edited.  I have facilitated this by
pulling that base URL out into a single obvious location in the file.

Additional support from gclient would be needed to allow this base URL to be
passed by the user from the command line.

http://codereview.chromium.org/2112003/diff/1/4
File src/scripts/solution_to_deps.py (right):

http://codereview.chromium.org/2112003/diff/1/4#newcode10
src/scripts/solution_to_deps.py:10: #
On 2010/05/15 01:45:43, piman wrote:
> How about reading the file in a contained environment ?
> 
> env = {}
> execfile(file, env)
> solutions = env['solutions']
> 
> It feels less of a hack, and you can add catch exceptions around execfile for
> more resilience.

Done.

http://codereview.chromium.org/2112003/diff/1/4#newcode14
src/scripts/solution_to_deps.py:14: print ("#\n"
Google style guide disagrees with you on this one.

http://codereview.chromium.org/2112003/diff/1/4#newcode23
src/scripts/solution_to_deps.py:23: for (path, repository) in
solutions[0]["custom_deps"].items() :
On 2010/05/15 01:45:43, piman wrote:
> no space before :

Done.

http://codereview.chromium.org/2112003/diff/1/4#newcode33
src/scripts/solution_to_deps.py:33: if (remainder != "deps") :
On 2010/05/15 01:45:43, piman wrote:
> no space before :, no need for ()

Done.

http://codereview.chromium.org/2112003/diff/1/4#newcode37
src/scripts/solution_to_deps.py:37: print ("}\n")
On 2010/05/15 01:45:43, piman wrote:
> no need for ()

Done.

[piman, 2010-05-18 23:51:35]

http://codereview.chromium.org/2112003/diff/6001/7002
File src/scripts/make_relative_solution (right):

http://codereview.chromium.org/2112003/diff/6001/7002#newcode13
src/scripts/make_relative_solution:13: def readSnapshot(filename):
Style guide says CapWords for function names (here and below)

http://codereview.chromium.org/2112003/diff/6001/7002#newcode14
src/scripts/make_relative_solution:14: """Read a gclient revinfo snapshot file
and do minimal verification that
Read->Reads

http://codereview.chromium.org/2112003/diff/6001/7002#newcode44
src/scripts/make_relative_solution:44: match =
re.search('/[^/]+@[0-9a-fA-F]{40}$', url)
how about rsplit('/', 1) ?

http://codereview.chromium.org/2112003/diff/6001/7002#newcode85
src/scripts/make_relative_solution:85: """Write a pinned dep.  The output is
indented so that the URLs all line up
Write->Writes

http://codereview.chromium.org/2112003/diff/6001/7002#newcode117
src/scripts/make_relative_solution:117: """Write a gclient solution file to
stdout that pins all of the repositories
Write->writes

http://codereview.chromium.org/2112003/diff/6001/7002#newcode156
src/scripts/make_relative_solution:156: return 1
How about checking after the parse that you got a filename ?

http://codereview.chromium.org/2112003/diff/6001/7003
File src/scripts/save_pinned_deps (right):

http://codereview.chromium.org/2112003/diff/6001/7003#newcode2
src/scripts/save_pinned_deps:2: PROG=$(basename $0)
I don't think you're using this. If you do, can you move it below the copyright
header ?

[robotboy, 2010-05-19 22:05:40]

This version also supports substituting a new base git URL to
make the resulting files easier to use externally.

http://codereview.chromium.org/2112003/diff/6001/7002
File src/scripts/make_relative_solution (right):

http://codereview.chromium.org/2112003/diff/6001/7002#newcode13
src/scripts/make_relative_solution:13: def readSnapshot(filename):
On 2010/05/18 23:51:35, piman wrote:
> Style guide says CapWords for function names (here and below)

Done.  Hah, I was looking at an external Google Python style guide that didn't
agree...

http://codereview.chromium.org/2112003/diff/6001/7002#newcode14
src/scripts/make_relative_solution:14: """Read a gclient revinfo snapshot file
and do minimal verification that
On 2010/05/18 23:51:35, piman wrote:
> Read->Reads

Done.

http://codereview.chromium.org/2112003/diff/6001/7002#newcode44
src/scripts/make_relative_solution:44: match =
re.search('/[^/]+@[0-9a-fA-F]{40}$', url)
On 2010/05/18 23:51:35, piman wrote:
> how about rsplit('/', 1) ?

Done.

http://codereview.chromium.org/2112003/diff/6001/7002#newcode85
src/scripts/make_relative_solution:85: """Write a pinned dep.  The output is
indented so that the URLs all line up
On 2010/05/18 23:51:35, piman wrote:
> Write->Writes

Done.

http://codereview.chromium.org/2112003/diff/6001/7002#newcode117
src/scripts/make_relative_solution:117: """Write a gclient solution file to
stdout that pins all of the repositories
On 2010/05/18 23:51:35, piman wrote:
> Write->writes

Done.

[piman, 2010-05-19 22:48:52]

http://codereview.chromium.org/2112003/diff/15001/16003
File src/scripts/make_relative_solution (right):

http://codereview.chromium.org/2112003/diff/15001/16003#newcode18
src/scripts/make_relative_solution:18: __author__ = 'robotboy@google.com (Anton
Staaf)'
We actually don't put author lines in open source code. The copyright refers to
the AUTHORS files and we're covered that way.

http://codereview.chromium.org/2112003/diff/15001/16004
File src/scripts/save_pinned_deps (right):

http://codereview.chromium.org/2112003/diff/15001/16004#newcode90
src/scripts/save_pinned_deps:90: git reset origin/master
as we discussed, this is rather scary (pushing with dirty workspace), so if
possible to use rebase please do that. Otherwise please add some docs so that
it's clear what this is doing.

[robotboy, 2010-05-19 23:21:14]

Fixed the commit sequence and removed the authors line, PTAL.

[piman, 2010-05-19 23:29:02]

LGTM either way, but I was just thinking if you want to reduce the window for
races, you can move the git remote update right before the rebase.