Allow glob expression in experimental_requirement_cycles

[alexeagle]

Currently pip_parse supports a feature to "fix" cycles among third-party packages, for example:

pip_parse(
    ...
    experimental_requirement_cycles = {
        "airflow": [
            "apache-airflow",
            "apache-airflow-providers-common-sql",
            "apache-airflow-providers-sqlite",
            "apache-airflow-providers-ftp",
            "apache-airflow-providers-http",
            "apache-airflow-providers-imap",
        ],
    },
)

However it's difficult to keep this list updated, as it needs to include both direct and transitive dependencies. For example apache-airflow-providers-common-io appeared in the locked requirements for one of my clients, and that broke install with a surprising error message.

It would be better to write "airflow": ["apache-airflow-providers-*"] so that this is robust to whatever providers are installed. https://github.com/aspect-build/rules_js/blob/main/docs/npm_translate_lock.md#list_patches is an example of a similar repo rule in JS-land which supports globs. Note that bazel-lib provides the starlark glob implementation used there.

FYI @arrdem

[rickeylev]

Sounds reasonable to me. @aignas -- any concerns/objections?

[alexeagle]

I could possibly send a PR for it if it's accepted and no one else gets to it.

[aignas]

Accepting * only as the first or last character in the glob sounds sensible - that way we can have checks of startswith and endswith because Starlark does not support regexes. Feel free to submit a PR. :)

[arrdem]

Seems reasonable -- this would cover most of the cause we've had to adjust our cycle configs.

[alexeagle]

@aignas we can use https://github.com/bazel-contrib/bazel-lib/blob/main/docs/glob_match.md if we want to support all globs

[aignas]

Thanks for the suggestion.

I am not sure that adding an extra dependency to the ruleset for this is worth it. Just supporting prefixes and suffixes is enough for the first iteration. Phillip had a good way to remove the package cycles in his work and in the long run using that would be better.

In the long term, I think we should not have the users specify anything and rules_python should just handre the cycles itself in some way.

On 5 October 2024 00:25:38 GMT+09:00, Alex Eagle @.***> wrote:

@aignas we can use https://github.com/bazel-contrib/bazel-lib/blob/main/docs/glob_match.md if we want to support all globs

-- Reply to this email directly or view it on GitHub: https://github.com/bazelbuild/rules_python/issues/2255#issuecomment-2393956488 You are receiving this because you were mentioned.

Message ID: @.***>

[arrdem]

Does this not cover what we need? In the longer term I probably agree with Ignas, although how we handle Python dependencies would have to be very different to make that work.

A quick and dirty glob implementation

def glob_matches(pat, val):
  starts_with_glob = False
  ends_with_glob = False
  if pat[0] == "*":
    starts_with_glob = True
    pat = pat[1:]
  if pat[-1] == "*":
    ends_with_glob = True
    pat = pat[:-1]

  pat = pat.split("*")

  cur = 0
  for n, part in enumerate(pat):
    new_cur = val.find(part, cur)

    if n == 0 and not starts_with_glob:
      if new_cur != 0:
        return False

    elif n == len(pat) and not ends_with_glob:
      return pat[cur:] == part

    elif new_cur == -1:
      return False

    cur = new_cur

  return True

assert not glob_matches("*bar", "foo")
assert not glob_matches("bar*", "foo")
assert glob_matches("*bar", "bar")
assert glob_matches("*bar", "foobar")
assert glob_matches("bar", "bar*")
assert glob_matches("bar*", "barfoo")
assert glob_matches("foo*bar", "foobar")
assert glob_matches("foo*bar", "foo baz quux bar")

[aignas]

For now I am happy with any implementation as long as it does not bring extra deps to the ruleset. So supporting other globs is fine as well.