launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === modified file 'lpbuildd/binarypackage.py'
> --- lpbuildd/binarypackage.py	2015-07-16 13:03:35 +0000
> +++ lpbuildd/binarypackage.py	2015-07-23 12:27:10 +0000
> @@ -180,18 +200,42 @@
>      def relationMatches(self, dep, available):
>          """Return True iff a dependency matches an available package.
>  
> -        :param dep: A dictionary with at least a "name" key, perhaps also a
> -            "version" key, and optionally other keys, of the kind returned
> -            in a list of lists by
> +        :param dep: A dictionary with at least a "name" key, perhaps also
> +            "version" and "arch" keys, and optionally other keys, of the
> +            kind returned in a list of lists by
>              `debian.deb822.PkgRelation.parse_relations`.

"restrictions" is also a relevant key.

>          :param available: A dictionary mapping package names to a list of
>              the available versions of each package.
>          """
> +        dep_arch = dep.get("arch")
> +        if dep_arch is not None:
> +            seen_arch = False

"arch_match", maybe?

> +            for polarity, arch_wildcard in dep_arch:

"polarity" is called "enabled" in python-debian.

> +                if dpkg_architecture.match(self.arch_tag, arch_wildcard):
> +                    seen_arch = polarity
> +                    break

Is there any precedence to worry about? What does [i386 !linux-any] mean?

> +                elif not polarity:
> +                    # Any !other-architecture restriction implies that this
> +                    # architecture is allowed, unless it's specifically
> +                    # excluded later.

The "later" is confusing, as the set of architecture patterns is unordered.

> +                    seen_arch = True
> +            if not seen_arch:
> +                # This dependency "matches" in the sense that it's ignored
> +                # on this architecture.
> +                return True
> +        dep_restrictions = dep.get("restrictions")
> +        if dep_restrictions is not None:
> +            if all(
> +                any(restriction.enabled for restriction in restrlist)
> +                for restrlist in dep_restrictions):
> +                # This dependency "matches" in the sense that it's ignored
> +                # when no build profiles are enabled.
> +                return True
>          if dep["name"] not in available:
>              return False
> -        if dep.get("version") is None:
> +        dep_version = dep.get("version")
> +        if dep_version is None:
>              return True
> -        dep_version = dep.get("version")
>          operator_map = {
>              "<<": (lambda a, b: a < b),
>              "<=": (lambda a, b: a <= b),


-- 
https://code.launchpad.net/~cjwatson/launchpad-buildd/complex-build-deps/+merge/265650
Your team Launchpad code reviewers is subscribed to branch lp:launchpad-buildd.