Isolate PackageInfo from libsolv #2340
Conversation
|
nice! |
| .def(py::init<>( | ||
| [](MSolver& solver, MultiPackageCache& mpc) | ||
| { | ||
| deprecated("Use Transaction(Pool, Solver, MultiPackageCache) instead"); | ||
| return std::make_unique<MTransaction>(solver.pool(), solver, mpc); | ||
| } | ||
| )) |
There was a problem hiding this comment.
Keeping deprecated API in Python
| @@ -1468,7 +1473,7 @@ namespace mamba | |||
| } | |||
| else | |||
| { | |||
| if (!need_pkg_download(s, m_multi_cache)) | |||
| if (!need_pkg_download(mk_pkginfo(m_pool, s), m_multi_cache)) | |||
There was a problem hiding this comment.
mk_pkginfo returns a PackageInfo right? It replaces a solvable here?
There was a problem hiding this comment.
Yes, the previous line was calling an implicit conversion from Solvable* to PackageInfo.
| @@ -126,7 +122,7 @@ namespace mamba | |||
| const std::vector<MatchSpec>& specs_to_install, | |||
| MultiPackageCache& caches | |||
| ); | |||
| MTransaction(MSolver& solver, MultiPackageCache& caches); | |||
| MTransaction(MPool& pool, MSolver& solver, MultiPackageCache& caches); | |||
There was a problem hiding this comment.
Because the member m_pool is a value and not a reference, it is not clear what is going on here. In the constructor implementation we pass m_pool(pool), what does this mean? Is it a copy?
There was a problem hiding this comment.
It's a shared reference. We had the discussion last autumn to make MPool interally based on an std::shared_ptr.
This will also change as we refactor the pool.
There was a problem hiding this comment.
Ah ok, then why not pass it by value? that would make it clear that whatever the model, there is ownership being acquired. There is no condition to that so no reason to pass a reference.
There was a problem hiding this comment.
I missed your answer before I merged, I can make the change in a mechanical PR.
Solvable*->PackageInfois done in theMPool.MTransactionneeds to capture theMPoolto use that functions (but anyways it was using aPool*already, at least it's explicit).The rest of the changes are to cope with these changes throughout the code base.