Convert package lists in hints to actual lists

Convert package lists in hints to actual lists in one place, rather than
doing it every time we need to use the value.

This fixes the bug that build-requires wasn't written to JSON as a list.

Update test data appropriately.

Future work: Factor out "strip out version constraints from a package
list" into a utility function.

calm/package.py

@@ -268,13 +268,30 @@ def read_hints(p, fn, kind, strict=False):
         for l in hints['parse-warnings']:
             logging.info("package '%s': %s" % (p, l))
 
-    # generate depends: from requires:
-    # XXX: store this as a list, rather than splitting it into one everywhere we
-    # use it
-    hints['depends'] = ', '.join(process_package_constraint_list(hints.get('requires', '')))
-    # erase requires:, to ensure there is nothing using it
+    # convert hint keys which have a value which is a list to an actual list (to
+    # avoid doing the splitting and whitespace handling everywhere)
+    #
+    # XXX: guarantee they exist and are an empty list if empty, so we don't need
+    # to check if they exist everywhere?)
+    for k in ['obsoletes', 'provides', 'conflicts', 'build-depends']:
+        if k in hints:
+            v = hints[k].strip()
+            if not v:
+                v = []
+            else:
+                # split on comma, remove any extraneous whitespace
+                v = [i.strip() for i in v.split(',')]
+            hints[k] = v
+
+    # 'depends' is special, generated from from requires:
+    hints['depends'] = process_package_constraint_list(hints.get('requires', ''))
+    # erase requires:, to ensure there is nothing still using it
     hints.pop('requires', None)
 
+    # disable check is just whitespace separated
+    if 'disable-check' in hints:
+        hints['disable-check'] = hints['disable-check'].split()
+
     return hints
 
 
@@ -627,7 +644,7 @@ def upgrade_oldstyle_obsoletes(packages, missing_obsolete):
                     continue
                 logging.debug("_obsolete package '%s' version '%s' mtime '%s' is over cut-off age" % (p, vr, time.strftime("%F %T %Z", time.localtime(mtime))))
 
-                requires = packages[p].version_hints[vr].get('depends', '').split(', ')
+                requires = packages[p].version_hints[vr].get('depends', [])
                 requires = [re.sub(r'(.*) +\(.*\)', r'\1', r) for r in requires]
 
                 o = None
@@ -645,7 +662,7 @@ def upgrade_oldstyle_obsoletes(packages, missing_obsolete):
 
                 else:
                     # ignore self-destruct packages
-                    provides = packages[p].version_hints[vr].get('provides', '')
+                    provides = packages[p].version_hints[vr].get('provides', [])
                     if '_self-destruct' in provides:
                         continue
 
@@ -693,7 +710,7 @@ def validate_packages(args, packages, valid_provides_extra=None, missing_obsolet
     for p in packages:
         valid_requires.add(p)
         for hints in packages[p].version_hints.values():
-            valid_requires.update(hints.get('provides', '').split())
+            valid_requires.update(hints.get('provides', []))
 
             # reset computed package state
             packages[p].has_requires = False
@@ -718,10 +735,7 @@ def validate_packages(args, packages, valid_provides_extra=None, missing_obsolet
             ]:
                 # if c is in hints, and not the empty string
                 if hints.get(c, ''):
-                    for r in hints[c].split(','):
-                        # remove any extraneous whitespace
-                        r = r.strip()
-
+                    for r in hints[c]:
                         # strip off any version relation enclosed in '()'
                         # following the package name
                         r = re.sub(r'(.*) +\(.*\)', r'\1', r)
@@ -767,15 +781,13 @@ def validate_packages(args, packages, valid_provides_extra=None, missing_obsolet
             if p in packages:
                 for v in packages[p].version_hints:
 
-                    obsoletes = packages[p].version_hints[v].get('obsoletes', '').split(',')
-                    obsoletes = [o.strip() for o in obsoletes]
-                    obsoletes = [o for o in obsoletes if o]
+                    obsoletes = packages[p].version_hints[v].get('obsoletes', [])
 
                     def add_needed_obsoletes(needed):
                         for n in sorted(needed):
                             if n not in obsoletes:
                                 obsoletes.append(n)
-                                packages[p].version_hints[v]['obsoletes'] = ', '.join(obsoletes)
+                                packages[p].version_hints[v]['obsoletes'] = obsoletes
                                 logging.info("added 'obsoletes: %s' to package '%s' version '%s'" % (n, p, v))
 
                             # recurse so we don't drop transitive missing obsoletes
@@ -794,21 +806,20 @@ def add_needed_obsoletes(needed):
         # in read_hints(), so fix that up here.
     for p in sorted(packages):
         for hints in packages[p].version_hints.values():
-            obsoletes = hints.get('obsoletes', '')
+            obsoletes = hints.get('obsoletes', [])
             if obsoletes:
-                for o in obsoletes.split(','):
-                    o = o.strip()
+                for o in obsoletes:
                     o = re.sub(r'(.*) +\(.*\)', r'\1', o)
 
                     if o in packages:
                         packages[o].obsolete = True
 
                         for (ov, ohints) in packages[o].version_hints.items():
                             if 'depends' in ohints:
-                                depends = ohints['depends'].split(', ')
+                                depends = ohints['depends']
                                 if p in depends:
                                     depends = [d for d in depends if d != p]
-                                    packages[o].version_hints[ov]['depends'] = ', '.join(depends)
+                                    packages[o].version_hints[ov]['depends'] = depends
                                     logging.debug("removed obsoleting '%s' from the depends: of package '%s'" % (p, o))
                     else:
                         logging.debug("can't ensure package '%s' doesn't depends: on obsoleting '%s'" % (o, p))
@@ -910,7 +921,7 @@ def add_needed_obsoletes(needed):
                 if packages[p].tar(vr).is_empty:
                     # this classification relies on obsoleting packages
                     # not being present in depends
-                    if packages[p].version_hints[vr].get('depends', ''):
+                    if packages[p].version_hints[vr].get('depends', []):
                         # also allow '_obsolete' because old obsoletion
                         # packages depend on their replacement, but are not
                         # obsoleted by it
@@ -950,9 +961,7 @@ def add_needed_obsoletes(needed):
                     ('obsoletes', 'obsoleted_by'),
             ]:
                 if k in hints:
-                    dpl = hints[k].split(',')
-                    for dp in dpl:
-                        dp = dp.strip()
+                    for dp in hints[k]:
                         dp = re.sub(r'(.*)\s+\(.*\)', r'\1', dp)
                         if dp in packages:
                             getattr(packages[dp], a).add(p)
@@ -1104,7 +1113,7 @@ def assign_importance(packages):
     # recursively give dependencies of base packages the basedep importance
     def recursive_basedep(p):
         bv = p.best_version
-        requires = p.version_hints[bv].get('depends', '').split(', ')
+        requires = p.version_hints[bv].get('depends', [])
         requires = [re.sub(r'(.*) +\(.*\)', r'\1', r) for r in requires]
         for r in requires:
             if r in packages:
@@ -1354,27 +1363,26 @@ def write_setup_ini(args, packages, arch):
 
                 if version in po.versions():
                     if hints.get('depends', ''):
-                        print("depends2: %s" % hints.get('depends', ''), file=f)
+                        print("depends2: %s" % ', '.join(hints.get('depends', [])), file=f)
 
                     if hints.get('obsoletes', ''):
-                        print("obsoletes: %s" % hints['obsoletes'], file=f)
+                        print("obsoletes: %s" % ', '.join(hints['obsoletes']), file=f)
 
                     if hints.get('provides', ''):
-                        print("provides: %s" % hints['provides'], file=f)
+                        print("provides: %s" % ', '.join(hints['provides']), file=f)
 
                     if hints.get('conflicts', ''):
-                        print("conflicts: %s" % hints['conflicts'], file=f)
+                        print("conflicts: %s" % ','.join(hints['conflicts']), file=f)
 
                 if s:
                     src_hints = packages[s].version_hints.get(version, {})
-                    bd = src_hints.get('build-depends', '')
+                    bd = src_hints.get('build-depends', [])
 
                     # Ideally, we'd transform dependency atoms which aren't
                     # cygwin package names into package names. For the moment,
                     # we don't have the information to do that, so filter them
                     # all out.
-                    if bd:
-                        bd = [atom for atom in bd.split(', ') if '(' not in atom]
+                    bd = [atom for atom in bd if '(' not in atom]
 
                     if bd:
                         print("build-depends: %s" % ', '.join(bd), file=f)
@@ -1460,7 +1468,7 @@ def package(p):
             sp = {'name': sp, 'categories': hints.get('category', '').split()}
             for k in ['depends', 'provides', 'obsoletes']:
                 if hints.get(k, None):
-                    sp[k] = [d.strip() for d in hints[k].split(',')]
+                    sp[k] = hints[k]
             spl.append(sp)
         d['subpackages'] = spl
 
@@ -1645,7 +1653,7 @@ def mark_fn(packages, po, v, certain_age, vault_requests):
 
     # - if package depends on anything in expired_provides
     #
-    requires = po.version_hints[v].get('depends', '').split(', ')
+    requires = po.version_hints[v].get('depends', [])
     if any(ep in requires for ep in past_mistakes.expired_provides):
         logging.debug("package '%s' version '%s' not retained as it requires a provide known to be expired" % (pn, v))
         return Freshness.conditional

calm/pkg2html.py

@@ -248,12 +248,8 @@ class PackageData(NamedTuple):
                             if details[key].is_attr:
                                 value[arch] = getattr(pos[arch], key, set())
                             else:
-                                t = pos[arch].version_hints[pos[arch].best_version].get(key, None)
-
-                                if t:
-                                    value[arch] = set(t.split(', '))
-                                else:
-                                    value[arch] = set()
+                                t = pos[arch].version_hints[pos[arch].best_version].get(key, [])
+                                value[arch] = set(t)
                             values.update(value[arch])
 
                         if values:

calm/reports.py

@@ -295,15 +295,15 @@ def provides_rebuild(args, packages, fn, provide_package, reportlist):
 
     if pp_package:
         pp_bv = pp_package.best_version
-        pp_provide = pp_package.version_hints[pp_bv]['provides']
+        pp_provide = pp_package.version_hints[pp_bv]['provides'][0]
         pp_provide_base = re.sub(r'\d+$', '', pp_provide)
 
         for p in packages[arch]:
             po = packages[arch][p]
             bv = po.best_version
 
-            depends = packages[arch][p].version_hints[bv]['depends'].split(', ')
-            depends = [re.sub(r'(.*) +\(.*\)', r'\1', r) for r in depends]
+            depends = packages[arch][p].version_hints[bv]['depends']
+            depends = utils.deplist_without_verrel(depends)
 
             for d in depends:
                 if not d.startswith(pp_provide_base):
@@ -350,12 +350,12 @@ def python_rebuild(args, packages, fn, reportlist):
     # XXX: look into how we can change this, after x86 is dropped
     arch = 'x86_64'
 
-    # assume that python3 depends on the latest python3n package
+    # assume that python3 depends only on the latest python3n package
     py_package = packages[arch].get('python3', None)
     if not py_package:
         return
 
-    latest_py = py_package.version_hints[py_package.best_version]['depends'].split(', ')[0]
+    latest_py = py_package.version_hints[py_package.best_version]['depends'][0]
 
     modules = {}
 
@@ -366,8 +366,8 @@ def python_rebuild(args, packages, fn, reportlist):
         if po.obsoleted_by:
             continue
 
-        depends = packages[arch][p].version_hints[bv]['depends'].split(', ')
-        depends = [re.sub(r'(.*) +\(.*\)', r'\1', r) for r in depends]
+        depends = packages[arch][p].version_hints[bv]['depends']
+        depends = utils.deplist_without_verrel(depends)
 
         for d in depends:
             # scan for a 'pythonnn' dependency

calm/utils.py

@@ -204,8 +204,5 @@ def sendmail(hdr, msg):
 #
 # remove version-constrains from a list of dependencies
 #
-def deplist_without_verrel(dl):
-    dpl = dl.split(',')
-    dpl = [dp.strip() for dp in dpl]
-    dpl = [re.sub(r'(.*)\s+\(.*\)', r'\1', dp) for dp in dpl]
-    return dpl
+def deplist_without_verrel(dpl):
+    return [re.sub(r'(.*)\s+\(.*\)', r'\1', dp) for dp in dpl]

test/testdata/process_arch/packages.json.expected

@@ -411,7 +411,9 @@
  '            "arches": [\n'
  '                "x86_64"\n'
  '            ],\n'
- '            "build-depends": "cygwin-devel",\n'
+ '            "build-depends": [\n'
+ '                "cygwin-devel"\n'
+ '            ],\n'
  '            "name": "test-e",\n'
  '            "subpackages": [\n'
  '                {\n'

test/testdata/uploads/pkglist.expected

@@ -4,7 +4,7 @@
                     'Like it’s you’re Markup Language™ Nokogiri’s tool―that '
                     'Bézier."',
            'category': 'Devel',
-           'depends': 'cygwin'}}, {}, False),
+           'depends': ['cygwin']}}, {}, False),
  'testpackage-src': Package('testpackage', {'1.0-1': Tar('testpackage-1.0-1-src.tar.bz2', 'x86_64/release/testpackage', 'acfd77df3347e6432ccf29c12989964bc680a158d574f85dfa7ef222759f411006c7bd2773e37c5abdee628bea769b2da9aae213db615cd91402fd385373933d', 266, False)}, {'1.0-1': {'sdesc': '"A test package"',
            'ldesc': '"A test package\n'
                     "It's description might contains some unicode junk\n"
@@ -13,25 +13,25 @@
            'category': 'Devel',
            'homepage': 'http://homepage.url',
            'parse-warnings': ["key 'license' missing"],
-           'depends': ''}}, {}, False),
+           'depends': []}}, {}, False),
  'testpackage-subpackage': Package('testpackage/testpackage-subpackage', {'1.0-1': Tar('testpackage-subpackage-1.0-1.tar.bz2', 'x86_64/release/testpackage/testpackage-subpackage', 'aff488008bee3486e25b539fe6ccd1397bd3c5c0ba2ee2cf34af279554baa195af7493ee51d6f8510735c9a2ea54436d776a71e768165716762aec286abbbf83', 195, False)}, {'1.0-1': {'sdesc': '"A test subpackage"',
            'ldesc': '"A test subpackage"',
            'category': 'Devel',
            'external-source': 'testpackage-src',
-           'depends': ''}}, {}, False),
+           'depends': []}}, {}, False),
  'testpackage-zstd': Package('testpackage-zstd', {'1.0-1': Tar('testpackage-zstd-1.0-1.tar.zst', 'x86_64/release/testpackage-zstd', '044066c54c036190f9b0496ccf31f74748d209cce961352e19631876d5abd79ef6d2b34edfb955b8d1a7a781294ee0636bb1305afe410b34562367a2cb77988d', 98, False)}, {'1.0-1': {'category': 'Base',
            'sdesc': '"test package (zstd compressed)"',
            'ldesc': '"test package (zstd compressed)"',
-           'depends': ''}}, {}, False),
+           'depends': []}}, {}, False),
  'testpackage-zstd-src': Package('testpackage-zstd', {'1.0-1': Tar('testpackage-zstd-1.0-1-src.tar.zst', 'x86_64/release/testpackage-zstd', '90561ec4dad76268773856cbdda891b0e7b53f26492777f1ff76757844cb47124396feb76f1e30bc1baa680f1d788de21d89e612faeb30b5039b210ca9186434', 313, False)}, {'1.0-1': {'category': 'Base',
-           'build-depends': 'cygport',
+           'build-depends': ['cygport'],
            'sdesc': '"test package (zstd compressed)"',
            'ldesc': '"test package (zstd compressed)"',
            'homepage': 'http://zstd.testpkg.invalid',
            'skip': '',
            'parse-warnings': ["key 'license' missing"],
-           'depends': ''}}, {}, False),
+           'depends': []}}, {}, False),
  'testpackage2-subpackage': Package('testpackage2/testpackage2-subpackage', {'1.0-1': Tar('testpackage2-subpackage-1.0-1.tar.bz2', 'x86_64/release/testpackage2/testpackage2-subpackage', 'c4bf8e28d71b532e2b741e2931906dec0f0a70d4d051c0503476f864a5228f43765ae3342aafcebfd5a1738073537726b2bfbbd89c6da939a5f46d95aca3feaf', 46, True)}, {'1.0-1': {'sdesc': '"A test subpackage 2"',
            'ldesc': '"A test subpackage 2"',
            'category': 'Devel',
-           'depends': ''}}, {}, False)}
+           'depends': []}}, {}, False)}