Fix generate_policy_source.py to honour supported_on ranges.

components/policy/tools/generate_policy_source.py

Index: components/policy/tools/generate_policy_source.py
diff --git a/components/policy/tools/generate_policy_source.py b/components/policy/tools/generate_policy_source.py
index b3a723b72e6da0c8dea5acb2698afd9028d6c1f3..37f15d5599725902b6016d6fa1b51a83092113a9 100755
--- a/components/policy/tools/generate_policy_source.py
+++ b/components/policy/tools/generate_policy_source.py
@@ -62,7 +62,7 @@ class PolicyDetails:
       self.caption = PolicyDetails._RemovePlaceholders(item['caption'])
       self.value = item['value']
 
-  def __init__(self, policy, os, is_chromium_os):
+  def __init__(self, policy, version, os, is_chromium_os):
     self.id = policy['id']
     self.name = policy['name']
     features = policy.get('features', {})
@@ -75,10 +75,21 @@ class PolicyDetails:
     if self.has_enterprise_default:
       self.enterprise_default = policy['default_for_enterprise_users']
 
+    major_version = int(version.split('.')[0])
     expected_platform = 'chrome_os' if is_chromium_os else os.lower()
     self.platforms = []
-    for platform, version in [ p.split(':') for p in policy['supported_on'] ]:
-      if not version.endswith('-'):
+    for platform, version_range in [ p.split(':')
+                                     for p in policy['supported_on'] ]:
+      split_result = version_range.split('-')
+      if len(split_result) != 2:
+        raise RuntimeError('supported_on must have exactly one dash: "%s"' % p)
+      (version_min, version_max) = split_result
+      if version_min == '':
+        raise RuntimeError('supported_on must define a start version: "%s"' % p)
+
+      # Skip if the current Chromium version does not support the policy.
+      if (int(version_min) > major_version or
+          version_max != '' and int(version_max) < major_version):
         continue
 
       if platform.startswith('chrome.'):
@@ -155,17 +166,28 @@ def main():
 
   (opts, args) = parser.parse_args()
 
-  if len(args) != 3:
-    print 'exactly platform, chromium_os flag and input file must be specified.'
+  if len(args) != 4:
+    print('Exactly VERSION path, platform, chromium_os flag and '

[Mattias Nissler (ping if slow), 2015/04/23 09:01:29]

what is a VERSION path?

[Thiemo Nagel, 2015/04/23 09:58:00]

Maybe a VERSION file?

[Mattias Nissler (ping if slow), 2015/04/23 10:14:52]

How about "src/chrome/VERSION file path" ?

[Thiemo Nagel, 2015/04/23 10:56:26]

How about "path to src/chrome/VERSION"?

+          'input file must be specified as free parameters.')

[Mattias Nissler (ping if slow), 2015/04/23 09:01:29]

free parameters? as opposed to bound parameters? I think the more common term is
"arguments"

[Thiemo Nagel, 2015/04/23 09:58:00]

The correct term seems to be "positional parameters" (200k Google results) or
"positional arguments" (90k Google results).

     parser.print_help()
     return 2
 
-  os = args[0]
-  is_chromium_os = args[1] == '1'
-  template_file_name = args[2]
+  version_path = args[0]
+  os = args[1]
+  is_chromium_os = args[2] == '1'
+  template_file_name = args[3]
+
+  version = None

[Mattias Nissler (ping if slow), 2015/04/23 09:01:29]

It looks like this stuff should be broken out into a function

[Thiemo Nagel, 2015/04/23 09:58:00]

Done.

+  for line in open(version_path, 'r').readlines():
+    key, val = line.rstrip('\r\n').split('=', 1)
+    if key == 'MAJOR':
+      version = val
+      break
+  if version == None:

[Mattias Nissler (ping if slow), 2015/04/23 09:01:29]

nit: if version is None

[Thiemo Nagel, 2015/04/23 09:58:00]

Done.

+    raise RuntimeError('VERSION file does not contain major version.')
 
   template_file_contents = _LoadJSONFile(template_file_name)
-  policy_details = [ PolicyDetails(policy, os, is_chromium_os)
+  policy_details = [ PolicyDetails(policy, version, os, is_chromium_os)
                      for policy in _Flatten(template_file_contents) ]
   sorted_policy_details = sorted(policy_details, key=lambda policy: policy.name)