Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

direct preference in resolution doesn't work #12975

Open
1 task done
notatallshaw opened this issue Sep 22, 2024 · 1 comment
Open
1 task done

direct preference in resolution doesn't work #12975

notatallshaw opened this issue Sep 22, 2024 · 1 comment
Labels
state: needs discussion This needs some more discussion type: bug A confirmed bug or unintended behavior

Comments

@notatallshaw
Copy link
Member

notatallshaw commented Sep 22, 2024

Description

I'm trying to write a fix #12973. But it occurred to me that I don't understand what the intention of this preference is. I always assumed it meant "the requirement pointed to a file, url, or VCS".

direct was added in #10032, but in it there is no discussion of direct beyond the added docstring "Prefer if any of the known requirements is "direct", e.g. points to an explicit URL."

I'm fairly confident the code didn't do anything at the time:

lookups = (r.get_candidate_lookup() for r, _ in information[identifier])
candidate, ireqs = zip(*lookups)
...
direct = candidate is not None

get_candidate_lookup always returns a tuple of (None, None), (Candidate, None), or (None, InstallRequirement). So assuming the simplest case where there is just 1 tuple of (None, None) we get a tuple:

>>> candidate, ireqs = zip(*((None, None), ))
>>> candidate
(None,)

So the candidate could never be None.

But let's assume we actually get a Candidate in the candidate tuple, this represents that resolution has already found a candidate for this requirement, not anything specific about the requirement.

So my question is, is that intentional? And if so, what is the expected benefit to the resolution process? Or is it meant to act like how I assumed it would act?

Code of Conduct

@notatallshaw notatallshaw added type: bug A confirmed bug or unintended behavior S: needs triage Issues/PRs that need to be triaged labels Sep 22, 2024
@notatallshaw notatallshaw added state: needs discussion This needs some more discussion and removed S: needs triage Issues/PRs that need to be triaged labels Sep 23, 2024
@notatallshaw
Copy link
Member Author

notatallshaw commented Sep 24, 2024

I'm leaning toward removing this preference on the basis of:

  1. When it was implemented it never did anything
  2. Since backjumping it is doing the wrong thing
  3. There is no normal workflow that would get a file, url, VCS to be involved in backtracking, because they are typically top level requirements that are resolved with a BFS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: needs discussion This needs some more discussion type: bug A confirmed bug or unintended behavior
Projects
None yet
1 participant