--- a/src/modules/client/image.py	Fri May 13 15:17:29 2011 +0100
+++ b/src/modules/client/image.py	Fri May 13 13:24:01 2011 -0700
@@ -1809,23 +1809,20 @@
                 if not self.cfg.allowed_to_move(pub):
                         raise apx.ModifyingSyspubException(_("Publisher '%s' "
                             "is a system publisher and cannot be moved.") % pub)
+
+                pubs = self.get_sorted_publishers()
                 relative = None
-                ranks = self.get_publisher_ranks()
-                rel_rank = None
-                for p in ranks:
-                        rel_pub = self.get_publisher(p)
-                        if not self.cfg.allowed_to_move(rel_pub):
-                                continue
-                        rank = ranks[p][0]
-                        if rel_rank is None or rank < rel_rank:
-                                rel_rank = rank
-                                relative = rel_pub
+                for p in pubs:
+                        # If we've gotten to the publisher we want to make
+                        # highest ranked, then there's nothing to do because
+                        # it's already as high as it can be.
+                        if p == pub:
+                                return
+                        if self.cfg.allowed_to_move(p):
+                                relative = p
+                                break
                 assert relative, "Expected %s to already be part of the " + \
                     "search order:%s" % (relative, ranks)
-                if relative == pub:
-                        # It's already first in the list of non-system
-                        # publishers, so nothing to do.
-                        return
                 self.cfg.change_publisher_search_order(pub.prefix,
                     relative.prefix, after=False)
 

--- a/src/tests/cli/t_pkg_publisher.py	Fri May 13 15:17:29 2011 +0100
+++ b/src/tests/cli/t_pkg_publisher.py	Fri May 13 13:24:01 2011 -0700
@@ -903,6 +903,25 @@
                 self.pkg("set-publisher -p %s" % self.durl2)
                 self.pkg("set-publisher -p %s" % self.durl3)
 
+        def test_bug_18283(self):
+                """Test that having a unset publisher with packages installed
+                doesn't break adding a publisher with the -P option."""
+
+                # Test what happens when another publisher is configured.
+                self.pkg("unset-publisher test2")
+                self.pkg("install foo")
+                self.pkg("unset-publisher test1")
+                self.pkg("set-publisher -P -p %s" % self.durl2)
+
+                # Test what happens when no publishers are configured
+                self.pkg("unset-publisher test2")
+                self.pkg("unset-publisher test3")
+                self.pkg("set-publisher -P -p %s" % self.durl2)
+
+                # set publishers to expected configuration
+                self.pkg("set-publisher -P -p %s" % self.durl1)
+                self.pkg("set-publisher -p %s" % self.durl3)
+
 
 class TestPkgPublisherCACerts(pkg5unittest.ManyDepotTestCase):
         # Tests in this suite use the read only data directory.