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

Never skip running pipenv install #1526

Merged
merged 1 commit into from
Jan 5, 2024
Merged

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Jan 5, 2024

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 Pipenv install is skipped in cases where it's not safe to do so #1525), or when using editable local dependencies with the current path re-writing strategy (see Python buildpack fails to deploy without first purging cache #1520).
  2. The current Git VCS check has false positives (see Fix for Pipenv not caching dependencies #1130, pipenv cache is skipped when it should not be #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 As of v2023.8.19 local file dependencies default to being installed as editable 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.
GUS-W-14762837.

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.
@edmorley edmorley added the bug label Jan 5, 2024
@edmorley edmorley self-assigned this Jan 5, 2024
@edmorley edmorley marked this pull request as ready for review January 5, 2024 14:37
@edmorley edmorley requested a review from a team as a code owner January 5, 2024 14:37
@edmorley edmorley merged commit a97924c into main Jan 5, 2024
6 checks passed
@edmorley edmorley deleted the pipenv-always-run-install branch January 5, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants