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

Change pipenv install to update the lock file first #1486

Closed
ncoghlan opened this issue Feb 24, 2018 · 4 comments
Closed

Change pipenv install to update the lock file first #1486

ncoghlan opened this issue Feb 24, 2018 · 4 comments
Assignees
Labels
Status: Requires Approval This issue requires additional approval to move forward. Type: Enhancement 💡 This is a feature or enhancement request.

Comments

@ncoghlan
Copy link
Member

(Breaking out another substep of #1255)

When a specific package to install is passed to pipenv install, the current logical flow is as follows:

  • for each package requested on the command line:
    • install the package into the current environment
    • if that fails, abort the operation
    • update Pipfile with the new or revised dependency
  • after all requested packages have been installed, regenerate the lock file (as long as --skip-lock was omitted)

This creates some opportunities for the environment and the lock file to become inconsistent with each other:

  • the individual install commands don't use the lock file as a source of constraints, so they may install different versions of dependencies from those that would be installed when using the lock file
  • because the installation happens before the lock file generation, there's a race condition where the lock file generation may see a newer release than the install operation did
  • installing individual packages will resolve environment markers inside the virtual environment, while lock file generation will resolve them outside the virtual environment (this point means we may need to resolve Lock file differs depending on Python version of Pipenv installation #857 before we can safely make the logic change proposed below)

The proposed change to the installation logic would be as follows:

  • for each package requested on the command line:
    • check that the requested package is available from one of the source repositories listed in Pipfile
    • if it's missing, abort the operation
    • don't make any changes to Pipfile until all the requested packages have been checked
  • for each package requested on the command line:
    • update Pipfile with the new or revised dependency
  • update Pipfile with the new pre-release setting (if --pre is specified)
  • assuming all requested packages are available and --skip-lock was omitted, regenerate the lock file
  • run do_init (just as the command already does for the case where no packages are specified, which means this approach also already handles the --ignore-pipfile and --skip-lock options)
@ncoghlan ncoghlan self-assigned this Feb 24, 2018
@ncoghlan ncoghlan added the Status: Requires Approval This issue requires additional approval to move forward. label Feb 24, 2018
@kennethreitz
Copy link
Contributor

my only concern with this is something shouldn't be added to the lockfile if it can't be installed.

@ncoghlan
Copy link
Member Author

The simplest alternative that comes to mind would be to leave the current logic alone, and just add the do_init call at the end after the lock file is updated: in the typical case, that call would be a no-op, but in the cases where a version discrepancy had arisen, it should fix it implicitly as part of the command.

If discrepancies happen frequently enough for users to complain about the extra output from the sync operation, then it may also be feasible to convert Pipfile.lock to a constraints file that gets passed to the install command.

@kennethreitz kennethreitz added the Type: Enhancement 💡 This is a feature or enhancement request. label Feb 24, 2018
@kennethreitz
Copy link
Contributor

Sounds like a plan :)

kennethreitz added a commit that referenced this issue Feb 24, 2018
Signed-off-by: Kenneth Reitz <me@kennethreitz.org>
@ncoghlan
Copy link
Member Author

@kennethreitz Are there any other parameters that might need to be passed to the new do_init call? The one earlier in the function for the "no arguments" case seems to pass down pretty much every parameter that do_init accepts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Requires Approval This issue requires additional approval to move forward. Type: Enhancement 💡 This is a feature or enhancement request.
Projects
None yet
Development

No branches or pull requests

2 participants