Skip to content

First stab at implementing the resolver provider using pip internals #7789

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

Closed
wants to merge 4 commits into from

Conversation

uranusjr
Copy link
Member

This is not intended to be merged, but posted to raise awareness and discussions.

I went through a few iterations on this, but couldn’t really figure out how to directly use InstallRequirement, so right now I am using Requirement from packaging, InstallationCandidate returned by the finder, and some custom PODs to do the job. I implemented the straightforward parts, but the most hairy one awaits: Read metadata from a random candidate (for version and dependencies).

cc @pfmoore

@pfmoore
Copy link
Member

pfmoore commented Feb 26, 2020

Nice. See my post on zulip for some code snippets that would help here. I'll see if I can integrate those into what you have.

Copy link
Member

@pfmoore pfmoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some further comments.

It would be nice to have a simple unit test that ensured this was callable somehow. I'll look at writing one. I don't know what the best way is to develop code based on someone else's PR, though. For now I'll develop locally on a branch based off your PR, but we should work out how to share that code, and how to merge any useful changes into your PR.

@pfmoore
Copy link
Member

pfmoore commented Feb 27, 2020

We're working in very close to the same areas of code at the moment, and it'll be easy for us to overlap (see my latest round of comments, which were partially outdated immediately after I posted them because I hadn't refreshed my browser!)

We should discuss how to avoid clashing at today's meeting. But for now I'll work on looking at how we can write some tests for these new classes. That should avoid me duplicating what you are doing too directly.

@pfmoore
Copy link
Member

pfmoore commented Feb 27, 2020

What's the best way to share my code? I don't know how to write a PR based on another PR... 😕

Edit: Just noticed the comment below, "Add more commits by pushing to the provider-integration branch on uranusjr/pip". I guess that answers my question - I can do PRs against your repo/branch. Does that seem OK?

@uranusjr
Copy link
Member Author

I can do PRs against your repo/branch. Does that seem OK?

Yup definitely, go ahead 👍

@uranusjr
Copy link
Member Author

And if you can’t, just push this branch to pypa/pip instead. I think I can commit to non-protected branches (i.e. other than master).

@pfmoore
Copy link
Member

pfmoore commented Feb 27, 2020

Hmm, I'm getting in a mess here :-(

Given that I have a clone of https://github.com/pfmoore/pip, with a remote called "upstream" pointing at https://github.com/pypa/pip, how do I:

  1. Check out your PR? At the moment I have an alias (picked up from somewhere) that does git fetch upstream pull/${1}/head:pr_${1} && git checkout pr_${1}. That gives me a pr_7789 branch, but I don't know how to pull updates to the PR into it.
  2. Make changes so that I can push them back somewhere, and still pull/merge your changes to the PR?

I suspect what I should do is create a new uranusjr remote, and pull your branch from there and branch off that. Is that right? git pull uranusjr provider-integration; git checkout uranusjr/provider-integration; git checkout -b my_fixes?

Sorry if this is all basic, I've never needed to do anything more complex than "create a PR" workflow with git before...

Also, unrelated, but I've created a couple of new test fixtures. To keep them separate, I put them in tests.lib.pip_api_helpers, but if I do that I need to import them in test files, and flake8 then complains about unused imports and shadowing redefinitions. If I import them in conftest.py, I get an unused import error there. If I don't import them, I can't use them 🙁 What's the best fix? A noqa marker? Or give up and implement them in conftest.py?

@pradyunsg
Copy link
Member

1. git fetch upstream pull/${1}/head:pr_${1} && git checkout pr_${1}

One of the things with this is that GitHub's pull/ references are read only.

@uranusjr
Copy link
Member Author

Let’s just push to pypa/pip instead, I believe all of us have permission to do that. (I hate permission stuff.)

@uranusjr
Copy link
Member Author

Moving to #7799.

@uranusjr uranusjr closed this Feb 27, 2020
@uranusjr uranusjr deleted the provider-integration branch February 27, 2020 14:36
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Apr 2, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants