-
Notifications
You must be signed in to change notification settings - Fork 379
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.
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: Support levels properly for compatible versions #3651
fix: Support levels properly for compatible versions #3651
Conversation
Closing for reason given in #3647 (comment). |
bccce93
to
17148e1
Compare
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
… for `MatchSpec`) (mamba-org#3483)" This reverts commit 0b824fe.
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
a5ed577
to
48bb5fa
Compare
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
48bb5fa
to
e5b4440
Compare
out, | ||
"{}{}", | ||
VersionSpec::compatible_str, | ||
pred.m_version.str(op.level) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to reviewer: this line cause the bug as the solver relies on the valid assumption that:
std::string match_spec_as_str = "numpy~=1.26"; // for instance.
match_spec_as_str == MatchSpec::parse(match_spec_as_str).value().str()
here:
mamba/libmamba/src/solver/libsolv/helpers.cpp
Line 860 in ad9b2d6
const auto [first, second] = make_abused_namespace_dep_args(pool, ms.str()); |
which was not verified for compatible versions.
I think we need to add tests cases (in another PR) to test that this is valid for many MatchSpec
.
REQUIRE_FALSE("~=1.7"_vs.contains("1"_v)); | ||
REQUIRE_FALSE("~=1.7"_vs.contains("1.6.0"_v)); | ||
REQUIRE_FALSE("~=1.7"_vs.contains("0.1.0"_v)); | ||
REQUIRE("~=1.7.0"_vs.str() == "~=1.7.0"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a first test for the assumption described in https://github.com/mamba-org/mamba/pull/3651/files#r1952499642.
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
auto ms2 = MatchSpec::parse(ms.str()).value(); | ||
REQUIRE(ms2 == ms); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a first test for the assumption described in https://github.com/mamba-org/mamba/pull/3651/files#r1952499642.
~=
for MatchSpec
) with build string
Closing in preference of #3818. |
Fix #3647.
Fix #3472 properly, reverting parts of #3483.