heroku / heroku-buildpack-python

Heroku's buildpack for Python applications.
https://www.heroku.com/python
MIT License
974 stars 1.84k forks source link

Pipenv install is skipped in cases where it's not safe to do so #1525

Closed edmorley closed 8 months ago

edmorley commented 8 months ago

Currently the buildpack skips re-running pipenv install for repeat builds iff (a) the SHA256 of the Pipfile.lock file has not changed since the last successful build, and (b) the Pipfile.lock file does not contain git references: https://github.com/heroku/heroku-buildpack-python/blob/d1ca05a398eed6824f29bf91176f5bd86e463f7a/bin/steps/pipenv#L9-L22

However, there are other cases where pipenv install still needs to be run.

For example, imagine a Pipfile containing:

[packages]
mypackage = {file = "packages/mypackage", editable = false}

Since editable = false the local package will be copied into site-packages on the initial build (rather than the original source linked via a .pth file, as happens for editable installs). And then on subsequent builds if no other changes have been made to Pipfile.lock, the pipenv install step will be skipped, even though changes could have been made to files under packages/mypackage/.

(Note: The above is assuming a Pipenv version is being used that is not affected by the regression https://github.com/pypa/pipenv/issues/6054, since otherwise the install will be editable regardless of editable = false)

Whilst we could add "editable" as another property to check within the Pipfile.lock file:

Ultimately, any optimisation strategy here requires us to make assumptions about pipenv implementation details, and hardcode some of those details in the buildpack, which isn't ideal.

As such, I think we should instead always run pipenv install, and defer to pipenv itself to make the determination as to whether any changes are required. Pipenv install should not take too long to run if there are no changes required (and if it does, that's an upstream bug that pipenv should fix, and not something we should hack around and risk environment correctness issues).

GUS-W-14762837.