fix: Support globs in MatchSpec build strings
#3735
Conversation
jjerphan
force-pushed
the
fix/correct-regexspec
branch
2 times, most recently
from
3ef58f7 to
c7db207
Compare
jjerphan
force-pushed
the
fix/correct-regexspec
branch
from
c7db207 to
6f965bc
Compare
Signed-off-by: Julien Jerphanion <[email protected]>
jjerphan
force-pushed
the
fix/correct-regexspec
branch
from
6f965bc to
844e4bc
Compare
Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: Johan Mabille <[email protected]>
| // Construct ss from raw_pattern, in particular make sure to replace all `*` by `.*` | ||
| // in the pattern if they are not preceded by a `.`. |
There was a problem hiding this comment.
Wouldn't this mean that a glob string py3.* would match py310 when it shouldn't have? i.e. we should first escape the existing . to \., and then replace * by .*?
There was a problem hiding this comment.
Yes, I am trying to think of other cases convertions to regular expression might have changed (e.g. pattern with ?).
I will add more test cases.
There was a problem hiding this comment.
Arf, I support your remark, but this clashes with behavior:
mamba/libmamba/tests/src/specs/test_chimera_string_spec.cpp
Lines 43 to 57 in 374228f
There was a problem hiding this comment.
In conda, the "full regex mode" only kicks in if the build string is wrapped in ^...$. Otherwise, it's understood as a normal glob string. See docstring and implementation.
Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: jaimergp <[email protected]>
Signed-off-by: Julien Jerphanion <[email protected]>
| TEST_CASE("RegexSpec * semantic") | ||
| { | ||
| auto spec = RegexSpec::parse("py3.*").value(); | ||
|
|
||
| REQUIRE(spec.contains("py3.")); | ||
| REQUIRE(spec.contains("py3.10")); | ||
| REQUIRE(spec.contains("py3.10_cuda11.8_cudnn8.7.0_0")); | ||
|
|
||
| REQUIRE_FALSE(spec.contains("py3")); | ||
| REQUIRE_FALSE(spec.contains("py310")); | ||
| } | ||
|
|
||
| TEST_CASE("RegexSpec ? semantic") | ||
| { | ||
| auto spec = RegexSpec::parse("py3.?").value(); | ||
|
|
||
| REQUIRE(spec.contains("py3.")); | ||
| REQUIRE(spec.contains("py3.1")); | ||
| REQUIRE(spec.contains("py3.0")); | ||
| REQUIRE(spec.contains("py3.?")); | ||
| REQUIRE(spec.contains("py3..")); | ||
|
|
||
| REQUIRE_FALSE(spec.contains("py3.10")); | ||
| REQUIRE_FALSE(spec.contains("py3")); | ||
| REQUIRE_FALSE(spec.contains("py310")); | ||
| } |
There was a problem hiding this comment.
I believe this is the semantics we want to use, yet it clashes with ChimeraString's (see https://github.com/mamba-org/mamba/pull/3735/files#r1908971307).
Fix #3699.