Add a script for installing Syzygy binaries directly from the archives.

Add a script for installing Syzygy binaries directly from the archives.

This will be the mechanism for distributing Syzygy binaries to Chrome going forward, as moving to a pure git repo will incur a much longer checkout for Chrome developers. This script will be invoked via a gclient hook, with a pinned revision.

BUG=
R=sebmarchand@chromium.org, siggi@chromium.org

Committed: https://code.google.com/p/sawbuck/source/detail?r=2148

[chrisha, 2014-05-08 17:51:14]

PTAL

(siggi and rogerm for Python readability)

[Sébastien Marchand, 2014-05-08 18:04:03]

Nice !

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
File syzygy/build/get_syzygy_binaries.py (right):

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
syzygy/build/get_syzygy_binaries.py:163: 'if it has been determined that they
arere up to date.')
s/arere/are/

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
syzygy/build/get_syzygy_binaries.py:169: option_parser.add_option('--verbose',
action='store_true', default=False,
Might also be:

parser.add_option('--verbose',
                  dest='log_level',
                  default=logging.ERROR,
                  action='store_const',
                  const=logging.INFO,
                  help='Enable verbose logging.')

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
syzygy/build/get_syzygy_binaries.py:280: continue
Shouldn't you be setting the executable bit after this ?

[chrisha, 2014-05-08 18:17:31]

Thanks, another look?

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
File syzygy/build/get_syzygy_binaries.py (right):

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
syzygy/build/get_syzygy_binaries.py:169: option_parser.add_option('--verbose',
action='store_true', default=False,
On 2014/05/08 18:04:03, Sébastien Marchand wrote:
> Might also be:
> 
> parser.add_option('--verbose',
>                   dest='log_level',
>                   default=logging.ERROR,
>                   action='store_const',
>                   const=logging.INFO,
>                   help='Enable verbose logging.')

Good point. Done.

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
syzygy/build/get_syzygy_binaries.py:280: continue
On 2014/05/08 18:04:03, Sébastien Marchand wrote:
> Shouldn't you be setting the executable bit after this ?

This whole loop is not needed anymore... vestigial from release_binaries...

[Sébastien Marchand, 2014-05-08 18:19:19]

lgtm.

[Siggi, 2014-05-08 18:31:21]

lgtm with a couple of nits.

https://codereview.appspot.com/94270045/diff/40001/syzygy/build/get_syzygy_bi...
File syzygy/build/get_syzygy_binaries.py (right):

https://codereview.appspot.com/94270045/diff/40001/syzygy/build/get_syzygy_bi...
syzygy/build/get_syzygy_binaries.py:226: archive_url = _SYZYGY_ARCHIVE_URL % {
'revision': options.revision }
So many archives.
Is this list likely to change over time? If so, will it be inconvenient to
change this script across two repos?
In that case, maybe change archiving, or maybe configure this list at the same
spot as the version in the Chrome repo?

https://codereview.appspot.com/94270045/diff/40001/syzygy/build/get_syzygy_bi...
syzygy/build/get_syzygy_binaries.py:248: shutil.rmtree(binaries_dir, True)
Hmmm, this seems a little draconian, is this how this is done for other
binaries?

https://codereview.appspot.com/94270045/diff/40001/syzygy/build/get_syzygy_bi...
syzygy/build/get_syzygy_binaries.py:287: state = _BuildState(options.revision,
options.output_dir)
You do this also on dry run? Maybe want to log and make conditional?

[Roger McFarlane, 2014-05-08 18:40:49]

Some python feedback

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
File syzygy/build/get_syzygy_binaries.py (right):

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
syzygy/build/get_syzygy_binaries.py:63: if not os.path.exists(path):
isfile

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
syzygy/build/get_syzygy_binaries.py:82: f.write(json.dumps(state,
sort_keys=True, indent=2))
json.dump(state, f, ...)

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
syzygy/build/get_syzygy_binaries.py:90: return hashlib.md5(open(path,
'r').read()).hexdigest()
use 'rb' to avoid python auto crlf magic on read.

uber-nit:

  with open(path, 'rb') as f:  # avoid python auto-string magic!
    return hashlib.md5(f.read()).hexdigest()

entirely optional.

https://codereview.appspot.com/94270045/diff/1/syzygy/build/get_syzygy_binari...
syzygy/build/get_syzygy_binaries.py:101: while dirs:
os.walk() ?

  # I think this is equivalent.
  for root, dirs, files in os.walk(output_dir):
    for f in files:
      p = os.path.join(root, f)
      r = os.path.relpath(p, output_dir)
      if r == _STATE:
        continue
      contents[r] = _Md5(p)

[chrisha, 2014-05-09 15:12:14]

Siggi convinced me that this needed to be a little more conservative in how it
deleted files from a previous installation, taking care not to blow away local
changes or additional files.

So, PTAL?

https://codereview.appspot.com/94270045/diff/40001/syzygy/build/get_syzygy_bi...
File syzygy/build/get_syzygy_binaries.py (right):

https://codereview.appspot.com/94270045/diff/40001/syzygy/build/get_syzygy_bi...
syzygy/build/get_syzygy_binaries.py:226: archive_url = _SYZYGY_ARCHIVE_URL % {
'revision': options.revision }
On 2014/05/08 18:31:21, Siggi wrote:
> So many archives.
> Is this list likely to change over time? If so, will it be inconvenient to
> change this script across two repos?
> In that case, maybe change archiving, or maybe configure this list at the same
> spot as the version in the Chrome repo?

It has changed a couple time, but is unlikely to do so further. I think I'll
keep it simple for now, but leave a note for this.

https://codereview.appspot.com/94270045/diff/40001/syzygy/build/get_syzygy_bi...
syzygy/build/get_syzygy_binaries.py:248: shutil.rmtree(binaries_dir, True)
On 2014/05/08 18:31:21, Siggi wrote:
> Hmmm, this seems a little draconian, is this how this is done for other
> binaries?

Other binaries do a very targeted download and install of a single binary at a
time. I'll see what I can do here...

https://codereview.appspot.com/94270045/diff/40001/syzygy/build/get_syzygy_bi...
syzygy/build/get_syzygy_binaries.py:287: state = _BuildState(options.revision,
options.output_dir)
On 2014/05/08 18:31:21, Siggi wrote:
> You do this also on dry run? Maybe want to log and make conditional?

Good point. Done.

[Siggi, 2014-05-09 16:59:05]

lgtm

https://codereview.appspot.com/94270045/diff/100001/syzygy/build/get_syzygy_b...
File syzygy/build/get_syzygy_binaries.py (right):

https://codereview.appspot.com/94270045/diff/100001/syzygy/build/get_syzygy_b...
syzygy/build/get_syzygy_binaries.py:143: """Validates if two state dictionaries
are consistent. Both must be valid
nit: if->whether?

[chrisha, 2014-05-09 18:41:48]

Thanks, committing.

https://codereview.appspot.com/94270045/diff/100001/syzygy/build/get_syzygy_b...
File syzygy/build/get_syzygy_binaries.py (right):

https://codereview.appspot.com/94270045/diff/100001/syzygy/build/get_syzygy_b...
syzygy/build/get_syzygy_binaries.py:143: """Validates if two state dictionaries
are consistent. Both must be valid
On 2014/05/09 16:59:05, Siggi wrote:
> nit: if->whether?

Done.

[chrisha, 2014-05-09 18:43:06]

Committed patchset #5 manually as r2148.