Fix for --no-clean errors under new resolver

[pfmoore]

Preliminary fix for the --no-clean test error. Before merging this, I'd like to look at whether there is a better way to get the requirement set on the InstallRequirement we create.

We could merge this and refactor later, but I'd rather try to avoid incurring the technical debt of that approach.

[pradyunsg]

A minor unrelated-to-this-change naming comment.

[pradyunsg]

Would it make sense to rename parent to template/reference/something along those lines? parent sounds like it's related to the graph we're building -- parent child relationship AKA dependent-dependency relationship -- but that's not the case here.

[pfmoore]

I think of it as parent in the sense that it's the req we create this one from - but yes, it does get confusing with the dependency graph. I'm happy to change, although I'm not keen on template - we don't take the original and "fill in the blanks" which is how I think of a template.

But a strong +1 on trying to keep addressing thses sorts of peripheral issues as we go along - it would be way too easy to end up with an accumulated pile of slightly-not-ideal decisions, and have another maintenance mess on our hands. (Says the person who inflicted PEP 517 on us 🙂)

[uranusjr]

 # The req attribute needs to be set, as otherwise this looks like an
 # "unnamed requirement" which causes req.ensure_build_location to ignore
 # the build_dir and use a temporary directory.

Does #8005 affect this? It is solving a different issue (direct URL vs specifier), but the solution also involves make_install_req_from_link(), and I think building an InstallRequirement from a requirement string would make it named?

[uranusjr]

InstallRequirement internals are such fun 🤦

[pfmoore]

Does #8005 affect this?

I wondered about that (not #8005, but the direct URL syntax). It might well, but I'm honestly not comfortable enough with that syntax to be sure.

I wish I could find a better mental model for all of this, at the moment it feels too much like "set stuff until it works" 🙁

[uranusjr]

I ran the test (test_no_clean_option_blocks_cleaning_after_wheel) against #8005, and… it passes. I have no idea what that means exactly, but probably an indication these two are related 🤯

[pfmoore]

So I guess we should decide which of #8005 and this to merge? My feeling is that #8005 looks cleaner, although the parts setting prepared and successfully_downloaded from this PR might need adding independently.

Any merges should wait till after the release now, I guess, based on @pradyunsg's comment. So there's no rush to decide, but we should keep track of stuff awaiting merge as we work through the errors.

[uranusjr]

So I guess we should decide which of #8005 and this to merge?

That’s a good idea. My work in #8066 also ends up interlocking with #8005, so getting it in first would definitely help.

I guess we should avoid anything related to InstallRequirement for now, and try to work on other (easier?) stuff like refining the error message, until the release is out.

[pfmoore]

OK, I'm fine with going for #8005

I've not found a case where we need prepared and successfully_downloaded to be set yet. Do we want to do this "for completeness", or take a YAGNI approach and wait for a test to fail before we do it?

Also - "(easier?) stuff like refining the error message" lol, "easier" isn't the word I'd use! I'll put my thoughts on error messages into Zulip, though, as it's too much for here.

[uranusjr]

Seems like #8005 fixes all the failures identified by --no-clean tests. I’ll close this for now; we can revisit this if needed.