pipenv
pipenv copied to clipboard
Issue 4371 incorrect dependencies when install dev packages
The issue
I'm trying to fix https://github.com/pypa/pipenv/issues/4371. I think it's better to consider 2 cases below:
- Installing from Pipfile.
- Installing from provided packages (the issue itself).
The fix
Installing from Pipfile
I read through the code, and here is what it does when installing packages from Pipfile:
- Resolve
dev-packagesdependencies. Write to lockfile as default. - Resolve
packagesdependencies. Write to lockfile as develop. - Overwrite duplicated develop packages by default packages.
- Install default and develop packages in the lockfile.
Because, dev-packages is resolved separately, there might be a chance where dev-packages is overwritten but its dependencies are not (they aren't duplicated with packages). So I think the result dependency graph would be more accurate if packages is considered as constraint while resolving dev-packages dependencies. Luckily pip provided constraints files.
I modified the code to do the followings:
- Resolve
packagesdependencies. Write to lockfile. - Create constraints file from
packages. Skip editable and extras packages as describe in pip constraints files. - Resolve
dev-packagesdependencies using constraints file above. Write to lockfile as default. - Overwrite duplicated packages. Although
dev-packageswas resolved with constraints, this step is also important because there some cases constraints might be skipped (editable and extras packages). - Install resolved packages.
Installing from provided packages
When packages are passed as arguments, pipenv does the followings:
- Install passed packages without resolving dependencies. Write to Pipfile and lockfile.
- Resolving
dev-packagesandpackagesindependently. Modify the lockfile. - Overwrite duplicated packages.
- Install all the packages in the lockfile again. There might be a case it will uninstall the packages from step 1 before installing.
Similar to the first problem, I tried to use default packages as constraints to resolve packages dependencies. I think it is enough to just resolve dev-packages at step 1. Because default packages should be installed directly, and resolved packages at step 2 don't need to be resolved again.
The checklist
- [ ] Associated issue
- [ ] A news fragment in the
news/directory to describe this fix with the extension.bugfix,.feature,.behavior,.doc..vendor. or.trivial(this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.
@dqkqd I left a few more notes about areas I think we could further refine the code and reduce duplication. I think if we can factor out the three spots we write constraint files into a common utility and some of the places we can use @cached_property to allow for simpler and more readable properties. I think the change makes sense and could be merged, but chipping away at the some more of this type of technical debt would be good.
@dqkqd Nice work!
@matteius Thank you.