Merge ~xnox/germinate:master into germinate:master

[Dimitri John Ledkov]

[Dimitri John Ledkov (xnox)]

Mostly raising this as a WIP =) my first git merge proposal on launchpad. So wanted to see what it would look like =)

[Dimitri John Ledkov (xnox)]

Not sure what is easier to review, the full diff or commit-by-commit.

There are two things here:

* follow-build-depends feature tag, which is on by default. But specifying no-follow-build-depends, makes the code-paths stop trans-versing build-depends.

* built-using are now taken into consideration, and are added into dependencies as if they are a source package, which ended up added into the seeds via a binary build-dependency. I'm not sure if this is elegant or not, as there is no "reason" generated for them. Ideally it should hook something in like "pkg (Built-using bar)" or some such.

Emacs was complaining about invalid coding line, and I have refactored three Build-Depends* as an austerity measure.

[Dimitri John Ledkov (xnox)]

Sample output is here:
http://people.canonical.com/~xnox/germinate-output/

old -> "follow-build-depends"
new -> "no-follow-build-depends"

[Barry Warsaw (barry)]

Some minor comments inlined.

I don't know why Emacs complains about uppercase coding tags. That seems... unfortunate.

[Dimitri John Ledkov (xnox)]

> Some minor comments inlined.
>
> I don't know why Emacs complains about uppercase coding tags. That seems...
> unfortunate.

Removed unnecessory list() cast.

Added comment about Built-Using unpacking. I am unpacking the first pkg name, out of the possible multiple alternatives, for each specified Built-Using. See:
https://apt.alioth.debian.org/python-apt-doc/library/apt_pkg.html#apt_pkg.parse_depends

For the actual format -> list of alternatives, specified as triplets of (pkg, version, operator).

[Steve Langasek (vorlon)]

Test component-mismatches output using these changes and marking the Ubuntu seeds as no-follow-build-depends is here: http://people.canonical.com/~vorlon/components-mismatches-builddeps/component-mismatches.html

This looks like the correct behavior to me.

review: Approve

[Dimitri John Ledkov (xnox)]

+1 from me, please merge / deploy.

review: Approve

[Colin Watson (cjwatson)]

review: Needs Fixing

[Dimitri John Ledkov (xnox)]

Colin, I believe I have addressed all of your review comments in an extra commit I have just pushed. Things still work correctly as far as I can test, and invalid Built-Using packages have been spotted (golang removed, whilst not everything was rebuilt yet)

[Colin Watson (cjwatson)]

One more efficiency improvement, but I've fixed that as part of my merge. Thanks!

review: Approve

diff --git a/germinate/archive.py b/germinate/archive.py
index 64f6ddf..4ed3348 100644
--- a/germinate/archive.py
+++ b/germinate/archive.py
@@ -1,4 +1,4 @@
-# -*- coding: UTF-8 -*-
+# -*- coding: utf-8 -*-
 """Representations of archives for use by Germinate."""
 
 # Copyright (c) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012
diff --git a/germinate/defaults.py b/germinate/defaults.py
index 4222992..bbc894c 100644
--- a/germinate/defaults.py
+++ b/germinate/defaults.py
@@ -1,4 +1,4 @@
-# -*- coding: UTF-8 -*-
+# -*- coding: utf-8 -*-
 """Defaults for Germinate."""
 
 # Copyright (c) 2011 Canonical Ltd.
diff --git a/germinate/germinator.py b/germinate/germinator.py
index 18a120d..f2152ed 100644
--- a/germinate/germinator.py
+++ b/germinate/germinator.py
@@ -1,4 +1,4 @@
-# -*- coding: UTF-8 -*-
+# -*- coding: utf-8 -*-
 """Expand seeds into dependency-closed lists of packages."""
 
 # Copyright (c) 2004, 2005, 2006, 2007, 2008, 2009, 2011 Canonical Ltd.
@@ -39,6 +39,11 @@ __all__ = [
     'Germinator',
 ]
 
+BUILD_DEPENDS = (
+    "Build-Depends",
+    "Build-Depends-Indep",
+    "Build-Depends-Arch",
+)
 
 _logger = logging.getLogger(__name__)
 
@@ -445,7 +450,7 @@ class Germinator(object):
 
         self._packages[pkg]["Essential"] = section.get("Essential", "")
 
-        for field in "Pre-Depends", "Depends", "Recommends":
+        for field in "Pre-Depends", "Depends", "Recommends", "Built-Using":
             value = section.get(field, "")
             self._packages[pkg][field] = self._parse_depends(value)
 
@@ -569,8 +574,7 @@ class Germinator(object):
             _ensure_unicode(section.get("Maintainer", ""))
         self._sources[src]["Version"] = ver
 
-        for field in (
-                "Build-Depends", "Build-Depends-Indep", "Build-Depends-Arch"):
+        for field in BUILD_DEPENDS:
             value = section.get(field, "")
             self._sources[src][field] = self._parse_src_depends(value)
 
@@ -1168,6 +1172,18 @@ class Germinator(object):
                 return False
         return "follow-recommends" in structure.features
 
+    def _follow_build_depends(self, structure, seed=None):
+        """
+        Test whether we should follow Build-Depends for this seed.
+        Defaults to True, if not explicitly specified.
+        """
+        if seed is not None:
+            if "follow-build-depends" in seed._features:
+                return True
+            if "no-follow-build-depends" in seed._features:
+                return False
+        return "no-follow-build-depends" not in structure.features
+
     def _add_reverse(self, pkg, field, rdep):
         """Add a reverse dependency entry."""
         if "Reverse-Depends" not in self._packages[pkg]:
@@ -1192,9 +1208,10 @@ class Germinator(object):
                             self._add_reverse(depname, field, pkg)
 
         for src in output._all_srcs:
-            for field in (
-                    "Build-Depends", "Build-Depends-Indep",
-                    "Build-Depends-Arch"):
+            fields = ()
+            if self._follow_build_depends(structure):
+                fields = BUILD_DEPENDS
+            for field in fields:
                 for deplist in self._sources[src][field]:
                     for dep in deplist:
                         depname = dep[0].split(":", 1)[0]
@@ -1210,8 +1227,7 @@ class Germinator(object):
             if (self._follow_recommends(structure) or
                 self._packages[pkg]["Section"] == "metapackages"):
                 fields.append("Recommends")
-            fields.extend([
-                "Build-Depends", "Build-Depends-Indep", "Build-Depends-Arch"])
+            fields.extend(BUILD_DEPENDS)
             for field in fields:
                 if field not in self._packages[pkg]["Reverse-Depends"]:
                     continue
@@ -1537,40 +1553,53 @@ class Germinator(object):
                                       recommends=True)
 
         src = self._packages[pkg]["Source"]
-        if src not in self._sources:
-            _logger.error("Missing source package: %s (for %s)", src, pkg)
-            return
 
+        # Built-Using field is in a form of apt_pkg.parse_depends For
+        # common-case "pkg (= 1)" it returns
+        #     [[('pkg', '1', '=')]]
+        # We thus unpack the first listed alternative pkg-name, for
+        # each built-using source.
+        built_using = [i[0][0] for i in self._packages[pkg]["Built-Using"]]
+        pkg_srcs = []
+
+        # Create set of all sources needed for pkg: Source + Built-Using
+        for pkg_src in built_using + [src]:
+            if pkg_src in self._sources and pkg_src not in pkg_srcs:
+                pkg_srcs.append(pkg_src)
+            else:
+                _logger.error("Missing source package: %s (for %s)", pkg_src, pkg)
+
+        # Filter/exclude sources already part of an inner seed
         if second_class:
-            for innerseed in self._inner_seeds(seed):
-                if src in innerseed._build_srcs:
-                    return
+            excluded_srcs = "_build_srcs"
         else:
-            for innerseed in self._inner_seeds(seed):
-                if src in innerseed._not_build_srcs:
-                    return
+            excluded_srcs = "_not_build_srcs"
 
-        if build_tree:
-            seed._build_sourcepkgs.add(src)
-            if src in output._blacklist:
-                seed._blacklisted.add(src)
+        for innerseed in self._inner_seeds(seed):
+            for excluded_src in getattr(innerseed, excluded_srcs):
+                if excluded_src in pkg_srcs:
+                    pkg_srcs.remove(excluded_src)
+
+        # Use build_tree flag for src
+        # Treat all Built-Using as if it's part of build_tree
+        for pkg_src in pkg_srcs:
+            if build_tree or pkg_src in built_using:
+                seed._build_sourcepkgs.add(pkg_src)
+                if pkg_src in output._blacklist:
+                    seed._blacklisted.add(pkg_src)
+            else:
+                seed._not_build_srcs.add(pkg_src)
+                seed._sourcepkgs.add(pkg_src)
 
-        else:
-            seed._not_build_srcs.add(src)
-            seed._sourcepkgs.add(src)
+            output._all_srcs.add(pkg_src)
+            seed._build_srcs.add(pkg_src)
 
-        output._all_srcs.add(src)
-        seed._build_srcs.add(src)
+            if self._follow_build_depends(seed.structure, seed):
+                for build_depends in BUILD_DEPENDS:
+                    self._add_dependency_tree(seed, pkg,
+                                              self._sources[pkg_src][build_depends],
+                                              build_depend=True)
 
-        self._add_dependency_tree(seed, pkg,
-                                  self._sources[src]["Build-Depends"],
-                                  build_depend=True)
-        self._add_dependency_tree(seed, pkg,
-                                  self._sources[src]["Build-Depends-Indep"],
-                                  build_depend=True)
-        self._add_dependency_tree(seed, pkg,
-                                  self._sources[src]["Build-Depends-Arch"],
-                                  build_depend=True)
 
     def _rescue_includes(self, structure, seedname, rescue_seedname,
                          build_tree):
@@ -1962,9 +1991,7 @@ class Germinator(object):
         if "Reverse-Depends" not in self._packages[pkg]:
             return
 
-        for field in ("Pre-Depends", "Depends", "Recommends",
-                      "Build-Depends", "Build-Depends-Indep",
-                      "Build-Depends-Arch"):
+        for field in ("Pre-Depends", "Depends", "Recommends") + BUILD_DEPENDS:
             if field not in self._packages[pkg]["Reverse-Depends"]:
                 continue
 
diff --git a/germinate/log.py b/germinate/log.py
index 3e5be54..dc7c0f2 100644
--- a/germinate/log.py
+++ b/germinate/log.py
@@ -1,4 +1,4 @@
-# -*- coding: UTF-8 -*-
+# -*- coding: utf-8 -*-
 """Custom logging for Germinate."""
 
 # Copyright (c) 2011, 2012 Canonical Ltd.
diff --git a/germinate/scripts/germinate_main.py b/germinate/scripts/germinate_main.py
index 34a930c..e27f463 100644
--- a/germinate/scripts/germinate_main.py
+++ b/germinate/scripts/germinate_main.py
@@ -1,4 +1,4 @@
-# -*- coding: UTF-8 -*-
+# -*- coding: utf-8 -*-
 """Expand dependencies in a list of seed packages."""
 
 # Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012
diff --git a/germinate/scripts/germinate_pkg_diff.py b/germinate/scripts/germinate_pkg_diff.py
index c99b88e..15736ea 100644
--- a/germinate/scripts/germinate_pkg_diff.py
+++ b/germinate/scripts/germinate_pkg_diff.py
@@ -1,4 +1,4 @@
-# -*- coding: UTF-8 -*-
+# -*- coding: utf-8 -*-
 
 # Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012
 #               Canonical Ltd.
diff --git a/germinate/scripts/germinate_update_metapackage.py b/germinate/scripts/germinate_update_metapackage.py
index 26f4728..08a697b 100644
--- a/germinate/scripts/germinate_update_metapackage.py
+++ b/germinate/scripts/germinate_update_metapackage.py
@@ -1,4 +1,4 @@
-# -*- coding: UTF-8 -*-
+# -*- coding: utf-8 -*-
 
 # Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Canonical Ltd.
 # Copyright (C) 2006 Gustavo Franco
diff --git a/germinate/seeds.py b/germinate/seeds.py
index e149ccb..aecec48 100644
--- a/germinate/seeds.py
+++ b/germinate/seeds.py
@@ -1,4 +1,4 @@
-# -*- coding: UTF-8 -*-
+# -*- coding: utf-8 -*-
 """Fetch seeds from a URL collection or from a VCS."""
 
 # Copyright (c) 2004, 2005, 2006, 2008, 2009, 2011, 2012 Canonical Ltd.
diff --git a/germinate/tests/test_germinator.py b/germinate/tests/test_germinator.py
index 90ff8ee..79956dc 100644
--- a/germinate/tests/test_germinator.py
+++ b/germinate/tests/test_germinator.py
@@ -169,6 +169,7 @@ class TestGerminator(TestCase):
             "Maintainer": u("Test Person <[e-mail]>"),
             "Essential": "",
             "Pre-Depends": [],
+            "Built-Using": [],
             "Depends": [[("hello-dependency", "", "")]],
             "Recommends": [],
             "Size": 0,
@@ -186,6 +187,7 @@ class TestGerminator(TestCase):
             "Maintainer": u(""),
             "Essential": "",
             "Pre-Depends": [],
+            "Built-Using": [],
             "Depends": [],
             "Recommends": [],
             "Size": 0,