DependencyInfo is redundant (#2314)

Removed redundant DependencyInfo and renamed MatchSpec::build
This commit is contained in:
Antoine Prouvost 2023-03-02 11:18:22 +01:00 committed by GitHub
parent ce1affbaab
commit 8c4212f5d4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 91 additions and 244 deletions

View File

@ -34,7 +34,7 @@ namespace mamba
std::string channel;
std::string ns;
std::string subdir;
std::string build;
std::string build_string;
std::string fn;
std::string url;
std::string build_number;

View File

@ -21,6 +21,7 @@
#include <fmt/color.h>
#include <solv/solver.h>
#include "mamba/core/match_spec.hpp"
#include "mamba/core/package_info.hpp"
#include "mamba/core/util_graph.hpp"
@ -30,34 +31,6 @@ namespace mamba
class MSolver;
class MPool;
/**
* Separate a dependency spec into a package name and the version range.
*/
class DependencyInfo
{
public:
DependencyInfo(const std::string& dependency);
DependencyInfo(const DependencyInfo&) = default;
DependencyInfo(DependencyInfo&&) noexcept = default;
DependencyInfo& operator=(const DependencyInfo&) = default;
DependencyInfo& operator=(DependencyInfo&&) noexcept = default;
const std::string& name() const;
const std::string& version() const;
const std::string& build_string() const;
std::string str() const;
bool operator==(const DependencyInfo& other) const;
private:
std::string m_name;
std::string m_version_range;
std::string m_build_range;
};
template <typename T>
class conflict_map : private std::unordered_map<T, vector_set<T>>
{
@ -104,7 +77,7 @@ namespace mamba
PackageNode& operator=(const PackageNode&) = default;
PackageNode& operator=(PackageNode&&) noexcept = default;
};
struct UnresolvedDependencyNode : DependencyInfo
struct UnresolvedDependencyNode : MatchSpec
{
SolverRuleinfo problem_type;
@ -113,7 +86,7 @@ namespace mamba
UnresolvedDependencyNode& operator=(const UnresolvedDependencyNode&) = default;
UnresolvedDependencyNode& operator=(UnresolvedDependencyNode&&) noexcept = default;
};
struct ConstraintNode : DependencyInfo
struct ConstraintNode : MatchSpec
{
static constexpr SolverRuleinfo problem_type = SOLVER_RULE_PKG_CONSTRAINS;
@ -124,7 +97,7 @@ namespace mamba
};
using node_t = std::variant<RootNode, PackageNode, UnresolvedDependencyNode, ConstraintNode>;
using edge_t = DependencyInfo;
using edge_t = MatchSpec;
using graph_t = DiGraph<node_t, edge_t>;
using node_id = graph_t::node_id;
@ -164,7 +137,7 @@ namespace mamba
template <typename T>
struct RoughCompare
{
bool operator()(const T& a, const T& b);
bool operator()(const T& a, const T& b) const;
};
/**
@ -243,7 +216,7 @@ namespace mamba
UnresolvedDependencyListNode,
ConstraintListNode>;
using edge_t = NamedList<DependencyInfo>;
using edge_t = NamedList<MatchSpec>;
using graph_t = DiGraph<node_t, edge_t>;
using node_id = graph_t::node_id;

View File

@ -19,6 +19,10 @@ namespace mamba
/**
* A sorted vector behaving like a set.
*
* Like, ``std::set``, uniqueness is determined by using the equivalence relation.
* In imprecise terms, two objects ``a`` and ``b`` are considered equivalent if neither
* compares less than the other: ``!comp(a, b) && !comp(b, a)``
*/
template <typename Key, typename Compare = std::less<Key>, typename Allocator = std::allocator<Key>>
class vector_set : private std::vector<Key, Allocator>
@ -86,6 +90,7 @@ namespace mamba
key_compare m_compare;
bool key_eq(const value_type& a, const value_type& b) const;
template <typename U>
std::pair<const_iterator, bool> insert_impl(U&& value);
void sort_and_remove_duplicates();
@ -119,6 +124,12 @@ namespace mamba
const vector_set<Key, Compare, Allocator>& rhs
);
template <typename Key, typename Compare, typename Allocator>
bool operator!=(
const vector_set<Key, Compare, Allocator>& lhs,
const vector_set<Key, Compare, Allocator>& rhs
);
// Simplified implementation of a directed graph
template <typename Node, typename Derived>
class DiGraphBase
@ -386,11 +397,18 @@ namespace mamba
return insert_impl(std::move(value));
}
template <typename K, typename C, typename A>
bool vector_set<K, C, A>::key_eq(const value_type& a, const value_type& b) const
{
return !m_compare(a, b) && !m_compare(b, a);
}
template <typename K, typename C, typename A>
void vector_set<K, C, A>::sort_and_remove_duplicates()
{
std::sort(Base::begin(), Base::end(), m_compare);
Base::erase(std::unique(Base::begin(), Base::end()), Base::end());
auto is_eq = [this](const value_type& a, const value_type& b) { return key_eq(a, b); };
Base::erase(std::unique(Base::begin(), Base::end(), is_eq), Base::end());
}
template <typename K, typename C, typename A>
@ -406,7 +424,7 @@ namespace mamba
auto vector_set<K, C, A>::insert_impl(U&& value) -> std::pair<const_iterator, bool>
{
auto it = std::lower_bound(begin(), end(), value, m_compare);
if ((it == end()) || (!(*it == value)))
if ((it == end()) || (!(key_eq(*it, value))))
{
Base::insert(it, std::forward<U>(value));
}
@ -416,8 +434,14 @@ namespace mamba
template <typename K, typename C, typename A>
bool operator==(const vector_set<K, C, A>& lhs, const vector_set<K, C, A>& rhs)
{
return static_cast<const std::vector<K, A>&>(lhs)
== static_cast<const std::vector<K, A>&>(rhs);
auto is_eq = [&lhs](const auto& a, const auto& b) { return lhs.key_eq(a, b); };
return std::equal(lhs.cbegin(), lhs.cend(), rhs.cbegin(), rhs.cend(), is_eq);
}
template <typename K, typename C, typename A>
bool operator!=(const vector_set<K, C, A>& lhs, const vector_set<K, C, A>& rhs)
{
return !(lhs == rhs);
}
/********************************

View File

@ -350,7 +350,7 @@ namespace mamba
MatchSpec ms(u.substr(0, hash));
PackageInfo p(ms.name);
p.url = ms.url;
p.build_string = ms.build;
p.build_string = ms.build_string;
p.version = ms.version;
p.channel = ms.channel;
p.fn = ms.fn;

View File

@ -58,7 +58,7 @@ namespace mamba
package.info.url = package_node["url"].as<std::string>();
const MatchSpec spec{ package.info.url };
package.info.fn = spec.fn;
package.info.build_string = spec.build;
package.info.build_string = spec.build_string;
package.info.subdir = spec.subdir;
package.info.channel = spec.channel;

View File

@ -94,7 +94,7 @@ namespace mamba
name = dist[0];
version = dist[1];
build = dist[2];
build_string = dist[2];
channel = parsed_channel.canonical_name();
// TODO how to handle this with multiple platforms?
@ -223,20 +223,20 @@ namespace mamba
auto [pv, pb] = parse_version_and_build(std::string(strip(version)));
version = pv;
build = pb;
build_string = pb;
// translate version '=1.2.3' to '1.2.3*'
// is it a simple version starting with '='? i.e. '=1.2.3'
if (version.size() >= 2 && version[0] == '=')
{
auto rest = version.substr(1);
if (version[1] == '=' && build.empty())
if (version[1] == '=' && build_string.empty())
{
version = version.substr(2);
}
else if (rest.find_first_of("=,|") == rest.npos)
{
if (build.empty() && version.back() != '*')
if (build_string.empty() && version.back() != '*')
{
version = concat(version, "*");
}
@ -250,7 +250,7 @@ namespace mamba
else
{
version = "";
build = "";
build_string = "";
}
// TODO think about using a hash function here, (and elsewhere), like:
@ -263,7 +263,7 @@ namespace mamba
}
else if (k == "build")
{
build = v;
build_string = v;
}
else if (k == "version")
{
@ -299,9 +299,9 @@ namespace mamba
{
res << " " << version;
// if (!build.empty() && (build != "*"))
if (!build.empty())
if (!build_string.empty())
{
res << " " << build;
res << " " << build_string;
}
}
return res.str();
@ -356,7 +356,7 @@ namespace mamba
}
else if (starts_with(version, "!=") || starts_with(version, "~="))
{
if (!build.empty())
if (!build_string.empty())
{
formatted_brackets.push_back(concat("version='", version, "'"));
}
@ -396,23 +396,23 @@ namespace mamba
}
}
if (!build.empty())
if (!build_string.empty())
{
if (is_complex_relation(build))
if (is_complex_relation(build_string))
{
formatted_brackets.push_back(concat("build='", build, "'"));
formatted_brackets.push_back(concat("build='", build_string, "'"));
}
else if (build.find("*") != build.npos)
else if (build_string.find("*") != build_string.npos)
{
formatted_brackets.push_back(concat("build=", build));
formatted_brackets.push_back(concat("build=", build_string));
}
else if (version_exact)
{
res << "=" << build;
res << "=" << build_string;
}
else
{
formatted_brackets.push_back(concat("build=", build));
formatted_brackets.push_back(concat("build=", build_string));
}
}
@ -461,6 +461,6 @@ namespace mamba
bool MatchSpec::is_simple() const
{
return version.empty() && build.empty() && build_number.empty();
return version.empty() && build_string.empty() && build_number.empty();
}
} // namespace mamba

View File

@ -30,80 +30,6 @@
namespace mamba
{
/**************************************
* Implementation of DependencyInfo *
**************************************/
DependencyInfo::DependencyInfo(const std::string& dep)
{
static std::regex const regexp("\\s*(\\w[\\w-]*)\\s*([^\\s]*)(?:\\s+([^\\s]+))?\\s*");
std::smatch matches;
const bool matched = std::regex_match(dep, matches, regexp);
// First match is the whole regex match
if (!matched || matches.size() != 4)
{
throw std::runtime_error("Invalid dependency info: " + dep);
}
m_name = matches.str(1);
m_version_range = matches.str(2);
if (m_version_range.empty())
{
m_version_range = "*";
}
m_build_range = matches.str(3);
if (m_build_range.empty())
{
m_build_range = "*";
}
}
const std::string& DependencyInfo::name() const
{
return m_name;
}
const std::string& DependencyInfo::version() const
{
return m_version_range;
}
const std::string& DependencyInfo::build_string() const
{
return m_build_range;
}
std::string DependencyInfo::str() const
{
std::string out(m_name);
out.reserve(
m_name.size() + (m_version_range.empty() ? 0 : 1) + m_version_range.size()
+ (m_build_range.empty() ? 0 : 1) + m_version_range.size()
);
if (!m_version_range.empty())
{
out += ' ';
out += m_version_range;
}
if (!m_build_range.empty())
{
out += ' ';
out += m_build_range;
}
return out;
}
bool DependencyInfo::operator==(const DependencyInfo& other) const
{
auto attrs = [](const DependencyInfo& x)
{ return std::tie(x.name(), x.version(), x.build_string()); };
return attrs(*this) == attrs(other);
}
/*************************************
* Implementation of ProblemsGraph *
*************************************/
namespace
{
@ -238,7 +164,7 @@ namespace mamba
PackageNode{ std::move(target).value(), { type } }
);
node_id cons_id = add_solvable(problem.dep_id, ConstraintNode{ dep.value() });
DependencyInfo edge(dep.value());
MatchSpec edge(dep.value());
m_graph.add_edge(src_id, cons_id, std::move(edge));
add_conflict(cons_id, tgt_id);
break;
@ -258,7 +184,7 @@ namespace mamba
problem.source_id,
PackageNode{ std::move(source).value(), std::nullopt }
);
DependencyInfo edge(dep.value());
MatchSpec edge(dep.value());
bool added = add_expanded_deps_edges(src_id, problem.dep_id, edge);
if (!added)
{
@ -277,7 +203,7 @@ namespace mamba
warn_unexpected_problem(problem);
break;
}
DependencyInfo edge(dep.value());
MatchSpec edge(dep.value());
bool added = add_expanded_deps_edges(m_root_node, problem.dep_id, edge);
if (!added)
{
@ -296,7 +222,7 @@ namespace mamba
warn_unexpected_problem(problem);
break;
}
DependencyInfo edge(dep.value());
MatchSpec edge(dep.value());
node_id dep_id = add_solvable(
problem.dep_id,
UnresolvedDependencyNode{ std::move(dep).value(), type }
@ -315,7 +241,7 @@ namespace mamba
warn_unexpected_problem(problem);
break;
}
DependencyInfo edge(dep.value());
MatchSpec edge(dep.value());
node_id src_id = add_solvable(
problem.source_id,
PackageNode{ std::move(source).value(), std::nullopt }
@ -728,9 +654,6 @@ namespace mamba
const node_id_mapping& old_to_new
)
{
// Check nothrow move for efficient push_back
static_assert(std::is_nothrow_move_constructible_v<ProblemsGraph::edge_t>);
auto add_new_edge = [&](ProblemsGraph::node_id old_from, ProblemsGraph::node_id old_to)
{
auto const new_from = old_to_new[old_from];
@ -820,29 +743,25 @@ namespace mamba
* Implementation of CompressedProblemsGraph::RoughCompare *
*************************************************************/
template <>
bool CompressedProblemsGraph::RoughCompare<ProblemsGraph::PackageNode>::operator()(
const ProblemsGraph::PackageNode& a,
const ProblemsGraph::PackageNode& b
)
{
auto attrs = [](const ProblemsGraph::PackageNode& x)
{ return std::tie(x.name, x.version, x.build_number, x.build_string); };
return attrs(a) < attrs(b);
}
template <typename T>
bool CompressedProblemsGraph::RoughCompare<T>::operator()(const T& a, const T& b)
bool CompressedProblemsGraph::RoughCompare<T>::operator()(const T& a, const T& b) const
{
auto attrs = [](const DependencyInfo& x)
{ return std::tie(x.name(), x.version(), x.build_string()); };
auto attrs = [](const auto& x)
{
return std::tie(
std::invoke(&T::name, x),
std::invoke(&T::version, x),
std::invoke(&T::build_number, x),
std::invoke(&T::build_string, x)
);
};
return attrs(a) < attrs(b);
}
template struct CompressedProblemsGraph::RoughCompare<ProblemsGraph::PackageNode>;
template struct CompressedProblemsGraph::RoughCompare<ProblemsGraph::UnresolvedDependencyNode>;
template struct CompressedProblemsGraph::RoughCompare<ProblemsGraph::ConstraintNode>;
template struct CompressedProblemsGraph::RoughCompare<DependencyInfo>;
template struct CompressedProblemsGraph::RoughCompare<MatchSpec>;
/**********************************************************
* Implementation of CompressedProblemsGraph::NamedList *
@ -1032,7 +951,7 @@ namespace mamba
template class CompressedProblemsGraph::NamedList<ProblemsGraph::PackageNode>;
template class CompressedProblemsGraph::NamedList<ProblemsGraph::UnresolvedDependencyNode>;
template class CompressedProblemsGraph::NamedList<ProblemsGraph::ConstraintNode>;
template class CompressedProblemsGraph::NamedList<DependencyInfo>;
template class CompressedProblemsGraph::NamedList<MatchSpec>;
/***********************************
* Implementation of summary_msg *

View File

@ -326,19 +326,21 @@ namespace mamba
selected_channel = make_channel(selected_channel).name();
MatchSpec modified_spec(ms);
if (!ms.channel.empty() || !ms.version.empty() || !ms.build.empty())
if (!ms.channel.empty() || !ms.version.empty() || !ms.build_string.empty())
{
Console::stream() << ms.conda_build_form()
<< ": overriding channel, version and build from "
"installed packages due to --force-reinstall.";
ms.channel = "";
ms.version = "";
ms.build = "";
ms.build_string = "";
}
modified_spec.channel = selected_channel;
modified_spec.version = check_char(pool_id2str(pool, s->evr));
modified_spec.build = check_char(solvable_lookup_str(s, SOLVABLE_BUILDFLAVOR));
modified_spec.build_string = check_char(
solvable_lookup_str(s, SOLVABLE_BUILDFLAVOR)
);
LOG_INFO << "Reinstall " << modified_spec.conda_build_form() << " from channel "
<< selected_channel;
return add_channel_specific_job(modified_spec, job_flag);

View File

@ -478,7 +478,7 @@ namespace mamba
{
PackageInfo p(ms.name);
p.url = ms.url;
p.build_string = ms.build;
p.build_string = ms.build_string;
p.version = ms.version;
p.channel = ms.channel;
p.fn = ms.fn;

View File

@ -141,7 +141,7 @@ namespace mamba
);
EXPECT_EQ(ms.name, "_libgcc_mutex");
EXPECT_EQ(ms.version, "0.1");
EXPECT_EQ(ms.build, "conda_forge");
EXPECT_EQ(ms.build_string, "conda_forge");
EXPECT_EQ(
ms.url,
"https://conda.anaconda.org/conda-forge/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2"
@ -152,7 +152,7 @@ namespace mamba
MatchSpec ms("/home/randomguy/Downloads/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2");
EXPECT_EQ(ms.name, "_libgcc_mutex");
EXPECT_EQ(ms.version, "0.1");
EXPECT_EQ(ms.build, "conda_forge");
EXPECT_EQ(ms.build_string, "conda_forge");
#ifdef _WIN32
std::string driveletter = fs::absolute(fs::u8path("/")).string().substr(0, 1);
EXPECT_EQ(

View File

@ -28,48 +28,6 @@
namespace mamba
{
TEST(dependency_info, unconstrained)
{
const auto d = DependencyInfo("foo7 ");
EXPECT_EQ(d.name(), "foo7");
EXPECT_EQ(d.version(), "*");
EXPECT_EQ(d.build_string(), "*");
EXPECT_EQ(d.str(), "foo7 * *");
}
TEST(dependency_info, version_range)
{
const auto d = DependencyInfo(" foo_bar >=4.3.0,<5.0 ");
EXPECT_EQ(d.name(), "foo_bar");
EXPECT_EQ(d.version(), ">=4.3.0,<5.0");
EXPECT_EQ(d.build_string(), "*");
EXPECT_EQ(d.str(), "foo_bar >=4.3.0,<5.0 *");
}
TEST(dependency_info, version_equality)
{
const auto d = DependencyInfo("foo-bar==4.3.0");
EXPECT_EQ(d.name(), "foo-bar");
EXPECT_EQ(d.version(), "==4.3.0");
EXPECT_EQ(d.build_string(), "*");
EXPECT_EQ(d.str(), "foo-bar ==4.3.0 *");
}
TEST(dependency_info, build_range)
{
const auto d = DependencyInfo(" python_abi 3.10.* *_cp310 ");
EXPECT_EQ(d.name(), "python_abi");
EXPECT_EQ(d.version(), "3.10.*");
EXPECT_EQ(d.build_string(), "*_cp310");
EXPECT_EQ(d.str(), "python_abi 3.10.* *_cp310");
}
TEST(dependency_info, fail)
{
EXPECT_ANY_THROW(DependencyInfo("<foo"));
}
TEST(conflict_map, symetric)
{
auto c = conflict_map<std::size_t>();

View File

@ -32,6 +32,8 @@ namespace mamba
EXPECT_EQ(vector_set<int>({ 1, 2 }), vector_set<int>({ 1, 2 }));
EXPECT_EQ(vector_set<int>({ 1, 2 }), vector_set<int>({ 2, 1 }));
EXPECT_EQ(vector_set<int>({ 1, 2, 1 }), vector_set<int>({ 2, 2, 1 }));
EXPECT_NE(vector_set<int>({ 1, 2 }), vector_set<int>({ 1, 2, 3 }));
EXPECT_NE(vector_set<int>({ 2 }), vector_set<int>({}));
}
TEST(vector_set, insertion)

View File

@ -8,7 +8,6 @@ __all__ = [
"CompressedProblemsGraph",
"Configuration",
"Context",
"DependencyInfo",
"DownloadTargetList",
"ExtraPkgInfo",
"History",
@ -263,7 +262,7 @@ class CompressedProblemsGraph:
def __init__(self) -> None: ...
def __iter__(self) -> typing.Iterator: ...
def __len__(self) -> int: ...
def add(self, arg0: DependencyInfo) -> None: ...
def add(self, arg0: MatchSpec) -> None: ...
def build_strings_trunc(
self,
sep: str = "|",
@ -671,28 +670,6 @@ class Context:
pass
pass
class DependencyInfo:
def __eq__(self, arg0: DependencyInfo) -> bool: ...
def __init__(self, arg0: str) -> None: ...
def __str__(self) -> str: ...
@property
def build_string(self) -> str:
"""
:type: str
"""
@property
def name(self) -> str:
"""
:type: str
"""
@property
def version(self) -> str:
"""
:type: str
"""
__hash__ = None
pass
class DownloadTargetList:
def __init__(self) -> None: ...
def add(self, arg0: SubdirData) -> None: ...
@ -721,7 +698,7 @@ class ExtraPkgInfo:
class History:
def __init__(self, arg0: Path) -> None: ...
def get_requested_specs_map(self) -> typing.Dict[str, mamba::MatchSpec]: ...
def get_requested_specs_map(self) -> typing.Dict[str, MatchSpec]: ...
pass
class Key:
@ -1065,7 +1042,7 @@ class ProblemsGraph:
class ConflictMap:
pass
class ConstraintNode(DependencyInfo):
class ConstraintNode(MatchSpec):
problem_type: libmambapy.bindings.SolverRuleinfo # value = <SolverRuleinfo.SOLVER_RULE_PKG_CONSTRAINS: 267>
pass
@ -1075,7 +1052,7 @@ class ProblemsGraph:
class RootNode:
pass
class UnresolvedDependencyNode(DependencyInfo):
class UnresolvedDependencyNode(MatchSpec):
@property
def problem_type(self) -> SolverRuleinfo:
"""
@ -1099,7 +1076,7 @@ class ProblemsGraph:
ProblemsGraph.ConstraintNode,
]
],
typing.Dict[typing.Tuple[int, int], DependencyInfo],
typing.Dict[typing.Tuple[int, int], MatchSpec],
]: ...
def root_node(self) -> int: ...
pass

View File

@ -253,22 +253,19 @@ PYBIND11_MODULE(bindings, m)
.def_readwrite("description", &MSolverProblem::description)
.def("__str__", [](const MSolverProblem& self) { return self.description; });
py::class_<DependencyInfo>(m, "DependencyInfo")
py::class_<MatchSpec>(m, "MatchSpec")
.def(py::init<>())
.def(py::init<const std::string&>())
.def_property_readonly("name", &DependencyInfo::name)
.def_property_readonly("version", &DependencyInfo::version)
.def_property_readonly("build_string", &DependencyInfo::build_string)
.def("__str__", &DependencyInfo::str)
.def(py::self == py::self);
.def("conda_build_form", &MatchSpec::conda_build_form);
using PbGraph = ProblemsGraph;
auto pyPbGraph = py::class_<PbGraph>(m, "ProblemsGraph");
py::class_<PbGraph::RootNode>(pyPbGraph, "RootNode").def(py::init<>());
py::class_<PbGraph::PackageNode, PackageInfo>(pyPbGraph, "PackageNode");
py::class_<PbGraph::UnresolvedDependencyNode, DependencyInfo>(pyPbGraph, "UnresolvedDependencyNode")
py::class_<PbGraph::UnresolvedDependencyNode, MatchSpec>(pyPbGraph, "UnresolvedDependencyNode")
.def_readwrite("problem_type", &PbGraph::UnresolvedDependencyNode::problem_type);
py::class_<PbGraph::ConstraintNode, DependencyInfo>(pyPbGraph, "ConstraintNode")
py::class_<PbGraph::ConstraintNode, MatchSpec>(pyPbGraph, "ConstraintNode")
.def_readonly_static("problem_type", &PbGraph::ConstraintNode::problem_type);
py::class_<PbGraph::conflicts_t>(pyPbGraph, "ConflictMap")
@ -340,11 +337,6 @@ PYBIND11_MODULE(bindings, m)
.def(py::init<const fs::u8path&>())
.def("get_requested_specs_map", &History::get_requested_specs_map);
py::class_<MatchSpec>(m, "MatchSpec")
.def(py::init<>())
.def(py::init<const std::string&>())
.def("conda_build_form", &MatchSpec::conda_build_form);
/*py::class_<Query>(m, "Query")
.def(py::init<MPool&>())
.def("find", &Query::find)