Catch req.link = None

[fmssn]

req.link is of type InstallRequirements which can be None. Return False (requirement not met) if this is the case.

The issue

Fix issue #5950.

req is of type InstallRequirements, which has an optional attribute link which can be None. However, attributes of req.link are accessed directly.

The fix

Catch if req.link is None and return False (requirement not met) before attributes of req.link are accessed.

The checklist

• Associated issue
• A news fragment in the news/ directory to describe this fix with the extension .bugfix.rst, .feature.rst, .behavior.rst, .doc.rst. .vendor.rst. or .trivial.rst (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

[matteius]

Initial thoughts, I am wondering if this should actually return True since there was a match -- alternatively, we might handle this on line 780/782: elif req.link and match.has_metadata("direct_url.json")

[fmssn]

Then we check on 772, 780 and 797 the boolean value of req.link, which could be done once before and the cut out from there. Maybe that is a style choice.

My idea was that if we can not properly compary e.g. that from

return (
    vcs_type == req.link.scheme
    and vcs_ref == requested_revision
    and direct_url_metadata["url"]
    == pipfile_part[req.link.scheme]
)

I would assume that expected behavour for not being able to make the comparison to req.link.scheme should default to False.

But yes, thinking about it, it makes more sense to return True, as after the if statements True is also returned by default.

[fmssn]

This should be the right order to check against. Tests passed locally, had an environment fault earlier which is why they passed. Sorry for the hickup.

[myii]

@fmssn I believe this PR resulted in a regression for VCS-based package upgrades, specifically by commit 85c1ba5. I've added my findings on the bug report here:

• Private packages not updated unless environment is removed #5791 (comment)

The main query I have is if the proposed reordering of the conditional blocks will actually trigger #5950 again, which this PR resolved. If you have any feedback to share, please reply to the comment I've linked above, on issue #5791 -- that would be greatly appreciated.