17700 pkgdep can trace back when two packages deliver paths which satisfy a dependency
authorBrock Pytlik <brock.pytlik@oracle.com>
Wed, 12 Jan 2011 17:25:38 -0800
changeset 2191 da5a579210cc
parent 2190 07952cb65bba
child 2192 906bafc90a02
17700 pkgdep can trace back when two packages deliver paths which satisfy a dependency
src/modules/publish/dependencies.py
src/tests/cli/t_pkgdep_resolve.py
--- a/src/modules/publish/dependencies.py	Wed Jan 12 14:09:56 2011 -0800
+++ b/src/modules/publish/dependencies.py	Wed Jan 12 17:25:38 2011 -0800
@@ -21,7 +21,7 @@
 #
 
 #
-# Copyright (c) 2009, 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2009, 2011, Oracle and/or its affiliates. All rights reserved.
 #
 
 import copy
@@ -484,6 +484,8 @@
                 # Copy the variants so that marking the variants as satisified
                 # doesn't change the sate of 'path_vars.'
                 tmp_vars = copy.copy(path_vars)
+                # If tmp_vars has been satisfied, then this function should
+                # never have been called.
                 assert(tmp_vars.is_empty() or not tmp_vars.is_satisfied())
                 # Check each package which delivers a file with this path.
                 for pfmri, p_vc in files_dict[path]:
@@ -609,6 +611,10 @@
         multiple_path_errs = []
         multiple_path_pkgs = set()
         for p in make_paths(file_dep):
+                # If orig_dep_vars is satisfied, then this function should never
+                # have been called.
+                assert(orig_dep_vars.is_empty() or
+                    not orig_dep_vars.is_satisfied())
                 paths_info, path_deps = resolve_links(os.path.normpath(p),
                     files_dict, links, orig_dep_vars, file_dep.attrs.copy())
                 link_deps.extend(path_deps)
@@ -688,11 +694,18 @@
         'pkg_vars' is the variants against which the package was published."""
 
         file_dep, orig_dep_vars = split_off_variants(file_dep, pkg_vars)
+        # If the file dependency has already satisfied all its variants, then
+        # this function should never have been called.
+        assert(orig_dep_vars.is_empty() or not orig_dep_vars.is_satisfied())
         dep_vars = copy.copy(orig_dep_vars)
         # First try to resolve the dependency against the delivered files.
         res, dep_vars, errs = find_package_using_delivered_files(
                 files.delivered, links, file_dep, dep_vars, orig_dep_vars)
-        if (res and dep_vars.is_satisfied()) or not use_system:
+        # If dep_vars is satisfied then we found at least one solution.  It's
+        # possible that more than one solution was found, causing an error.
+        assert(not dep_vars.is_satisfied() or
+            (res or errs or dep_vars.is_empty()))
+        if ((res or errs) and dep_vars.is_satisfied()) or not use_system:
                 return res, dep_vars, errs
 
         # If the dependency isn't fully satisfied, resolve it against the
@@ -701,6 +714,9 @@
         # We only need to resolve for the variants not already satisfied
         # above.
         const_dep_vars = copy.copy(dep_vars)
+        # If dep_vars has been satisfied, then we should have exited the
+        # function above.
+        assert(const_dep_vars.is_empty() or not const_dep_vars.is_satisfied())
         inst_res, dep_vars, inst_errs = find_package_using_delivered_files(
             files.installed, links, file_dep, dep_vars, const_dep_vars)
         res.extend(inst_res)
--- a/src/tests/cli/t_pkgdep_resolve.py	Wed Jan 12 14:09:56 2011 -0800
+++ b/src/tests/cli/t_pkgdep_resolve.py	Wed Jan 12 17:25:38 2011 -0800
@@ -20,7 +20,7 @@
 # CDDL HEADER END
 #
 
-# Copyright (c) 2009, 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2009, 2011, Oracle and/or its affiliates. All rights reserved.
 
 import testutils
 if __name__ == "__main__":
@@ -227,6 +227,43 @@
 file NOHASH group=sys mode=0755 owner=root path=kernel/exec/elfexec reboot-needed=true variant.opensolaris.zone=global
 """
 
+        bug_17700_dep = """\
+set name=pkg.fmri [email protected],5.11-1
+set name=variant.opensolaris.zone value=global value=nonglobal
+set name=variant.arch value=sparc value=i386
+depend fmri=__TBD pkg.debug.depend.file=bignum pkg.debug.depend.path=kernel/misc/sparcv9 pkg.debug.depend.path=platform/sun4u/kernel/misc/sparcv9 pkg.debug.depend.path=platform/sun4v/kernel/misc/sparcv9 pkg.debug.depend.path=usr/kernel/misc/sparcv9 pkg.debug.depend.reason=kernel/drv/sparcv9/emlxs pkg.debug.depend.type=elf type=require variant.arch=sparc variant.opensolaris.zone=global
+"""
+        bug_17700_res1 = """\
+set name=pkg.fmri value=system/[email protected],5.11-1
+set name=variant.opensolaris.zone value=global value=nonglobal
+set name=variant.arch value=sparc value=i386
+file NOHASH group=bin mode=0755 owner=root path=kernel/misc/sparcv9/bignum variant.arch=sparc variant.opensolaris.zone=global
+"""
+
+        installed_17700_res1 = """\
+open system/[email protected],5.11-1
+add set name=variant.opensolaris.zone value=global value=nonglobal
+add set name=variant.arch value=sparc value=i386
+add file tmp/foo group=bin mode=0755 owner=root path=kernel/misc/sparcv9/bignum variant.arch=sparc variant.opensolaris.zone=global
+close
+"""
+
+        bug_17700_res2 = """\
+set name=pkg.fmri value=system/kernel/[email protected],5.11-1
+set name=variant.opensolaris.zone value=global value=nonglobal
+set name=variant.arch value=sparc value=i386
+file NOHASH group=bin mode=0755 owner=root path=platform/sun4u/kernel/misc/sparcv9/bignum variant.arch=sparc variant.opensolaris.zone=global
+"""
+
+        installed_17700_res2 = """\
+open system/kernel/[email protected],5.11-1
+add set name=variant.opensolaris.zone value=global value=nonglobal
+add set name=variant.arch value=sparc value=i386
+add file tmp/foo group=bin mode=0755 owner=root path=platform/sun4u/kernel/misc/sparcv9/bignum variant.arch=sparc variant.opensolaris.zone=global
+close
+"""
+
+
         misc_files = ["tmp/foo"]
 
         def setUp(self):
@@ -773,5 +810,34 @@
                         if str(fmri).startswith("pkg:/double_provides"):
                                 self.assertEqual(str(fmri.version.branch), "1")
 
+        def test_bug_17700(self):
+                """Test that when multiple packages satisfy a dependency under
+                the same combination of two variants, that an error is reported
+                instead of an assertion being raised."""
+
+                self.pkgsend_bulk(self.rurl, self.installed_17700_res1)
+                self.pkgsend_bulk(self.rurl, self.installed_17700_res2)
+                self.api_obj.refresh(immediate=True)
+                self._api_install(self.api_obj, ["system/kernel",
+                    "system/kernel/platform"])
+                dep_path = self.make_manifest(self.bug_17700_dep)
+                res1_path = self.make_manifest(self.bug_17700_res1)
+                res2_path = self.make_manifest(self.bug_17700_res2)
+
+                pkg_deps, errs = dependencies.resolve_deps([dep_path,
+                    res1_path, res2_path], self.api_obj)
+                for k in pkg_deps:
+                        self.assertEqual(len(pkg_deps[k]), 0,
+                            "Should not have resolved any dependencies instead "
+                            "%s had the following dependencies found:%s" %
+                            (k, "\n".join([str(s) for s in pkg_deps[k]])))
+                self.assertEqual(len(errs), 2, "Should have gotten exactly one "
+                    "error, instead got:%s" % "\n".join([str(s) for s in errs]))
+                for e in errs:
+                        self.assert_(isinstance(e,
+                            dependencies.MultiplePackagesPathError) or
+                            isinstance(e,
+                            dependencies.UnresolvedDependencyError))
+
 if __name__ == "__main__":
         unittest.main()