Only apply 'not already met' blockers to necessary slots

Fixes: ticket:1086

7 files changed, 121 insertions, 28 deletions

diff --git a/paludis/resolver/decider.cc b/paludis/resolver/decider.cc
index e1ab44134..1c0d17c45 100644
--- a/paludis/resolver/decider.cc
+++ b/paludis/resolver/decider.cc
@@ -634,7 +634,7 @@ Decider::_make_constraints_from_dependency(
 
 const std::shared_ptr<ConstraintSequence>
 Decider::_make_constraints_from_blocker(
-        const std::shared_ptr<const Resolution> &,
+        const std::shared_ptr<const Resolution> & resolution,
         const BlockDepSpec & spec,
         const std::shared_ptr<const Reason> & reason) const
 {
@@ -650,11 +650,11 @@ Decider::_make_constraints_from_blocker(
             break;
 
         case bk_manual:
-            force_unable = ! _already_met(spec, maybe_from_package_id_from_reason(reason));
+            force_unable = ! _block_dep_spec_already_met(spec, maybe_from_package_id_from_reason(reason), resolution->resolvent());
             break;
 
         case bk_upgrade_blocked_before:
-            nothing_is_fine_too = ! _already_met(spec, maybe_from_package_id_from_reason(reason));
+            nothing_is_fine_too = ! _block_dep_spec_already_met(spec, maybe_from_package_id_from_reason(reason), resolution->resolvent());
             break;
 
         case last_bk:
@@ -1189,8 +1189,9 @@ Decider::_add_dependencies_if_necessary(
                 break;
         }
 
-        const std::shared_ptr<DependencyReason> reason(std::make_shared<DependencyReason>(
-                    package_id, changed_choices, our_resolution->resolvent(), *s, _already_met(s->spec(), package_id)));
+        /* don't have an 'already met' initially, since already met varies between slots */
+        const std::shared_ptr<DependencyReason> nearly_reason(std::make_shared<DependencyReason>(
+                    package_id, changed_choices, our_resolution->resolvent(), *s, indeterminate));
 
         /* empty resolvents is always ok for blockers, since blocking on things
          * that don't exist is fine */
@@ -1198,16 +1199,22 @@ Decider::_add_dependencies_if_necessary(
         std::shared_ptr<const Resolvents> resolvents;
 
         if (s->spec().if_package())
-            std::tie(resolvents, empty_is_ok) = _get_resolvents_for(*s->spec().if_package(), reason);
+            std::tie(resolvents, empty_is_ok) = _get_resolvents_for(*s->spec().if_package(), nearly_reason);
         else
-            resolvents = _get_resolvents_for_blocker(*s->spec().if_block(), reason);
+            resolvents = _get_resolvents_for_blocker(*s->spec().if_block(), nearly_reason);
 
         if ((! empty_is_ok) && resolvents->empty())
-            resolvents = _get_error_resolvents_for(*s->spec().if_package(), reason);
+            resolvents = _get_error_resolvents_for(*s->spec().if_package(), nearly_reason);
 
         for (Resolvents::ConstIterator r(resolvents->begin()), r_end(resolvents->end()) ;
                 r != r_end ; ++r)
         {
+            /* now we can find out per-resolvent whether we're really already met */
+            const std::shared_ptr<DependencyReason> reason(std::make_shared<DependencyReason>(
+                        package_id, changed_choices, our_resolution->resolvent(), *s,
+                        s->spec().if_block() ? _block_dep_spec_already_met(*s->spec().if_block(), package_id, *r) :
+                        _package_dep_spec_already_met(*s->spec().if_package(), package_id)));
+
             const std::shared_ptr<Resolution> dep_resolution(_resolution_for_resolvent(*r, true));
             const std::shared_ptr<ConstraintSequence> constraints(_make_constraints_from_dependency(our_resolution, *s, reason, interest));
 
@@ -1353,7 +1360,7 @@ Decider::find_any_score(
     if (! is_block)
     {
         const std::shared_ptr<DependencyReason> reason(std::make_shared<DependencyReason>(
-                    our_id, make_null_shared_ptr(), our_resolution->resolvent(), dep, _already_met(dep.spec(), our_id)));
+                    our_id, make_null_shared_ptr(), our_resolution->resolvent(), dep, _package_dep_spec_already_met(*dep.spec().if_package(), our_id)));
         const std::shared_ptr<const Resolvents> resolvents(_get_resolvents_for(spec, reason).first);
 
         /* next: will already be installing */
@@ -2137,18 +2144,15 @@ Decider::resolve()
 }
 
 bool
-Decider::_already_met(const PackageOrBlockDepSpec & spec, const std::shared_ptr<const PackageID> & from_id) const
+Decider::_package_dep_spec_already_met(const PackageDepSpec & spec, const std::shared_ptr<const PackageID> & from_id) const
 {
     const std::shared_ptr<const PackageIDSequence> installed_ids((*_imp->env)[selection::AllVersionsUnsorted(
-                generator::Matches(spec.if_package() ?  *spec.if_package() : spec.if_block()->blocking(), from_id, { }) |
+                generator::Matches(spec, from_id, { }) |
                 filter::InstalledAtRoot(_imp->env->system_root_key()->value()))]);
     if (installed_ids->empty())
-        return bool(spec.if_block());
+        return false;
     else
     {
-        if (spec.if_block())
-            return false;
-
         if (installed_ids->end() == std::find_if(installed_ids->begin(), installed_ids->end(),
                     std::bind(&Decider::_can_use, this, std::placeholders::_1)))
             return false;
@@ -2158,6 +2162,17 @@ Decider::_already_met(const PackageOrBlockDepSpec & spec, const std::shared_ptr<
 }
 
 bool
+Decider::_block_dep_spec_already_met(const BlockDepSpec & spec, const std::shared_ptr<const PackageID> & from_id,
+        const Resolvent & resolvent) const
+{
+    const std::shared_ptr<const PackageIDSequence> installed_ids((*_imp->env)[selection::SomeArbitraryVersion(
+                generator::Matches(spec.blocking(), from_id, { }) |
+                make_slot_filter(resolvent) |
+                filter::InstalledAtRoot(_imp->env->system_root_key()->value()))]);
+    return installed_ids->empty();
+}
+
+bool
 Decider::_can_use(
         const std::shared_ptr<const PackageID> & id) const
 {
diff --git a/paludis/resolver/decider.hh b/paludis/resolver/decider.hh
index 3b2bd4cc0..d7d9e09f3 100644
--- a/paludis/resolver/decider.hh
+++ b/paludis/resolver/decider.hh
@@ -248,10 +248,15 @@ namespace paludis
                         const std::shared_ptr<const PackageID> &,
                         const bool existing) const;
 
-                bool _already_met(
-                        const PackageOrBlockDepSpec &,
+                bool _package_dep_spec_already_met(
+                        const PackageDepSpec &,
                         const std::shared_ptr<const PackageID> &) const PALUDIS_ATTRIBUTE((warn_unused_result));
 
+                bool _block_dep_spec_already_met(
+                        const BlockDepSpec &,
+                        const std::shared_ptr<const PackageID> &,
+                        const Resolvent &) const PALUDIS_ATTRIBUTE((warn_unused_result));
+
                 bool _installed_but_allowed_to_remove(
                         const std::shared_ptr<const Resolution> &) const PALUDIS_ATTRIBUTE((warn_unused_result));
 
diff --git a/paludis/resolver/orderer.cc b/paludis/resolver/orderer.cc
index 0468fc18e..94e98f3e9 100644
--- a/paludis/resolver/orderer.cc
+++ b/paludis/resolver/orderer.cc
@@ -307,9 +307,9 @@ namespace
                             make_named_values<NAGEdgeProperties>(
                                 n::always() = false,
                                 n::build() = classifier.includes_buildish,
-                                n::build_all_met() = r.already_met() || ! classifier.includes_buildish,
+                                n::build_all_met() = r.already_met().is_true() || ! classifier.includes_buildish,
                                 n::run() = classifier.includes_non_post_runish,
-                                n::run_all_met() = r.already_met() || ! classifier.includes_non_post_runish
+                                n::run_all_met() = r.already_met().is_true() || ! classifier.includes_non_post_runish
                                 ));
                 }
                 else
diff --git a/paludis/resolver/reason.cc b/paludis/resolver/reason.cc
index 6bd3eed94..50977f458 100644
--- a/paludis/resolver/reason.cc
+++ b/paludis/resolver/reason.cc
@@ -81,11 +81,11 @@ namespace paludis
         const std::shared_ptr<const ChangedChoices> changed_choices;
         const Resolvent from_resolvent;
         const SanitisedDependency dep;
-        const bool already_met;
+        const Tribool already_met;
 
         Imp(const std::shared_ptr<const PackageID> & i,
                 const std::shared_ptr<const ChangedChoices> & c,
-                const Resolvent & r, const SanitisedDependency & d, const bool a) :
+                const Resolvent & r, const SanitisedDependency & d, const Tribool a) :
             from_id(i),
             changed_choices(c),
             from_resolvent(r),
@@ -100,7 +100,7 @@ DependencyReason::DependencyReason(const std::shared_ptr<const PackageID> & i,
         const std::shared_ptr<const ChangedChoices> & c,
         const Resolvent & r,
         const SanitisedDependency & d,
-        const bool a) :
+        const Tribool a) :
     _imp(i, c, r, d, a)
 {
 }
@@ -133,7 +133,7 @@ DependencyReason::sanitised_dependency() const
     return _imp->dep;
 }
 
-bool
+Tribool
 DependencyReason::already_met() const
 {
     return _imp->already_met;
@@ -143,7 +143,7 @@ void
 DependencyReason::serialise(Serialiser & s) const
 {
     s.object("DependencyReason")
-        .member(SerialiserFlags<>(), "already_met", already_met())
+        .member(SerialiserFlags<>(), "already_met", std::string(already_met().is_true() ? "true" : already_met().is_false() ? "false" : "indeterminate"))
         .member(SerialiserFlags<serialise::might_be_null>(), "from_id", from_id())
         .member(SerialiserFlags<serialise::might_be_null>(), "from_id_changed_choices", from_id_changed_choices())
         .member(SerialiserFlags<>(), "from_resolvent", from_resolvent())
@@ -400,6 +400,20 @@ ViaBinaryReason::serialise(Serialiser & s) const
         ;
 }
 
+namespace
+{
+    Tribool destringify_tribool(const std::string & s)
+    {
+        if (s == "true")
+            return true;
+        if (s == "false")
+            return false;
+        if (s == "indeterminate")
+            return indeterminate;
+        throw InternalError(PALUDIS_HERE, "bad tribool " + s);
+    }
+}
+
 const std::shared_ptr<Reason>
 Reason::deserialise(Deserialisation & d)
 {
@@ -433,7 +447,7 @@ Reason::deserialise(Deserialisation & d)
                     v.member<std::shared_ptr<const ChangedChoices> >("from_id_changed_choices"),
                     v.member<Resolvent>("from_resolvent"),
                     SanitisedDependency::deserialise(*v.find_remove_member("sanitised_dependency"), from_id),
-                    v.member<bool>("already_met")
+                    destringify_tribool(v.member<std::string>("already_met"))
                 );
     }
     else if (d.class_name() == "DependentReason")
diff --git a/paludis/resolver/reason.hh b/paludis/resolver/reason.hh
index 7a5364433..322c198da 100644
--- a/paludis/resolver/reason.hh
+++ b/paludis/resolver/reason.hh
@@ -27,6 +27,7 @@
 #include <paludis/util/pimp.hh>
 #include <paludis/util/simple_visitor.hh>
 #include <paludis/util/type_list.hh>
+#include <paludis/util/tribool.hh>
 #include <paludis/name-fwd.hh>
 #include <paludis/package_id-fwd.hh>
 #include <paludis/serialise-fwd.hh>
@@ -80,7 +81,7 @@ namespace paludis
                         const std::shared_ptr<const ChangedChoices> &,
                         const Resolvent &,
                         const SanitisedDependency & s,
-                        const bool already_met);
+                        const Tribool already_met);
 
                 ~DependencyReason();
 
@@ -88,7 +89,7 @@ namespace paludis
                 const std::shared_ptr<const ChangedChoices> from_id_changed_choices() const;
                 const Resolvent from_resolvent() const;
                 const SanitisedDependency & sanitised_dependency() const;
-                bool already_met() const PALUDIS_ATTRIBUTE((warn_unused_result));
+                Tribool already_met() const PALUDIS_ATTRIBUTE((warn_unused_result));
 
                 virtual void serialise(Serialiser &) const;
         };
diff --git a/paludis/resolver/resolver_TEST_blockers.cc b/paludis/resolver/resolver_TEST_blockers.cc
index 3033b7a60..1d9c723af 100644
--- a/paludis/resolver/resolver_TEST_blockers.cc
+++ b/paludis/resolver/resolver_TEST_blockers.cc
@@ -1,7 +1,7 @@
 /* vim: set sw=4 sts=4 et foldmethod=syntax : */
 
 /*
- * Copyright (c) 2009, 2010 Ciaran McCreesh
+ * Copyright (c) 2009, 2010, 2011 Ciaran McCreesh
  *
  * This file is part of the Paludis package manager. Paludis is free software;
  * you can redistribute it and/or modify it under the terms of the GNU General
@@ -542,5 +542,37 @@ namespace test_cases
                     );
         }
     } test_manual;
+
+    struct UpgradeOtherSlotFirst : ResolverBlockersTestCase
+    {
+        UpgradeOtherSlotFirst() :
+            ResolverBlockersTestCase("other-slot-first")
+        {
+            install("other-slot-first", "dep", "1")->set_slot(SlotName("1"));
+        }
+
+        void run()
+        {
+            std::shared_ptr<const Resolved> resolved(get_resolved("other-slot-first/target"));
+
+            check_resolved(resolved,
+                    n::taken_change_or_remove_decisions() = make_shared_copy(DecisionChecks()
+                        .change_slot(QualifiedPackageName("other-slot-first/dep"), SlotName("1"))
+                        .change_slot(QualifiedPackageName("other-slot-first/dep"), SlotName("2"))
+                        .change(QualifiedPackageName("other-slot-first/target"))
+                        .finished()),
+                    n::taken_unable_to_make_decisions() = make_shared_copy(DecisionChecks()
+                        .finished()),
+                    n::taken_unconfirmed_decisions() = make_shared_copy(DecisionChecks()
+                        .finished()),
+                    n::taken_unorderable_decisions() = make_shared_copy(DecisionChecks()
+                        .finished()),
+                    n::untaken_change_or_remove_decisions() = make_shared_copy(DecisionChecks()
+                        .finished()),
+                    n::untaken_unable_to_make_decisions() = make_shared_copy(DecisionChecks()
+                        .finished())
+                    );
+        }
+    } test_upgrade_other_slot_first;
 }
 
diff --git a/paludis/resolver/resolver_TEST_blockers_setup.sh b/paludis/resolver/resolver_TEST_blockers_setup.sh
index c15901168..47e8c3022 100755
--- a/paludis/resolver/resolver_TEST_blockers_setup.sh
+++ b/paludis/resolver/resolver_TEST_blockers_setup.sh
@@ -293,5 +293,31 @@ SLOT="0"
 DEPENDENCIES=""
 END
 
+# other-slot-first
+echo 'other-slot-first' >> metadata/categories.conf
+
+mkdir -p 'packages/other-slot-first/target'
+cat <<END > packages/other-slot-first/target/target-1.exheres-0
+SUMMARY="target"
+PLATFORMS="test"
+SLOT="0"
+DEPENDENCIES="other-slot-first/dep:1 other-slot-first/dep:2"
+END
+
+mkdir -p 'packages/other-slot-first/dep'
+cat <<END > packages/other-slot-first/dep/dep-1.1.exheres-0
+SUMMARY="target"
+PLATFORMS="test"
+SLOT="1"
+DEPENDENCIES=""
+END
+
+cat <<END > packages/other-slot-first/dep/dep-2.exheres-0
+SUMMARY="target"
+PLATFORMS="test"
+SLOT="2"
+DEPENDENCIES="!other-slot-first/dep[<1.1]"
+END
+
 cd ..