diff --git a/src/modules/catalog.py b/src/modules/catalog.py index f8795689c..79650840d 100644 --- a/src/modules/catalog.py +++ b/src/modules/catalog.py @@ -3354,14 +3354,16 @@ def get_matching_fmris(self, patterns): for k, l in six.iteritems(d): proposed_dict.setdefault(k, []).extend(l) - # construct references so that we can know which pattern - # generated which fmris... - references = dict([ - (f, p) - for p in ret.keys() - for flist in ret[p].values() - for f in flist - ]) + # construct references so that we can know which patterns + # generated which fmris... An FMRI can be matched by more + # than one pattern, so each entry is a list; callers (and + # this function's contract) rely on that. + references = {} + for p in ret.keys(): + for flist in ret[p].values(): + for f in flist: + references.setdefault(f, + []).append(p) return proposed_dict, references, unmatched diff --git a/src/pull.py b/src/pull.py index 8e4a7332b..97c168c0e 100755 --- a/src/pull.py +++ b/src/pull.py @@ -608,7 +608,11 @@ def get_matches(src_pub, tracker, xport, pargs, any_unmatched, any_matched, # Track anything that failed to match. any_unmatched.extend(unmatched) - any_matched.extend(set(p for p in refs.values())) + any_matched.extend(set( + p + for plist in refs.values() + for p in plist + )) matches = list(set(f for m in matches.values() for f in m)) else: matches = [f for f in src_cat.fmris()] diff --git a/src/tests/api/t_catalog.py b/src/tests/api/t_catalog.py index 3cb71a807..d0a3f6d21 100644 --- a/src/tests/api/t_catalog.py +++ b/src/tests/api/t_catalog.py @@ -776,6 +776,18 @@ def test_06_operations(self): ["xyzzy", "base"]) self.assertEqual(set(["xyzzy"]), unmatched) + # Verify that references maps each matching FMRI to the list + # of every pattern that matched it. Callers such as + # pkg.server.repository.Repository.get_matching_fmris merge + # these values together, so they have to be lists and not + # bare strings. + pdict, references, unmatched = cat.get_matching_fmris( + ["ba*", "base"]) + self.assertEqual(set(), unmatched) + self.assertEqual([p1_fmri], list(references.keys())) + self.assertEqual(["ba*", "base"], + sorted(references[p1_fmri])) + # Next, verify that removal of an FMRI not in the catalog will # raise the expected exception. Do this by removing an FMRI # and then attempting to remove it again. diff --git a/src/tests/cli/t_pkgrepo.py b/src/tests/cli/t_pkgrepo.py index 52655bfd1..bb8b008a4 100644 --- a/src/tests/cli/t_pkgrepo.py +++ b/src/tests/cli/t_pkgrepo.py @@ -1591,6 +1591,46 @@ def test_09_remove_packages(self): shutil.rmtree(src_repo) shutil.rmtree(dest_repo) + def test_09a_remove_duplicate_fmri(self): + """Verify that remove works when the same FMRI is reachable + from more than one repository store.""" + + repo_path = os.path.join(self.test_root, "dup-fmri-repo") + self.create_repo(repo_path) + self.pkgrepo("set -s {0} publisher/prefix=test".format( + repo_path)) + published = self.pkgsend_bulk(repo_path, self.zoo10) + + # Note where the manifest lives while 'test' is still the only + # store, so that the check made below is unambiguous. + mpath = self.get_repo(repo_path).manifest(published[0]) + + # Duplicate the publisher's storage under a second publisher + # directory. The copied catalog still describes its packages + # as belonging to 'test', so the very same FMRI is now + # returned by two different repository stores. This mirrors + # what a repository looks like after a rebuild has indexed + # manifests that ended up below the wrong publisher. + pub_root = os.path.join(repo_path, "publisher") + shutil.copytree(os.path.join(pub_root, "test"), + os.path.join(pub_root, "test2")) + + # Merging the per-store pattern references used to fail here + # with 'AttributeError: str object has no attribute extend'. + repo = self.get_repo(repo_path) + matching, refs = repo.get_matching_fmris(["zoo"]) + self.assertEqual(["zoo"], list(matching.keys())) + for pats in refs.values(): + self.assertEqual(["zoo"], sorted(set(pats))) + + # ...which meant remove could not be used on such a + # repository at all. + self.pkgrepo("remove -s {0} zoo".format(repo_path)) + self.assertTrue(not os.path.exists(mpath)) + + # Cleanup. + shutil.rmtree(repo_path) + def test_10_list(self): """Verify the list subcommand works as expected."""