Potential bug on install requirements mutation during dependency resolution

[atugushev]

Description

While working on the new resolver support in pip-tools (jazzband/pip-tools#1539) I ran into an issue with failing tests against pip's main branch. Using git bisect I've tracked down a bad commit 0c28452 which was introduced in #10962.

The main issue with the following line:

pip/src/pip/_internal/resolution/resolvelib/factory.py

Line 310 in 0c28452

template.req.specifier = specifier

... where template essentially is ireqs[0]:

pip/src/pip/_internal/resolution/resolvelib/factory.py

Line 258 in 0c28452

template = ireqs[0]

See also discussion on the commit.

Expected behavior

I've prepared a temporary fix where template is deep copied from ireqs[0] (see #11018) which makes pip-tools' tests pass. Any ideas on how to avoid ireqs mutation which might lead to potential bugs?

pip version

22.1.dev0

Python version

3.8

OS

macOS

How to Reproduce

Unfortunately, I haven't found yet simple reproducer other than pip-tools' tests:

1. git clone https://github.com/atugushev/pip-tools@new-resolver
2. cd pip-tools
3. tox -e py38-pipmain -- -k 'not network'

Output

https://github.com/jazzband/pip-tools/runs/5872141303?check_suite_focus=true

Code of Conduct

• I agree to follow the PSF Code of Conduct.

[pradyunsg]

I'm marking this as a release blocker, or at least, as something we'd want to see tested as part of a beta release before a proper release.

[pradyunsg]

OK, I'm leaning toward reverting #10962 at the moment, since we don't seem to have a clear understanding of what the right fix for this is (outside of doing a deepcopy, which makes me concerned about the memory usage of long-running resolution runs).

[pradyunsg]

Beyond that, this is in a pip beta release now -- so... if folks wanna try this out and let us know if things break in weird ways, please feel welcome to!

[pradyunsg]

@atugushev Could you provide a reproducer for the issue?

[pradyunsg]

I've just merged #11085, which drops this concern for now and unblocks this release.

[pradyunsg]

Leaving this open actually, because this still needs a reproducer.

[sleiner]

@pradyunsg

we don't seem to have a clear understanding of what the right fix for this is (outside of doing a deepcopy, which makes me concerned about the memory usage of long-running resolution runs)

If we must not modify ireqs, but to modify template in the special case for having in #9243, we could only copy template in the case where we actually perform those modifications (either deeply or even only shallow + the modified parts). As far as I see, this would not impact memory usage for the cases that work currently but make the cases of #9243 work. What do you think?

[uranusjr]

That’s already tried (see linked unmerged PR above) and did not seem to fix the issue, leading to suspicions that the pip-tools bug is not actually related to requirement mutation. If you want to address the mutation issue specifically for a different reason, feel free to open a new PR.