Aggregate specifier from all requirements #9060
Conversation
| # HACK: The InstallRequirement implementation requires us to give it a | ||
| # "template", so we try to "invent" one by aggregating available | ||
| # information from all the ireqs. This feels very brittle, hopefully | ||
| # the Project model can correct this mismatch in the future. | ||
| name = canonicalize_name(ireqs[0].req.name) | ||
| template = InstallRequirement( | ||
| PackagingRequirement("{} {}".format(name, specifier)), | ||
| user_supplied=any(r.user_supplied for r in ireqs), | ||
| comes_from=ireqs[0].comes_from, | ||
| use_pep517=any(r.use_pep517 for r in ireqs), | ||
| isolated=any(r.isolated for r in ireqs), | ||
| constraint=all(r.constraint for r in ireqs), | ||
| install_options=list(itertools.chain.from_iterable( | ||
| r.install_options for r in ireqs | ||
| )), | ||
| global_options=list(itertools.chain.from_iterable( | ||
| r.global_options for r in ireqs | ||
| )), | ||
| hash_options=dict(itertools.chain.from_iterable( | ||
| r.hash_options.items() for r in ireqs | ||
| )), | ||
| ) |
There was a problem hiding this comment.
OMG, this sucks like a sucky thing that's just won the world sucking championships. If this passes the tests, I'll be impressed and disturbed in about equal measure.
I don't think there's any better way of doing this, but it's going to be a maintenance nightmare. I would honestly give serious consideration to not fixing #9020 rather than making this change. The issue is a consequence of the fact that we stripped constraints back to being "just" limits on what versions the resolver sees, rather than being full requirements with a weird "but don't install anything unless there's another requirement for the same package" limitation. I understand the OP's concerns in #9020 but at some point we have to start clamping down on the whole "it was never documented but I use it so you have to support it forever" thing - or we'll end up blocked on ever changing pip, and we'll have another distutils on our hands 🙁
I'd characterise the change in behaviour as an "intentional regression", as the OP in #9020 put it.
There was a problem hiding this comment.
OMG, this sucks like a sucky thing that's just won the world sucking championships.
lol
I wouldn't go that faaar, but yea, re-reading #9020 I think it's fine to put this into "constraints aren't the right tool for this" camp.
There was a problem hiding this comment.
I wouldn't go that faaar
On re-reading, I should be clear, I mean the whole "InstallRequirement needs a template, and we don't have one so we have to make something up" situation, not @uranusjr's code (which as I say is about the best we can do in the situation). Sorry @uranusjr if it came across as criticising your code...
There was a problem hiding this comment.
Good news, there are regressions that I don’t understand and not very interested in looking into.
Let’s see what the test cases say…
Would fix #9020