fix: Support globs in `MatchSpec` build strings #3735

jjerphan · 2025-01-08T16:28:23Z

libmamba/include/mamba/specs/regex_spec.hpp

libmamba/src/specs/regex_spec.cpp

jaimergp · 2025-01-09T11:11:30Z

libmamba/src/specs/regex_spec.cpp

+        // Construct ss from raw_pattern, in particular make sure to replace all `*` by `.*`
+        // in the pattern if they are not preceded by a `.`.


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 .*?

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.

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

TEST_CASE("ChimeraStringSpec py.*")

{

auto spec = ChimeraStringSpec::parse("py.*").value();

REQUIRE(spec.contains("python"));

REQUIRE(spec.contains("py"));

REQUIRE(spec.contains("pypy"));

REQUIRE_FALSE(spec.contains(""));

REQUIRE_FALSE(spec.contains("cpython"));

REQUIRE(spec.str() == "^py.*$");

REQUIRE_FALSE(spec.is_explicitly_free());

REQUIRE_FALSE(spec.is_exact());

REQUIRE_FALSE(spec.is_glob());

}

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.

micromamba/tests/test_create.py

libmamba/src/specs/regex_spec.cpp

jjerphan · 2025-01-09T15:47:21Z

libmamba/tests/src/specs/test_regex_spec.cpp

+    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"));
+    }


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).

Signed-off-by: Julien Jerphanion <[email protected]>

Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: Johan Mabille <[email protected]>

Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: jaimergp <[email protected]>

Signed-off-by: Julien Jerphanion <[email protected]>

Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: jaimergp <[email protected]>

libmamba/src/specs/regex_spec.cpp

So that they can be initialized properly using a utility function. Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: Johan Mabille <[email protected]>

jjerphan force-pushed the fix/correct-regexspec branch 2 times, most recently from 3ef58f7 to c7db207 Compare January 8, 2025 16:31

jjerphan added the release::bug_fixes For PRs fixing bugs label Jan 8, 2025

jjerphan force-pushed the fix/correct-regexspec branch 2 times, most recently from 6f965bc to 844e4bc Compare January 8, 2025 16:55

jjerphan marked this pull request as ready for review January 8, 2025 18:07

jjerphan mentioned this pull request Jan 9, 2025

v2 doesn't solve pytorch=2.3.1=py3.10_cuda11.8*, but v1 does #3699

Closed

3 tasks

JohanMabille reviewed Jan 9, 2025

View reviewed changes

libmamba/include/mamba/specs/regex_spec.hpp Outdated Show resolved Hide resolved

libmamba/src/specs/regex_spec.cpp Outdated Show resolved Hide resolved

jaimergp reviewed Jan 9, 2025

View reviewed changes

micromamba/tests/test_create.py Outdated Show resolved Hide resolved

Klaim reviewed Jan 9, 2025

View reviewed changes

libmamba/src/specs/regex_spec.cpp Outdated Show resolved Hide resolved

jjerphan commented Jan 9, 2025

View reviewed changes

jjerphan marked this pull request as draft January 9, 2025 15:47

jjerphan force-pushed the fix/correct-regexspec branch from b8104d2 to 2d1ce12 Compare January 15, 2025 14:58

jjerphan force-pushed the fix/correct-regexspec branch from 2d1ce12 to 1dacead Compare January 24, 2025 10:33

jjerphan and others added 5 commits January 24, 2025 11:34

fix: Support globs in build strings

f6dd13c

Signed-off-by: Julien Jerphanion <[email protected]>

Apply review comments

9a95b2e

Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: Johan Mabille <[email protected]>

test: Monkeypatch environment variable

624edf8

Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: jaimergp <[email protected]>

Adapt tests

edd2261

Signed-off-by: Julien Jerphanion <[email protected]>

Early catch regular expression

5a33edd

Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: jaimergp <[email protected]>

jjerphan force-pushed the fix/correct-regexspec branch from 1dacead to 5a33edd Compare January 24, 2025 10:35

jjerphan marked this pull request as ready for review January 24, 2025 10:47

JohanMabille requested changes Jan 24, 2025

View reviewed changes

libmamba/src/specs/regex_spec.cpp Show resolved Hide resolved

Reorder and initialize RegexSpec's private members

4b50d22

So that they can be initialized properly using a utility function. Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: Johan Mabille <[email protected]>

jjerphan enabled auto-merge (squash) January 27, 2025 12:37

JohanMabille approved these changes Jan 27, 2025

View reviewed changes

Merge branch 'main' into fix/correct-regexspec

407c4dd

jjerphan merged commit 57a2c55 into mamba-org:main Jan 27, 2025
13 of 15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Support globs in `MatchSpec` build strings #3735

fix: Support globs in `MatchSpec` build strings #3735

jjerphan commented Jan 8, 2025

jaimergp Jan 9, 2025

jjerphan Jan 9, 2025 •

edited

Loading

jjerphan Jan 9, 2025

jaimergp Jan 9, 2025 •

edited

Loading

jjerphan Jan 9, 2025

		// Construct ss from raw_pattern, in particular make sure to replace all `` by `.`
		// in the pattern if they are not preceded by a `.`.

	TEST_CASE("ChimeraStringSpec py.*")
	{
	auto spec = ChimeraStringSpec::parse("py.*").value();

	REQUIRE(spec.contains("python"));
	REQUIRE(spec.contains("py"));
	REQUIRE(spec.contains("pypy"));
	REQUIRE_FALSE(spec.contains(""));
	REQUIRE_FALSE(spec.contains("cpython"));

	REQUIRE(spec.str() == "^py.*$");
	REQUIRE_FALSE(spec.is_explicitly_free());
	REQUIRE_FALSE(spec.is_exact());
	REQUIRE_FALSE(spec.is_glob());
	}

fix: Support globs in MatchSpec build strings #3735

fix: Support globs in MatchSpec build strings #3735

Conversation

jjerphan commented Jan 8, 2025

jaimergp Jan 9, 2025

Choose a reason for hiding this comment

jjerphan Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

jjerphan Jan 9, 2025

Choose a reason for hiding this comment

jaimergp Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

jjerphan Jan 9, 2025

Choose a reason for hiding this comment

fix: Support globs in `MatchSpec` build strings #3735

fix: Support globs in `MatchSpec` build strings #3735

jjerphan Jan 9, 2025 •

edited

Loading

jaimergp Jan 9, 2025 •

edited

Loading