Updating python-debian support for build profiles
John Wright
jsw at debian.org
Mon Sep 1 07:46:32 UTC 2014
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 <stuart at debian.org>
> 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 <jsw at debian.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20140901/043ff8f2/attachment.sig>
More information about the pkg-python-debian-maint
mailing list