Tox is unable to discover all the deps across multiple extras

[webknjaz]

Imagine the extras declared like this:

# pyproject.toml

[project.optional-dependencies]
extra-1 = ["pkg-a", "pkg-b"]
extra-2 = ["pkg-b", "pkg-c"]
extra-2 = ["pkg-d", "pkg-e"]

And tox is referencing them:

# tox.ini

extras =
  extra-1
  extra-2

I got into a situation where import pkg_d, pkg_e was failing: https://github.com/ansible/awx-plugins/actions/runs/11597267178/job/32290438874?pr=52#step:18:61.

With some debugging and staring at #2603, I discovered that the logic in https://github.com/tox-dev/tox/blob/5d880fc/src/tox/tox_env/python/virtual_env/package/util.py#L28 is flawed: when I put a breakpoint at the end of that function, I saw that the result only contains ['pkg-a', 'pkg-b']. This left me puzzled, but I already had my suspicions regarding only one of the extras contributing to the list of dependencies, having observed that some of the deps appear in the env. When I looked closer, I noticed that conditional if todo & extra_markers: and it explained what was happening: & operation on set()s returns only items that exist in both sets. So on the first iteration it could be {"pkg-a", "pkg-b"} & {"pkg-b", "pkg-c"} == {"pkg-a", "pkg-b"}, while on the following one it could be {"pkg-b", "pkg-c"} & {"pkg-d", "pkg-e"} == set(). And depending on the order of operations, a subset of extras would skip that entire if-block, not contributing any dependencies to the list.

I experimented with changing if todo & extra_markers: to if todo | extra_markers: and it helped since it's a union, not intersection. But that raised questions regarding the necessity of todo in that check. The thing is that there's while todo: a few lines above, meaning that todo is always non-empty and that check would then always be if True:. So I've gone further and changed it to just if extra_markers:. It works for me and does not seem to have any side effects. But I haven't attempted running tox's test suite against my patch.

I'm not sure if I'll get to looking into it more this week, so I'm doing the next best thing and document this observation on the tracker…

[gaborbernat]

That todo is there for circular extras I believe.

[webknjaz]

@gaborbernat but it looks like todo is still used outside the nested for-loop, within the while-loop. Removing it from this specific condition doesn't prevent handling recursive extras. Instead, it makes more cases unskipped.