 Chromium Code Reviews
 Chromium Code Reviews Issue 2267363004:
  Add CIPD pin reporting to swarming.  (Closed) 
  Base URL: https://chromium.googlesource.com/external/github.com/luci/luci-py@master
    
  
    Issue 2267363004:
  Add CIPD pin reporting to swarming.  (Closed) 
  Base URL: https://chromium.googlesource.com/external/github.com/luci/luci-py@master| Index: appengine/swarming/cipd.py | 
| diff --git a/appengine/swarming/cipd.py b/appengine/swarming/cipd.py | 
| index d04a087800194dca5f81e7745c39a40cc395ec40..ba0cdd434fdee99a0ff0c178d0a26f3a5ce1c482 100644 | 
| --- a/appengine/swarming/cipd.py | 
| +++ b/appengine/swarming/cipd.py | 
| @@ -33,6 +33,78 @@ PARAM_OS_VER = '${os_ver}' | 
| ALL_PARAMS = (PARAM_PLATFORM, PARAM_OS_VER) | 
| +class PinChecker(object): | 
| + PARAM_PLATFORM_ESC = re.escape(PARAM_PLATFORM) | 
| + PARAM_OS_VER_ESC = re.escape(PARAM_OS_VER) | 
| + | 
| + def __init__(self): | 
| + self.platform = None | 
| + self.os_ver = None | 
| + | 
| + def check(self, original, expanded): | 
| 
M-A Ruel
2016/08/30 02:18:07
optional nit: I think I'd prefer if this function
 
iannucci
2016/08/30 09:13:29
Done.
edit: Actually I changed it to a contextman
 | 
| + """Raises ValueError if expanded is not pluasibly a pinned version of | 
| 
M-A Ruel
2016/08/30 02:18:07
plausibly
 
iannucci
2016/08/30 09:13:29
Done.
 | 
| + original. | 
| + | 
| + Args: | 
| + original - a CipdPackage which may contain templates like ${platform} | 
| + expanded - a CipdPackage which is nominally an expansion of original. | 
| + | 
| + CipdPackage is duck-typed to have three string properties 'package_name', | 
| + 'path' and 'version'. | 
| + | 
| + Side effects: | 
| + If this PinChecker has not encountered a platform or os_ver value yet, | 
| + the appropriate property will be set on this PinChecker. Subsequent | 
| + executions of check will replace ${platform} with the previously-scanned | 
| + value. | 
| + | 
| + Raises: | 
| + ValueError if expanded is not a valid derivation of original. | 
| + """ | 
| + if original.path != expanded.path: | 
| + raise ValueError('Mismatched path') | 
| + | 
| + def sub_param(regex, param_esc, param_re, param_const): | 
| + # The 1 allows each substitution to only show up in the pattern once. | 
| + # If it shows up more than once, then this is a very strange package name, | 
| + # and should be subject to further scrutiny. If it turns out that for some | 
| + # reason the substitutions SHOULD be allowed more than once per name, we | 
| + # will need to assert that the matched-values of them are also all the | 
| + # same (e.g. ${platform} cannot resolve to more than one value) | 
| + ret = False | 
| + if param_const is None: | 
| + ret = param_esc in regex | 
| + if ret: | 
| + regex = regex.replace(param_esc, param_re, 1) | 
| + else: | 
| + regex = regex.replace(param_esc, param_const, 1) | 
| + return regex, ret | 
| + | 
| + name_regex = re.escape(original.package_name) | 
| + name_regex, scan_plat = sub_param( | 
| + name_regex, self.PARAM_PLATFORM_ESC, '(?P<platform>\w+-[a-z0-9]+)', | 
| 
M-A Ruel
2016/08/30 02:18:07
Use r'' otherwise your \ may not do what you think
 
iannucci
2016/08/30 09:13:29
Done
 | 
| + self.platform) | 
| + name_regex, scan_os_ver = sub_param( | 
| + name_regex, self.PARAM_OS_VER_ESC, '(?P<os_ver>[_a-z0-9]+)', | 
| 
M-A Ruel
2016/08/30 02:18:07
r'' as per habit.
 
iannucci
2016/08/30 09:13:29
Done
 | 
| + self.os_ver) | 
| + | 
| + match = re.match(name_regex, expanded.package_name) | 
| + if not match: | 
| + raise ValueError('Mismatched package_name') | 
| + | 
| + if is_pinned_version(original.version): | 
| + if original.version != expanded.version: | 
| + raise ValueError('Mismatched pins') | 
| + else: | 
| + if not is_pinned_version(expanded.version): | 
| + raise ValueError('Pin value is not a pin') | 
| + | 
| + if scan_plat: | 
| + self.platform = re.escape(match.group('platform')) | 
| + if scan_os_ver: | 
| + self.os_ver = re.escape(match.group('os_ver')) | 
| + | 
| + | 
| def is_valid_package_name(package_name): | 
| """Returns True if |package_name| is a valid CIPD package name.""" | 
| return bool(PACKAGE_NAME_RE.match(package_name)) |