Using PackageSpec struct in PackageArgs

diff --git a/src/PackageArgs.cc b/src/PackageArgs.cc
index b51e3e9..9d4dfc1 100644 (file)
--- a/src/PackageArgs.cc
+++ b/src/PackageArgs.cc
@@ -107,11 +107,12 @@ void PackageArgs::preprocess(const vector<string> & args)
 
 static bool
 remove_duplicate(
-    PackageArgs::CapRepoPairSet & set, const PackageArgs::CapRepoPair & obj)
+    PackageArgs::PackageSpecSet & set, const PackageSpec & obj)
 {
-  PackageArgs::CapRepoPairSet::iterator match = set.find(obj);
+  PackageArgs::PackageSpecSet::iterator match = set.find(obj);
   if (match != set.end())
   {
+    DBG << "found dupe: '" << match->orig_str << "' : " << obj.orig_str << endl;
     set.erase(match);
     return true;
   }
@@ -129,6 +130,8 @@ void PackageArgs::argsToCaps(const zypp::ResKind & kind)
     arg = *it;
     repo.clear();
 
+    PackageSpec spec;
+    spec.orig_str = arg;
 
     // For given arguments:
     //    +vim
@@ -198,7 +201,7 @@ void PackageArgs::argsToCaps(const zypp::ResKind & kind)
     }
 
     // parse the rest of the string as standard zypp package specifier
-    Capability parsedcap = Capability::guessPackageSpec(arg);
+    Capability parsedcap = Capability::guessPackageSpec(arg, spec.modified);
 
     // set the right kind (bnc #580571)
     // prefer those specified in args
@@ -236,13 +239,15 @@ void PackageArgs::argsToCaps(const zypp::ResKind & kind)
     MIL << "; repo '" << repo << "'" << endl;
 
     // Store, but avoid duplicates in do and dont sets.
+    spec.parsed_cap = parsedcap;
+    spec.repo_alias = repo;
     if (dont)
     {
-      if (!remove_duplicate(_do_caps, CapRepoPair(parsedcap, repo)))
-        _dont_caps.insert(CapRepoPair(parsedcap, repo));
+      if (!remove_duplicate(_dos, spec))
+        _donts.insert(spec);
     }
-    else if (!remove_duplicate(_dont_caps, CapRepoPair(parsedcap, repo)))
-      _do_caps.insert(CapRepoPair(parsedcap, repo));
+    else if (!remove_duplicate(_donts, spec))
+      _dos.insert(spec);
   }
 }
 

diff --git a/src/PackageArgs.h b/src/PackageArgs.h
index 3a9be64..1354abc 100644 (file)
--- a/src/PackageArgs.h
+++ b/src/PackageArgs.h
@@ -21,12 +21,33 @@
 
 class Zypper;
 
+struct PackageSpec
+{
+  PackageSpec() : modified(false) {}
+
+  std::string orig_str;
+  zypp::Capability parsed_cap;
+  std::string repo_alias;
+  bool modified;
+};
+
+/**
+ * Only comparing parsed capabilities. Even though repository may be different,
+ * if capability is the same, we must rule out one of them.
+ */
+struct PackageSpecCompare
+{
+  bool operator()(const PackageSpec & lhs, const PackageSpec & rhs) const
+  {
+    return lhs.parsed_cap < rhs.parsed_cap;
+  };
+};
+
 class PackageArgs
 {
 public:
   typedef std::set<std::string> StringSet;
-  typedef std::pair<zypp::Capability, std::string> CapRepoPair;
-  typedef std::set<CapRepoPair> CapRepoPairSet;
+  typedef std::set<PackageSpec, PackageSpecCompare> PackageSpecSet;
 
   struct Options
   {
@@ -60,14 +81,14 @@ public:
   { return _args; }
   /** Capabilities we want to install/upgrade and don't want to remove, plus
    * associated requested repo */
-  const CapRepoPairSet & doCaps() const
-  { return _do_caps; }
+  const PackageSpecSet & dos() const
+  { return _dos; }
   /** Capabilities we don't want to install/upgrade or want to remove. */
-  const CapRepoPairSet & dontCaps() const
-  { return _dont_caps; }
+  const PackageSpecSet & donts() const
+  { return _donts; }
 
   bool empty() const
-  { return doCaps().empty() && dontCaps().empty(); }
+  { return dos().empty() && donts().empty(); }
 
 protected:
   /** join arguments at comparison operators ('=', '>=', and the like) */
@@ -80,8 +101,8 @@ private:
   Zypper & zypper;
   Options _opts;
   StringSet _args;
-  CapRepoPairSet _do_caps;
-  CapRepoPairSet _dont_caps;
+  PackageSpecSet _dos;
+  PackageSpecSet _donts;
 };
 
 

diff --git a/src/SolverRequester.cc b/src/SolverRequester.cc
index 67ea6c0..2743992 100644 (file)
--- a/src/SolverRequester.cc
+++ b/src/SolverRequester.cc
@@ -89,16 +89,16 @@ void SolverRequester::installRemove(const PackageArgs & args)
   if (args.empty())
     return;
 
-  for_(it, args.doCaps().begin(), args.doCaps().end())
-    install(it->first, it->second);
+  for_(it, args.dos().begin(), args.dos().end())
+    install(*it);
 
   // TODO solve before processing dontCaps? so that we could unset any
   // dontCaps that are already set for installation. This would allow
   //   $ zypper install pattern:lamp_sever -someunwantedpackage
   // and similar nice things.
 
-  for_(it, args.dontCaps().begin(), args.dontCaps().end())
-    remove(it->first);
+  for_(it, args.donts().begin(), args.donts().end())
+    remove(*it);
 }
 
 // ----------------------------------------------------------------------------
@@ -127,9 +127,9 @@ void SolverRequester::installRemove(const PackageArgs & args)
  * - maybe a check for glob wildcards in cap name would make sense before trying
  *   by-cap
  */
-void SolverRequester::install(const Capability & cap, const string & repoalias)
+void SolverRequester::install(const PackageSpec & pkg)
 {
-  sat::Solvable::SplitIdent splid(cap.detail().name());
+  sat::Solvable::SplitIdent splid(pkg.parsed_cap.detail().name());
   ResKind capkind = splid.kind();
   string capname = splid.name().asString();
 
@@ -137,9 +137,9 @@ void SolverRequester::install(const Capability & cap, const string & repoalias)
 
   if (!_opts.force_by_cap)
   {
-    PoolQuery q = pkg_spec_to_poolquery(cap, _opts.from_repos);
-    if (!repoalias.empty())
-      q.addRepo(repoalias);
+    PoolQuery q = pkg_spec_to_poolquery(pkg.parsed_cap, _opts.from_repos);
+    if (!pkg.repo_alias.empty())
+      q.addRepo(pkg.repo_alias);
 
     // get the best matching items and tag them for installation.
     // FIXME this ignores vendor lock - we need some way to do --from which
@@ -151,7 +151,7 @@ void SolverRequester::install(const Capability & cap, const string & repoalias)
       {
         Selectable::Ptr s(asSelectable()(*sit));
         if (s->kind() == ResKind::patch)
-          installPatch(cap, repoalias, *sit);
+          installPatch(pkg.parsed_cap, pkg.repo_alias, *sit);
         else
         {
           PoolItem instobj = get_installed_obj(s);
@@ -159,20 +159,22 @@ void SolverRequester::install(const Capability & cap, const string & repoalias)
           {
             // whether user requested specific repo/version/arch
             bool userconstraints =
-                cap.detail().isVersioned() || cap.detail().hasArch()
-                || !_opts.from_repos.empty() || !repoalias.empty();
+                pkg.parsed_cap.detail().isVersioned()
+                || pkg.parsed_cap.detail().hasArch()
+                || !_opts.from_repos.empty()
+                || !pkg.repo_alias.empty();
             // check vendor (since PoolItemBest does not do it)
             bool changes_vendor = instobj->vendor() != (*sit)->vendor();
 
             PoolItem best;
             if (userconstraints)
-              updateTo(cap, repoalias, *sit);
+              updateTo(pkg.parsed_cap, pkg.repo_alias, *sit);
             else if ((best = s->updateCandidateObj()))
-              updateTo(cap, repoalias, best);
+              updateTo(pkg.parsed_cap, pkg.repo_alias, best);
             else if (changes_vendor)
-              updateTo(cap, repoalias, instobj);
+              updateTo(pkg.parsed_cap, pkg.repo_alias, instobj);
             else
-              updateTo(cap, repoalias, *sit);
+              updateTo(pkg.parsed_cap, pkg.repo_alias, *sit);
           }
           else if (_command == ZypperCommand::INSTALL)
           {
@@ -180,46 +182,46 @@ void SolverRequester::install(const Capability & cap, const string & repoalias)
             MIL << "installing " << *sit << endl;
           }
           else
-            addFeedback(Feedback::NOT_INSTALLED, cap, repoalias);
+            addFeedback(Feedback::NOT_INSTALLED, pkg.parsed_cap, pkg.repo_alias);
         }
       }
       return;
     }
     else if (_opts.force_by_name)
     {
-      addFeedback(Feedback::NOT_FOUND_NAME, cap, repoalias);
-      WAR << "'" << cap << "' not found" << endl;
+      addFeedback(Feedback::NOT_FOUND_NAME, pkg.parsed_cap, pkg.repo_alias);
+      WAR << "'" << pkg.parsed_cap << "' not found" << endl;
       return;
     }
 
-    addFeedback(Feedback::NOT_FOUND_NAME_TRYING_CAPS, cap, repoalias);
+    addFeedback(Feedback::NOT_FOUND_NAME_TRYING_CAPS, pkg.parsed_cap, pkg.repo_alias);
   }
 
   // try by capability
 
   // is there a provider for the requested capability?
-  sat::WhatProvides q(cap);
+  sat::WhatProvides q(pkg.parsed_cap);
   if (q.empty())
   {
-    addFeedback(Feedback::NOT_FOUND_CAP, cap, repoalias);
-    WAR << str::form("'%s' not found", cap.asString().c_str()) << endl;
+    addFeedback(Feedback::NOT_FOUND_CAP, pkg.parsed_cap, pkg.repo_alias);
+    WAR << str::form("'%s' not found", pkg.parsed_cap.asString().c_str()) << endl;
     return;
   }
 
   // is the provider already installed?
-  set<PoolItem> providers = get_installed_providers(cap);
+  set<PoolItem> providers = get_installed_providers(pkg.parsed_cap);
   // already installed, try to update()
   for_(it, providers.begin(), providers.end())
   {
     if (_command == ZypperCommand::INSTALL)
-      addFeedback(Feedback::ALREADY_INSTALLED, cap, repoalias, *it, *it);
-    MIL << "provider '" << *it << "' of '" << cap << "' installed" << endl;
+      addFeedback(Feedback::ALREADY_INSTALLED, pkg.parsed_cap, pkg.repo_alias, *it, *it);
+    MIL << "provider '" << *it << "' of '" << pkg.parsed_cap << "' installed" << endl;
   }
 
   if (providers.empty())
   {
-    DBG << "adding requirement " << cap << endl;
-    addRequirement(cap);
+    DBG << "adding requirement " << pkg.parsed_cap << endl;
+    addRequirement(pkg.parsed_cap);
   }
 }
 
@@ -228,9 +230,9 @@ void SolverRequester::install(const Capability & cap, const string & repoalias)
 /**
  * Remove packages based on given Capability & Options from the system.
  */
-void SolverRequester::remove(const Capability & cap)
+void SolverRequester::remove(const PackageSpec & pkg)
 {
-  sat::Solvable::SplitIdent splid(cap.detail().name());
+  sat::Solvable::SplitIdent splid(pkg.parsed_cap.detail().name());
   ResKind capkind = splid.kind();
   string capname = splid.name().asString();
 
@@ -238,7 +240,7 @@ void SolverRequester::remove(const Capability & cap)
 
   if (!_opts.force_by_cap)
   {
-    PoolQuery q = pkg_spec_to_poolquery(cap, "");
+    PoolQuery q = pkg_spec_to_poolquery(pkg.parsed_cap, "");
 
     if (!q.empty())
     {
@@ -256,8 +258,8 @@ void SolverRequester::remove(const Capability & cap)
         return;
       else
       {
-        addFeedback(Feedback::NOT_INSTALLED, cap);
-        MIL << "'" << cap << "' is not installed" << endl;
+        addFeedback(Feedback::NOT_INSTALLED, pkg.parsed_cap);
+        MIL << "'" << pkg.parsed_cap << "' is not installed" << endl;
         if (_opts.force_by_name)
           return;
       }
@@ -265,38 +267,38 @@ void SolverRequester::remove(const Capability & cap)
     }
     else if (_opts.force_by_name)
     {
-      addFeedback(Feedback::NOT_FOUND_NAME, cap);
-      WAR << "'" << cap << "' not found" << endl;
+      addFeedback(Feedback::NOT_FOUND_NAME, pkg.parsed_cap);
+      WAR << "'" << pkg.parsed_cap << "' not found" << endl;
       return;
     }
   }
 
   // try by capability
 
-  addFeedback(Feedback::NOT_FOUND_NAME_TRYING_CAPS, cap);
+  addFeedback(Feedback::NOT_FOUND_NAME_TRYING_CAPS, pkg.parsed_cap);
 
   // is there a provider for the requested capability?
-  sat::WhatProvides q(cap);
+  sat::WhatProvides q(pkg.parsed_cap);
   if (q.empty())
   {
-    addFeedback(Feedback::NOT_FOUND_CAP, cap);
-    WAR << str::form("'%s' not found", cap.asString().c_str()) << endl;
+    addFeedback(Feedback::NOT_FOUND_CAP, pkg.parsed_cap);
+    WAR << str::form("'%s' not found", pkg.parsed_cap.asString().c_str()) << endl;
     return;
   }
 
   // is the provider already installed?
-  set<PoolItem> providers = get_installed_providers(cap);
+  set<PoolItem> providers = get_installed_providers(pkg.parsed_cap);
 
   // not installed, nothing to do
   if (providers.empty())
   {
-    addFeedback(Feedback::NO_INSTALLED_PROVIDER, cap);
-    MIL << "no provider of " << cap << "is installed" << endl;
+    addFeedback(Feedback::NO_INSTALLED_PROVIDER, pkg.parsed_cap);
+    MIL << "no provider of " << pkg.parsed_cap << "is installed" << endl;
   }
   else
   {
-    MIL << "adding conflict " << cap << endl;
-    addConflict(cap);
+    MIL << "adding conflict " << pkg.parsed_cap << endl;
+    addConflict(pkg.parsed_cap);
   }
 }
 
@@ -309,12 +311,12 @@ void SolverRequester::update(const PackageArgs & args)
 
   _command = ZypperCommand::UPDATE;
 
-  for_(it, args.doCaps().begin(), args.doCaps().end())
-    install(it->first, it->second);
+  for_(it, args.dos().begin(), args.dos().end())
+    install(*it);
 
   /* TODO Solve and unmark dont which are setToBeInstalled in the pool?
-  for_(it, args.dontCaps().begin(), args.dontCaps().end())
-    remove(it->first);
+  for_(it, args.donts().begin(), args.donts().end())
+    remove(*it);
   */
 }
 

diff --git a/src/SolverRequester.h b/src/SolverRequester.h
index 7723449..27340bc 100644 (file)
--- a/src/SolverRequester.h
+++ b/src/SolverRequester.h
@@ -256,7 +256,7 @@ private:
    * \note Glob can be used in the capability name for matching by name.
    * \see Options
    */
-  void install(const zypp::Capability & cap, const std::string & repoalias = "");
+  void install(const PackageSpec & pkg);
 
   /**
    * Request removal of all packages matching given \a cap by name/edition/arch,
@@ -267,7 +267,7 @@ private:
    *
    * \see Options
    */
-  void remove(const zypp::Capability & cap);
+  void remove(const PackageSpec & pkg);
 
   /**
    * Update to specified \a candidate and report if there's any better

diff --git a/tests/PackageArgs_test.cc b/tests/PackageArgs_test.cc
index 2912daf..96acd7f 100644 (file)
--- a/tests/PackageArgs_test.cc
+++ b/tests/PackageArgs_test.cc
@@ -126,19 +126,35 @@ BOOST_AUTO_TEST_CASE(argToCaps_test)
     rawargs.push_back("-irda");
 
     PackageArgs args(rawargs);
-    const PackageArgs::CapRepoPairSet & caps = args.doCaps();
-    BOOST_CHECK(caps.find(PackageArgs::CapRepoPair(
-        Capability("", "zypper", ">=", "1.4.0"),"")) != caps.end());
-    BOOST_CHECK(caps.find(PackageArgs::CapRepoPair(
-        Capability("perl(Math::BigInt)"),"")) != caps.end());
-    BOOST_CHECK(caps.find(PackageArgs::CapRepoPair(
-        Capability("laptop", ResKind::pattern),"")) != caps.end());
-    BOOST_CHECK_EQUAL(caps.size(), 3);
-
-    const PackageArgs::CapRepoPairSet & dontcaps = args.dontCaps();
-    BOOST_CHECK(dontcaps.find(PackageArgs::CapRepoPair(
-        Capability("irda"),"")) != dontcaps.end());
-    BOOST_CHECK_EQUAL(dontcaps.size(), 1);
+    const PackageArgs::PackageSpecSet & specs = args.dos();
+    {
+      PackageSpec spec;
+      spec.orig_str = "zypper>=1.4.0";
+      spec.parsed_cap = Capability("", "zypper", ">=", "1.4.0");
+      BOOST_CHECK(specs.find(spec) != specs.end());
+    }
+    {
+      PackageSpec spec;
+      spec.orig_str = "perl(Math::BigInt)";
+      spec.parsed_cap = Capability("perl(Math::BigInt)");
+      BOOST_CHECK(specs.find(spec) != specs.end());
+    }
+    {
+      PackageSpec spec;
+      spec.orig_str = "pattern:laptop";
+      spec.parsed_cap = Capability("laptop", ResKind::pattern);
+      BOOST_CHECK(specs.find(spec) != specs.end());
+    }
+    BOOST_CHECK_EQUAL(specs.size(), 3);
+
+    const PackageArgs::PackageSpecSet & dontspecs = args.donts();
+    {
+      PackageSpec spec;
+      spec.orig_str = "-irda";
+      spec.parsed_cap = Capability("irda");
+      BOOST_CHECK(dontspecs.find(spec) != dontspecs.end());
+    }
+    BOOST_CHECK_EQUAL(dontspecs.size(), 1);
   }
 }
 
@@ -170,8 +186,8 @@ BOOST_AUTO_TEST_CASE(dont_by_default_test)
     argopts.do_by_default = false;
     PackageArgs args(rawargs, ResKind::package, argopts);
 
-    BOOST_CHECK_EQUAL(args.dontCaps().size(), 2);
-    BOOST_CHECK_EQUAL(args.doCaps().size(), 1);
+    BOOST_CHECK_EQUAL(args.donts().size(), 2);
+    BOOST_CHECK_EQUAL(args.dos().size(), 1);
   }
 }