Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix add missing pip PREREQ_MARKER #2612

Merged
merged 2 commits into from
Jun 21, 2023
Merged

Fix add missing pip PREREQ_MARKER #2612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae0b123
Add marker support in MSolvable::dependencies
AntoinePrv Jun 21, 2023
3df7548
Add PREREQ_MARKER to pip
AntoinePrv Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions libmamba/src/core/repo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ extern "C" // Incomplete header
#include <solv/repo_conda.h>
}

#include "mamba/core/channel.hpp"
#include "mamba/core/context.hpp"
#include "mamba/core/mamba_fs.hpp"
#include "mamba/core/output.hpp"
#include "mamba/core/package_info.hpp"
#include "mamba/core/pool.hpp"
#include "mamba/core/prefix_data.hpp"
#include "mamba/core/repo.hpp"
#include "mamba/core/util.hpp"
#include "solv-cpp/pool.hpp"
#include "solv-cpp/repo.hpp"

Expand Down Expand Up @@ -228,18 +228,13 @@ namespace mamba
srepo(*this).for_each_solvable(
[&](solv::ObjSolvableView s)
{
if (s.name() == "python")
if ((s.name() == "python") && !s.version().empty() && (s.version()[0] >= '2'))
{
if (!s.version().empty() && (s.version()[0] >= '2'))
{
s.add_dependency(pip_id);
}
s.add_dependency(pip_id);
}
if (s.name() == "pip")
{
auto deps = s.dependencies();
deps.insert(deps.begin(), python_id); // TODO is this right?
s.set_dependencies(std::move(deps));
s.add_dependency(python_id, SOLVABLE_PREREQMARKER);
}
}
);
Expand Down
1 change: 1 addition & 0 deletions libmamba/src/solv-cpp/ids.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace mamba::solv
using SolvableId = ::Id;
using RuleId = ::Id;
using ProblemId = ::Id;
using DependencyMarker = ::Id;

using RelationFlag = int;
using DistType = int;
Expand Down
29 changes: 12 additions & 17 deletions libmamba/src/solv-cpp/solvable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,37 +349,32 @@ namespace mamba::solv
return set_subdir(str.c_str());
}

auto ObjSolvableViewConst::dependencies() const -> ObjQueue
auto ObjSolvableViewConst::dependencies(DependencyMarker marker) const -> ObjQueue
{
auto q = ObjQueue{};
::solvable_lookup_deparray(
const_cast<::Solvable*>(raw()),
SOLVABLE_REQUIRES,
q.raw(),
/* marker= */ -1
);
::solvable_lookup_deparray(const_cast<::Solvable*>(raw()), SOLVABLE_REQUIRES, q.raw(), marker);
return q;
}

void ObjSolvableView::set_dependencies(const ObjQueue& q) const
void ObjSolvableView::set_dependencies(const ObjQueue& q, DependencyMarker marker) const
{
::solvable_set_deparray(
raw(),
SOLVABLE_REQUIRES,
const_cast<::Queue*>(q.raw()),
/* marker= */ 0
);
::solvable_set_deparray(raw(), SOLVABLE_REQUIRES, const_cast<::Queue*>(q.raw()), marker);
}

void ObjSolvableView::add_dependency(DependencyId dep) const
void ObjSolvableView::add_dependency(DependencyId dep, DependencyMarker marker) const
{
raw()->requires = ::repo_addid_dep(raw()->repo, raw()->requires, dep, /* marker= */ 0);
raw()->requires = ::repo_addid_dep(raw()->repo, raw()->requires, dep, marker);
}

auto ObjSolvableViewConst::provides() const -> ObjQueue
{
auto q = ObjQueue{};
::solvable_lookup_deparray(const_cast<::Solvable*>(raw()), SOLVABLE_PROVIDES, q.raw(), -1);
::solvable_lookup_deparray(
const_cast<::Solvable*>(raw()),
SOLVABLE_PROVIDES,
q.raw(),
/* marker= */ -1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know why we need this magic number here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was only reformated by I believe this magic number was already present pre-refactoring.

The idea is that dep array can be split in to with a magic marker. When retrieving the dependencies, -1 means first part, 1 means second and 0 means everything, including said marker (which is not a valid dependency).
That works if the default maker was used when setting the dependencies...
So yes, a bit of a mess.

See also the test case added in this PR. So far we have no need for markers in provide key so we keep sane defaults.

);
return q;
}

Expand Down
23 changes: 18 additions & 5 deletions libmamba/src/solv-cpp/solvable.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,15 @@ namespace mamba::solv
**/
auto subdir() const -> std::string_view;

/** Queue of ``DependencyId``. */
auto dependencies() const -> ObjQueue;
/**
* Queue of ``DependencyId``.
*
* When the array is split in two using a maker, @p marker can be used to get
* only a part of the the dependency array.
* Use ``-1`` to get the first part, ``1`` to get the second part, and ``0`` to get
* eveything including the marker.
*/
auto dependencies(DependencyMarker marker = -1) const -> ObjQueue;

/** Queue of ``DependencyId``. */
auto provides() const -> ObjQueue;
Expand Down Expand Up @@ -241,10 +248,16 @@ namespace mamba::solv
void set_subdir(const std::string& str) const;

/** Set the dependencies of the solvable. */
void set_dependencies(const ObjQueue& q) const;
void set_dependencies(const ObjQueue& q, DependencyMarker marker = 0) const;

/** Add a additional dependency to the solvable. */
void add_dependency(DependencyId dep) const;
/**
* Add a additional dependency to the solvable.
*
* Dependency array can be split in two using the given @p marker.
*
* @see dependencies
*/
void add_dependency(DependencyId dep, DependencyMarker marker = 0) const;

/** Add multiple additional dependencies to the solvable. */
template <typename Iter>
Expand Down
12 changes: 12 additions & 0 deletions libmamba/tests/src/solv-cpp/test_solvable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,18 @@ TEST_SUITE("ObjSolvable")
solv.set_dependencies({});
CHECK(solv.dependencies().empty());
}

SUBCASE("Dependencies with markers")
{
solv.add_dependency(34);
solv.add_dependency(11, SOLVABLE_PREREQMARKER);
solv.add_dependency(35);

CHECK_EQ(solv.dependencies(-1), ObjQueue{ 33, 34 });
CHECK_EQ(solv.dependencies(0), ObjQueue{ 33, 34, SOLVABLE_PREREQMARKER, 11, 35 });
CHECK_EQ(solv.dependencies(1), ObjQueue{ 11, 35 });
CHECK_EQ(solv.dependencies(SOLVABLE_PREREQMARKER), ObjQueue{ 11, 35 });
}
}

SUBCASE("Add provide")
Expand Down