Fix for Pipenv not caching dependencies #1130
Conversation
litchfield
changed the title
litchfield
changed the title
There was a problem hiding this comment.
@litchfield Sorry for the delayed reply. I don't suppose you have a testcase Pipfile + Pipfile.lock I can use to try out the before/after?
I'm also curious about the choices behind the buildpack's existing behaviour to skip pipenv install. Specifically:
- Is the speedup here from not having to install pipenv itself, or in being able to skip the
pipenv installstep? - Why does this code even need to check for the presence of
gitinPipfile.lockat all? Surely the lockfile should have an explicit SHA256 and not a branch name, so be deterministic? If not, should pipenv change here?
|
Thanks. Reading the patch, this was a quick fix to tighten existing regex and get it working. But I agree, not sure why we would even check if we have a lock file. I don’t have a pip file handy but any with a git source reference should do it. |
|
Reading through #656 and #654 it appears the issue is more to to with editable dependencies rather than git. I'm pretty sure this is due to the path rewriting hacks (#1006 has some background on these) meaning that editable installs aren't correctly restored for subsequent builds. Hopefully in the future when we can remove the path rewriting the need to repeat the pipenv install may go away. |
Previously the buildpack would skip running `pipenv install` for repeat
Pipenv builds if (a) the SHA256 of the `Pipfile.lock` file had not changed
since the last successful build, and (b) there were no Git VCS references
in the lockfile.
However, this has a few issues:
1. There are other cases where it's not safe to assume that there is no
need to re-run `pipenv install`, such as when installing a non-editable
local dependency (see #1525), or when using editable local dependencies
with the current path re-writing strategy (see #1520).
2. The current Git VCS check has false positives (see #1130, #1398).
3. Even if we try and add more checks/workarounds to resolve (1) and (2),
we're still having to make assumptions about internal Pipenv implementation
details, and hardcode these in the buildpack, hoping we didn't miss anything
and that Pipenv's behaviour doesn't change over time (which is not the case,
as seen by the recent regression pypa/pipenv#6054)
As such, we now instead always re-run `pipenv install`, and defer to Pipenv to
decide whether the environment needs updating.
This should still be fast, since the cached `site-packages` is still being used (and
if there are any scenarios in which it's not fast, then that's an upstream Pipenv
bug).
Integration tests were also added for various types of editable Pipenv installs,
since we previously only had test coverage of editable installs for Pip.
Fixes #1520.
Fixes #1525.
Closes #1130.
Closes #1398.
|
The fragile |
Having 'git' in one of your package names will prevent your deps from caching, resulting in painfully long build times. This is a simple fix.