On Sun, Aug 31, 2014 at 11:40:14PM +1000, Stuart Prescott wrote: > > Hi! > > I've been contemplating deb822's support for build profiles some more and in > particular looked the recent changes to the build profiles / build > restrictions > specification. The attached patches are my current thinking on the matter. > > The main changes are: > > * there now can be multiple sets of < … > restrictions specified for each > package > > * as a result, we need to return a list of restrictions not a single > restriction > > * I propose flattening the representation of each restriction from (enabled, > (namespace, label)) to (enabled, namespace, label) > > * Following John's suggestion, I'm looking to use namedtuples to provide > backwards compatibility for the architecture restrictions while also > providing > a nicer syntax for accessors to both arch and build restrictions objects.
> Comments and suggestions welcome! Thanks for doing this. LGTM overall - some minor comments inline. (I've left the descriptions in place but stripped out the patch hunks with no comments.) > From b8c2dbde7989c97d84054a528a8739f85bf61fb1 Mon Sep 17 00:00:00 2001 > From: Stuart Prescott <[email protected]> > Date: Sun, 31 Aug 2014 20:22:06 +1000 > Subject: [PATCH 2/2] Update build restrictions parser for new syntax > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > * Support multiple sets of <…> expressions > * Correctly call it "namespace.label" rather than "namespace.name". > * Use namedtuples for BuildRestrictions in symmetry with ArchRestrictions > * Add support for turning build restrictions back into strings > * Update documentation for _PkgRelationMixin to include build restrictions > * Update tests with examples of multiple sets of <…> > --- > lib/debian/deb822.py | 71 > ++++++++++++++++++++++++++++++++------------ > tests/test_Sources | 2 +- > tests/test_Sources.iso8859-1 | 2 +- > tests/test_deb822.py | 46 ++++++++++++++++++++++++---- > 4 files changed, 95 insertions(+), 26 deletions(-) > @@ -917,27 +920,31 @@ class PkgRelation(object): > archs.append(cls.ArchRestriction(not disabled, arch)) > return archs > > - def parse_profiles(raw): > + def parse_restrictions(raw): > """ split a string of restrictions into a list of restrictions > > Each restriction is a tuple of form: > > - (active, (namespace, name)) > + (active, namespace, label) > > where > active: boolean: whether the restriction is positive or > negative > namespace: the namespace of the restriction e.g. 'profile' > - name: the name of the restriction e.g. 'stage1' > + label: the name of the restriction e.g. 'stage1' Might be nice to add a comment that the tuple fields are also accessible by name. > """ > restrictions = [] > - for restriction in cls.__blank_sep_RE.split(raw.lower().strip()): > - match = cls.__restriction_RE.match(restriction) > - if match: > - parts = match.groupdict() > - restrictions.append(( > - parts['enabled'] != '!', > - (parts['namespace'], parts['name']), > - )) > + for rgrp in cls.__restriction_sep_RE.split(raw.lower().strip('<> > ')): > + group = [] > + for restriction in cls.__blank_sep_RE.split(rgrp): > + match = cls.__restriction_RE.match(restriction) > + if match: > + parts = match.groupdict() > + group.append(cls.BuildRestriction( > + parts['enabled'] != '!', > + parts['namespace'], > + parts['label'], > + )) > + restrictions.append(group) > return restrictions > > > @@ -981,12 +988,25 @@ class PkgRelation(object): > arch_spec.arch, > ) > > + def pp_restrictions(restrictions): > + s = [] > + for r in restrictions: > + s.append('%s%s.%s' % ( > + '' if r.enabled else '!', > + r.namespace, > + r.label > + ) > + ) > + return '<%s>' % ' '.join(s) > + > def pp_atomic_dep(dep): > s = dep['name'] > if dep.get('version') is not None: Optional, while you're here, might make sense to either: - Use the more idiomatic form: 'version' not in dep - Or save the result of dep.get('version') instead of looking it up twice. This is one area where Go's syntax is really nice. You can do something like if version, ok := dep['version']; ok { // Do something with version } A similar construct for Python would be nice... > s += ' (%s %s)' % dep['version'] > if dep.get('arch') is not None: > s += ' [%s]' % ' '.join(map(pp_arch, dep['arch'])) > + if dep.get('restrictions') is not None: > + s += ' %s' % ' '.join(map(pp_restrictions, > dep['restrictions'])) > return s > > pp_or_dep = lambda deps: ' | '.join(map(pp_atomic_dep, deps)) > @@ -1098,10 +1100,14 @@ class TestPkgRelations(unittest.TestCase): > [rel({'name': 'flex'})], > [rel({'name': 'gettext', 'archqual': 'any'})], > [rel({'name': 'texinfo', > - 'restrictions': [(False, ('profile', 'stage1')), > (False, ('profile', 'cross'))]})], > + 'restrictions': [ > + [(False, 'profile', 'stage1')], Weird, these are equal even though one is a raw tuple and one is a namedtuple type? > + [(False, 'profile', 'stage2'), > + (False, 'profile', 'cross')] > + ]})], > [rel({'arch': [(True, 'hppa')], 'name': > 'expect-tcl8.3', > 'version': ('>=', '5.32.2'), > - 'restrictions': [(False, ('profile', > 'stage1'))]})], > + 'restrictions': [[(False, 'profile', > 'stage1')]]})], > [rel({'name': 'dejagnu', 'version': ('>=', > '1.4.2-1.1'), 'arch': None})], > [rel({'name': 'dpatch'})], > [rel({'name': 'file'})], -- John Wright <[email protected]>
signature.asc
Description: Digital signature
-- http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-python-debian-maint
