cipd recipe_module

scripts/slave/recipe_modules/cipd/api.py

+# Copyright 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+from recipe_engine import recipe_api
+
+class CIPDApi(recipe_api.RecipeApi):
+  """CIPDApi provides support for CIPD."""
+  NATIVE = "native"

[Vadim Sh., 2015/06/29 21:00:46]

nit: remove it unless it's used for real

[seanmccullough, 2015/06/30 17:39:39]

Done.

+
+  def install_client(self,
+      step_name,
+      # TODO: get the arch from env instead of hardcoding it.
+      package='infra/tools/cipd',
+      version='9579504cec0336688292f5b0b68c3ed4e288e273'):

[Vadim Sh., 2015/06/29 21:00:46]

I prefer versions to be expressed as a dict, e.g.

class CIPDApi(...):

  CLIENT_VERSIONS = {
    'infra/tools/cipd/linux-amd64': '9579....',
    'infra/tools/cipd/mac-amd64': '....',
    ....
  }

[seanmccullough, 2015/06/30 17:39:39]

Done.

+    self.bin_path = ("cipd_%s" % version)

[Vadim Sh., 2015/06/29 21:00:46]

nit: () is unnecessary

Also use self.m.path to construct an absolute path, e.g.
self.m.path['slave_build'].join('cipd/cipd_%s' % version).

[seanmccullough, 2015/06/30 17:39:39]

Done.

+    print ("should install cipd at %s" % self.bin_path)

[Vadim Sh., 2015/06/29 21:00:46]

print is not allowed in recipes.

[seanmccullough, 2015/06/30 17:39:39]

Done.

+    package = "%s/%s" % (package, self.platform_tag(self.NATIVE))
+    script_input = {
+      'package': package,
+      'version': version,
+    }
+
+    self.m.python(
+        name=step_name,
+        script=self.resource('bootstrap.py'),
+        stdin=self.m.json.input(script_input))
+
+    # TODO: clean up older CIPD installations.
+    return self.bin_path
+
+  def platform_tag(self, style):
+    return "linux-amd64"
+
+  def ensure_installed(self, root):
+    pkg_list = []
+    for pkg_name in self.c.packages:

[Vadim Sh., 2015/06/29 21:00:46]

sorted(self.c.packages), for reproducibility

[seanmccullough, 2015/06/30 17:39:38]

Done.

+      pkg_spec = self.c.packages[pkg_name]
+      pkg_list.append(("%s %s" % (pkg_name, pkg_spec['version'])))

[Vadim Sh., 2015/06/29 21:00:46]

() is not required

[seanmccullough, 2015/06/30 17:39:39]

Done.

+
+    print "*** pkg list: ***"
+    print "\n".join(pkg_list)
+
+    self.m.file.write("write_test", "pkg_list.txt", "\n".join(pkg_list))

[Vadim Sh., 2015/06/29 21:00:46]

this is unnecessary, use raw_io module instead (search code for raw_io.input for
examples).

[seanmccullough, 2015/06/30 17:39:38]

Done.

+
+    script_input = {
+      'list': "pkg_list.txt",
+      'root': root,
+    }
+    self.m.python(
+      name="ensure_installed",
+      script=self.resource('ensure.py'),

[Vadim Sh., 2015/06/29 21:00:46]

what is the purpose to ensure.py? Why not

self.m.step(
    name="ensure_installed",
    [self.bin_path, "ensure",  ....])

?

[seanmccullough, 2015/06/30 17:39:38]

Done.

+      stdin=self.m.json.input(script_input))
+
+    print("package: %s/%s", pkg_name, pkg_spec['version'])
+
+