swarming: custom cipd package paths

appengine/swarming/server/task_request.py

Index: appengine/swarming/server/task_request.py
diff --git a/appengine/swarming/server/task_request.py b/appengine/swarming/server/task_request.py
index 7e4c1b8ef32c11ab563eb47b46ec3048ca37c6db..de6d90286b13c8ae7ed1fa8d21f84cecc48c125d 100644
--- a/appengine/swarming/server/task_request.py
+++ b/appengine/swarming/server/task_request.py
@@ -46,6 +46,7 @@ separate entity to clearly declare the boundary for task request deduplication.
 """
 
 
+import collections
 import datetime
 import hashlib
 import random
@@ -259,6 +260,9 @@ class CipdPackage(ndb.Model):
   # Most users will specify tags.
   version = ndb.StringProperty(
       indexed=False, validator=_validate_package_version)
+  # Path to dir, relative to the run dir, where to install the package.
+  # If empty, the package will be installed in run dir.
+  path = ndb.StringProperty(indexed=False)
 
   def __str__(self):
     return '%s:%s' % (self.package_name, self.version)
@@ -270,6 +274,18 @@ class CipdPackage(ndb.Model):
     if not self.version:
       raise datastore_errors.BadValueError('CIPD package version is required')
 
+    if self.path:

[M-A Ruel, 2016/06/15 17:28:27]

Use a validator instead.

[nodir, 2016/06/15 17:53:42]

Done.

+      if '\\' in self.path:
+        raise datastore_errors.BadValueError(
+            'CIPD package path cannot contain \\. On Windows forward-slashes '
+            'will be replaced with back-slashes.')
+      if '..' in self.path.split('/'):
+        raise datastore_errors.BadValueError(
+            'CIPD package path cannot contain "..".')
+      if self.path.startswith('/'):
+        raise datastore_errors.BadValueError(
+            'CIPD package path cannot start with "/".')
+
 
 class CipdInput(ndb.Model):
   """Specifies which CIPD client and packages to install, from which server.
@@ -281,6 +297,7 @@ class CipdInput(ndb.Model):
 
   # CIPD package of CIPD client to use.
   # client_package.version is required.
+  # client_package.path must be None.
   client_package = ndb.LocalStructuredProperty(CipdPackage)
 
   # List of packages to install in $CIPD_PATH prior task execution.
@@ -291,6 +308,8 @@ class CipdInput(ndb.Model):
       raise datastore_errors.BadValueError('cipd server is required')
     if not self.client_package:
       raise datastore_errors.BadValueError('client_package is required')
+    if self.client_package.path:
+      raise datastore_errors.BadValueError('client_package.path must be unset')
     self.client_package._pre_put_hook()
 
     if not self.packages:
@@ -306,6 +325,14 @@ class CipdInput(ndb.Model):
       package_names.add(p.package_name)
     self.packages.sort(key=lambda p: p.package_name)
 
+  def packages_grouped_by_path(self):
+    """Returns sorted [(path), [package]) list. Used by user_task.html."""
+    packages = collections.defaultdict(list)
+    for p in self.packages:
+      packages[p.path or '.'].append(p)

[M-A Ruel, 2016/06/15 17:28:27]

I don't like that path=None and path='.' have the same outcome. This means one
can specify the same task in two different ways and get different
properties_hash. I prefer to unconditionally store '.', never None. This
simplify the code, as in user_task.html.

[nodir, 2016/06/15 17:53:41]

Good point, done.

+    for pkgs in packages.values():

[M-A Ruel, 2016/06/15 17:28:27]

itervalues()

[nodir, 2016/06/15 17:53:42]

Done.

+      pkgs.sort()
+    return sorted(packages.iteritems())
 

[M-A Ruel, 2016/06/15 17:28:27]

2 lines

[nodir, 2016/06/15 17:53:42]

Done.

 class TaskProperties(ndb.Model):
   """Defines all the properties of a task to be run on the Swarming