From 9ba0d148231795953c1b193e1fc2969bbd40327b Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A1n=20Kupec?= Date: Mon, 10 May 2010 10:10:06 +0200 Subject: [PATCH] Using PackageSpec struct in PackageArgs --- src/PackageArgs.cc | 19 +++++---- src/PackageArgs.h | 39 ++++++++++++++----- src/SolverRequester.cc | 98 ++++++++++++++++++++++++----------------------- src/SolverRequester.h | 4 +- tests/PackageArgs_test.cc | 46 ++++++++++++++-------- 5 files changed, 125 insertions(+), 81 deletions(-) diff --git a/src/PackageArgs.cc b/src/PackageArgs.cc index b51e3e9..9d4dfc1 100644 --- a/src/PackageArgs.cc +++ b/src/PackageArgs.cc @@ -107,11 +107,12 @@ void PackageArgs::preprocess(const vector & 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 --- 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 StringSet; - typedef std::pair CapRepoPair; - typedef std::set CapRepoPairSet; + typedef std::set 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 --- 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 providers = get_installed_providers(cap); + set 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 providers = get_installed_providers(cap); + set 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 --- 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 --- 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); } } -- 2.7.4