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

implement PubGrub for pip resolver? #8203

Closed
brainwane opened this issue May 7, 2020 · 7 comments
Closed

implement PubGrub for pip resolver? #8203

brainwane opened this issue May 7, 2020 · 7 comments
Labels
C: dependency resolution About choosing which dependencies to install state: needs discussion This needs some more discussion

Comments

@brainwane
Copy link
Contributor

Per conversation in a meeting today (will link once notes are up), we are now finding in our error messages disentangling work that we're rediscovering needs/features that are in PubGrub. Thus, we are discussing whether we should add a PubGrub implementation to pip (or ResolveLib).

Personally I agree PubGrub is the best technical choice for Python, and also the best choice for pip in theory. The problem here however is that there is no Python implementation suitable for pip’s usage. As mentioned above, it is difficult to integrate Mixology into pip due to data structure incompatibilities; on the other hand, both of our backtracking choices are designed to fit well into pip.

I’ve been thinking about implementing a PubGrub resolver (maybe based on Mixology, maybe not) that expose a similar interface to ResolveLib and Zazo, that can be better used by projects without forcing them into using dedicated data structures. But I don’t believe it is the best use of my time in the near term given my responsibilities (including bringing a resolver into pip within a deadline).

Given that proper resolvers are functionally interchangeable, my choice now is to pursue a more reachable goal. [insert Perfect is the Enemy of Good cliche here] But I also intend to help line up goals and efforts for a future alternative implementation, and work on that after pip has something to show for.

Originally posted by @uranusjr in #7406 (comment)

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label May 7, 2020
@brainwane brainwane added C: dependency resolution About choosing which dependencies to install C: new resolver state: needs discussion This needs some more discussion state: needs eyes Needs a maintainer/triager to take a closer look and removed S: needs triage Issues/PRs that need to be triaged labels May 7, 2020
@brainwane
Copy link
Contributor Author

This would be a good place to lay out (or link to an explanation of): what would be the benefit of adding a PubGrub implementation?

And we don't know how long it would take, and whether it would change the ResolveLib API. So how long would it take to do a little investigation and get an estimate?

@ddelange
Copy link
Contributor

don't know much about the pip internals, but fwiw I modified mixology for compatibility with pkg_resources.Requirement (only picked up the [extras] but ;python_requires additionally should be a small step from there).

ddelange/pipgrip#8

@pradyunsg
Copy link
Member

pradyunsg commented Oct 7, 2020

FTR, the bigger issue for using mixology in pip's context is the design of mixology's Range class, which can't be implemented based off of packaging.Specifier trivially, as far as I can tell.

@uranusjr
Copy link
Member

uranusjr commented Oct 7, 2020

I believe most of those things that cannot be implemented trivially are also blockers to implementing PubGrub for Python packaging in general. Much of the rich version arithmetic stuff in Mixology involves semver assumptions, which Python packaging does not enforce. This is fine for Poetry, but not more generic tools such as pip.

@ddelange
Copy link
Contributor

ddelange commented Oct 7, 2020

Are there Specifier counter examples where mixology would break/give unwanted results? the Constraint below is what mixology uses in solving. Instead of a plain 'wheel', in pipgrip it will also take extras like ipython[test].

>>> from packaging.specifiers import SpecifierSet
>>> specifier = SpecifierSet("~=1.0.0rc1") & SpecifierSet("!=1.0.3")
<SpecifierSet('!=1.0.3,~=1.0.0rc1')>

>>> from pipgrip.libs.semver import parse_constraint
>>> versions = parse_constraint(str(specifier))
<VersionUnion >=1.0.0rc1,<1.0.3 || >1.0.3,<1.1.0>

>>> from pipgrip.libs.mixology.package import Package
>>> package = Package('wheel')
Package("wheel")

>>> from pipgrip.libs.mixology.constraint import Constraint
>>> constraint = Constraint(package, versions)
>>> str(constraint)
'wheel (>=1.0.0rc1,<1.0.3 || >1.0.3,<1.1.0)'

@pfmoore
Copy link
Member

pfmoore commented Oct 7, 2020

@ddelange I think the key point here is that mixology doesn't conform to PEP 440, so it's not suitable for pip. Here's an example - I completely concede that it's an edge case (projects don't typically use epochs) but pip is committed to implementing the standards, so we have to handle cases like this:

>>> from packaging.specifiers import SpecifierSet
>>> s = SpecifierSet("==3!2020.12.dev4")
>>> from pipgrip.libs.semver import parse_constraint
>>> versions = parse_constraint(str(s))
>>> versions
<Version 3>
>>> str(s)
'==3!2020.12.dev4'

Maybe things like this can be worked around, but that's why "working out how to integrate mixology" is a blocker.

@brainwane
Copy link
Contributor Author

I just spoke with Pradyun and we found:

  • we have consensus that we are not going to implement PubGrub wholesale in pip or resolvelib
  • we may want some specific features/bits of functionality that would be nice to have (that would generally be implemented in resolvelib), such as remembering that a conflict took place (rather than having to rediscover it later on in the process)

Therefore I am closing this issue, but @pradyunsg @uranusjr could you please ensure there are open issues within resolvelib for any specific desired features that we'd be copying/borrowing from PubGrub?

Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2021
@pradyunsg pradyunsg removed the state: needs eyes Needs a maintainer/triager to take a closer look label Dec 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: dependency resolution About choosing which dependencies to install state: needs discussion This needs some more discussion
Projects
None yet
Development

No branches or pull requests

5 participants