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

Add a `pipenv sync` subcommand (remove `update`) #1463

Closed
ncoghlan opened this Issue Feb 22, 2018 · 28 comments

Comments

Projects
None yet
8 participants
@ncoghlan
Member

ncoghlan commented Feb 22, 2018

(Breaking this out as the first step towards #1255)

pipenv install is currently overloaded to handle a number of different tasks, including both editing Pipfile to add new dependencies, and updating the current environment to match the contents of Pipfile and/or Pipfile.lock. This overloading currently leads to some quirky behaviours, like those described in #1137 (where [packages] and [dev-packages] are resolved independently of each other).

To start resolving this, pipenv sync would be a new subcommand that works solely based on Pipfile.lock: if that's missing, the command will fail, with a direction to run pipenv lock first.

The basic behaviour would be similar to the current pipenv install --deploy --ignore-pipfile, but with [dev-packages] handling adjusted as follows:

  • when --dev is given, both [packages] and [dev-packages] will be installed, anything else will be uninstalled
  • when --no-dev given, only [packages] will be installed, anything else will be uninstalled
  • when neither is given, the default will be --keep-dev, which installs [packages], ensures versions match for any already installed [dev-packages], and uninstalls everything else

Other changes relative to pipenv install:

  • no --pre argument (only uses exact versions from Pipfile.lock)
  • no --skip-lock (only uses exact versions from Pipfile.lock)
  • no --ignore-pipfile (only uses exact versions from Pipfile.lock)
  • no -r or -c import options (only uses exact versions from Pipfile.lock)
  • always aborts if Pipfile.lock is missing or out of date
  • --deploy only adds the check of the Python version constraint
  • no --system option (at least for now)

Options shared with pipenv install:

  • --three/--two/--python: affect implicit venv creation as usual
  • --verbose/-h/--help: work the same way as for any other subcommand
@techalchemy

This comment has been minimized.

Show comment
Hide comment
@techalchemy

techalchemy Feb 22, 2018

Member

Thanks for detailing this, I feel there are a lot of critical elements to untangle resolution that are kind of scattered across a few issues. I think breaking this apart into smaller pieces and detailing a plan of action for each one will help guide both a badly needed refactor of certain elements as well as a reimplementation of some of this core logic.

If you are feeling ambitious, I wouldn't mind seeing some of these pieces in project form (though I might be the only one who likes to work that way), but from a very high-level we are talking about essentially:

  • Creating environments
  • Installing packages
    • 'Deploying' already-resolved dependency graphs
    • Updating dependencies (by either an only-as-needed or aggressive re-resolution strategy)
  • Resolving dependencies (constrained currently by the given system/platform/python)
    • Hash retrieval / comparison from APIs & hash generation

As far as pipenv sync, when you say uninstall, I assume you do mean this as opposed to pipenv --rm, which does a full replacement of the virtualenv -- presumably we would be comparing against pip freeze or something like that?

I see the utility for this separation internally, but I'm not convinced about implementing it as a completely separate command (but that leaves us with another argument for install which as you mention is quite cluttered and confusing) -- I just think we need to be confident this isn't going to be even more confusing to the average user when they encounter it in pipenv --help.

Member

techalchemy commented Feb 22, 2018

Thanks for detailing this, I feel there are a lot of critical elements to untangle resolution that are kind of scattered across a few issues. I think breaking this apart into smaller pieces and detailing a plan of action for each one will help guide both a badly needed refactor of certain elements as well as a reimplementation of some of this core logic.

If you are feeling ambitious, I wouldn't mind seeing some of these pieces in project form (though I might be the only one who likes to work that way), but from a very high-level we are talking about essentially:

  • Creating environments
  • Installing packages
    • 'Deploying' already-resolved dependency graphs
    • Updating dependencies (by either an only-as-needed or aggressive re-resolution strategy)
  • Resolving dependencies (constrained currently by the given system/platform/python)
    • Hash retrieval / comparison from APIs & hash generation

As far as pipenv sync, when you say uninstall, I assume you do mean this as opposed to pipenv --rm, which does a full replacement of the virtualenv -- presumably we would be comparing against pip freeze or something like that?

I see the utility for this separation internally, but I'm not convinced about implementing it as a completely separate command (but that leaves us with another argument for install which as you mention is quite cluttered and confusing) -- I just think we need to be confident this isn't going to be even more confusing to the average user when they encounter it in pipenv --help.

@ncoghlan

This comment has been minimized.

Show comment
Hide comment
@ncoghlan

ncoghlan Feb 22, 2018

Member

The Python precedent I'd cite here is pip-tools, which makes a very clear distinction between "update the lock file from the input file" (pip-compile) and "update the current environment to exactly match the lock file" (pip-sync, which upgrades, downgrades, installs, and uninstalls things as needed to make the environment match the specification). This is a good model, since it clearly separates the dependency management process into three phases:

  • define what you want/need (recorded in Pipfile)
  • resolve the full dependency tree (recorded in Pipfile.lock)
  • make a particular environment match the specification (shown by pipenv graph)

Right now, pipenv doesn't model that management process clearly at all - pipenv install and pipenv uninstall cover all three aspects (with a grab bag of arcane switches to turn different substeps on and off), and only the middle step is clearly exposed as its own subcommand (pipenv lock).

With pipenv sync added, then using pipenv install and pipenv uninstall would become entirely optional: you'd be free to instead just always edit Pipfile directly, use pipenv lock to update the lock file, then pipenv sync to get an environment to match that.

And then the later steps in #1255 would aim to ensure behavioural consistency by having install and uninstall actually run both lock and sync in order to make the change they make take effect locally.

Member

ncoghlan commented Feb 22, 2018

The Python precedent I'd cite here is pip-tools, which makes a very clear distinction between "update the lock file from the input file" (pip-compile) and "update the current environment to exactly match the lock file" (pip-sync, which upgrades, downgrades, installs, and uninstalls things as needed to make the environment match the specification). This is a good model, since it clearly separates the dependency management process into three phases:

  • define what you want/need (recorded in Pipfile)
  • resolve the full dependency tree (recorded in Pipfile.lock)
  • make a particular environment match the specification (shown by pipenv graph)

Right now, pipenv doesn't model that management process clearly at all - pipenv install and pipenv uninstall cover all three aspects (with a grab bag of arcane switches to turn different substeps on and off), and only the middle step is clearly exposed as its own subcommand (pipenv lock).

With pipenv sync added, then using pipenv install and pipenv uninstall would become entirely optional: you'd be free to instead just always edit Pipfile directly, use pipenv lock to update the lock file, then pipenv sync to get an environment to match that.

And then the later steps in #1255 would aim to ensure behavioural consistency by having install and uninstall actually run both lock and sync in order to make the change they make take effect locally.

@techalchemy

This comment has been minimized.

Show comment
Hide comment
@techalchemy

techalchemy Feb 22, 2018

Member

Perfect, thanks for your continuing work and patience on this @ncoghlan -- I think that description clarifies the distinction perfectly.

@kennethreitz, thoughts?

Member

techalchemy commented Feb 22, 2018

Perfect, thanks for your continuing work and patience on this @ncoghlan -- I think that description clarifies the distinction perfectly.

@kennethreitz, thoughts?

@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Feb 22, 2018

Contributor

this sounds a lot like what update does.

Contributor

kennethreitz commented Feb 22, 2018

this sounds a lot like what update does.

@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Feb 22, 2018

Contributor

Perhaps it could replace it.

Contributor

kennethreitz commented Feb 22, 2018

Perhaps it could replace it.

@ncoghlan

This comment has been minimized.

Show comment
Hide comment
@ncoghlan

ncoghlan Feb 22, 2018

Member

Based on the help text, I had assumed that pipenv update ignored the current state of the lock file and updated everything (including the lock file entries) to the latest version compatible with the Pipfile version specifications.

If it's actually an environment syncing operation that doesn't modify the lock file at all, then I'd suggest renaming it, as I doubt I'd be the only person to interpret it as "update the lock file and the current environment to the latest compatible version of everything".

Member

ncoghlan commented Feb 22, 2018

Based on the help text, I had assumed that pipenv update ignored the current state of the lock file and updated everything (including the lock file entries) to the latest version compatible with the Pipfile version specifications.

If it's actually an environment syncing operation that doesn't modify the lock file at all, then I'd suggest renaming it, as I doubt I'd be the only person to interpret it as "update the lock file and the current environment to the latest compatible version of everything".

@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Feb 22, 2018

Contributor

You are correct @ncoghlan. I suggest we remove it in favor of sync.

Contributor

kennethreitz commented Feb 22, 2018

You are correct @ncoghlan. I suggest we remove it in favor of sync.

@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Feb 22, 2018

Contributor

I think having both would be confusing.

Contributor

kennethreitz commented Feb 22, 2018

I think having both would be confusing.

@techalchemy techalchemy added this to the 10.0 milestone Feb 22, 2018

@ncoghlan

This comment has been minimized.

Show comment
Hide comment
@ncoghlan

ncoghlan Feb 22, 2018

Member

Yeah, we definitely wouldn't want both. I think it does change the nature of this RFE a bit though, as now I understand pipenv update a bit better (thanks!), I think this suggestion breaks down into two different pieces:

  1. Rename pipenv update to pipenv sync
  2. Make the renamed pipenv sync more efficient by borrowing implementation ideas from pip-sync
Member

ncoghlan commented Feb 22, 2018

Yeah, we definitely wouldn't want both. I think it does change the nature of this RFE a bit though, as now I understand pipenv update a bit better (thanks!), I think this suggestion breaks down into two different pieces:

  1. Rename pipenv update to pipenv sync
  2. Make the renamed pipenv sync more efficient by borrowing implementation ideas from pip-sync
@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Feb 22, 2018

Contributor

+100

we can alias pipenv update to still work too, but not list it as a command.

Contributor

kennethreitz commented Feb 22, 2018

+100

we can alias pipenv update to still work too, but not list it as a command.

@kennethreitz kennethreitz changed the title from Add a `pipenv sync` subcommand to Add a `pipenv sync` subcommand (remove `update`) Feb 23, 2018

@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Feb 23, 2018

Contributor

working on this now

Contributor

kennethreitz commented Feb 23, 2018

working on this now

@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Feb 23, 2018

Contributor

also adding clean.

Contributor

kennethreitz commented Feb 23, 2018

also adding clean.

@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Feb 23, 2018

Contributor

this is done

Contributor

kennethreitz commented Feb 23, 2018

this is done

@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Feb 23, 2018

Contributor

should be released shortly, as v10.0.0

Contributor

kennethreitz commented Feb 23, 2018

should be released shortly, as v10.0.0

@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Feb 23, 2018

Contributor

released! 🍰

Contributor

kennethreitz commented Feb 23, 2018

released! 🍰

@amit-traiana

This comment has been minimized.

Show comment
Hide comment
@amit-traiana

amit-traiana May 3, 2018

Just a quick clarification here, based on trial and error (as it seems like the documentation doesn't mentioned it):

pipenv lock - Will create a .lock file based on the specification on the pipfile. It won't install anything on the virtual environment.

pipenv sync - Reads the pipfilefile, create/updates the pipfile.lock file, and install the packages.

pipenv update - According to the --help information, it runs pipenv lock and then pipenv sync. But isn't it exactly what pipenv sync already do?

pipenv install - If package name is being specific, it will add this package to the pipfile and install it (sort if like manually adding the package name to pipfile and running pipenv sync). I'm not sure what pipenv install does without package name, because it looks like it doing the exact thing like pipenv sync?

Thanks!

amit-traiana commented May 3, 2018

Just a quick clarification here, based on trial and error (as it seems like the documentation doesn't mentioned it):

pipenv lock - Will create a .lock file based on the specification on the pipfile. It won't install anything on the virtual environment.

pipenv sync - Reads the pipfilefile, create/updates the pipfile.lock file, and install the packages.

pipenv update - According to the --help information, it runs pipenv lock and then pipenv sync. But isn't it exactly what pipenv sync already do?

pipenv install - If package name is being specific, it will add this package to the pipfile and install it (sort if like manually adding the package name to pipfile and running pipenv sync). I'm not sure what pipenv install does without package name, because it looks like it doing the exact thing like pipenv sync?

Thanks!

@uranusjr

This comment has been minimized.

Show comment
Hide comment
@uranusjr

uranusjr May 3, 2018

Member

The issue here seems to be that sync now locks automatically. Maybe it shouldn’t?

update and install without arguments are unfavored, you can pretend they don’t exist; they are kept mainly for compatibility reasons.

Member

uranusjr commented May 3, 2018

The issue here seems to be that sync now locks automatically. Maybe it shouldn’t?

update and install without arguments are unfavored, you can pretend they don’t exist; they are kept mainly for compatibility reasons.

@amit-traiana

This comment has been minimized.

Show comment
Hide comment
@amit-traiana

amit-traiana May 3, 2018

Yes. I think sync should only sync your virtualenv according to the spec defined in the lock file and that's it. Here's what makes sense to me:

pipenv lock - Create a new .lock file or modify you current .lock file if your pipfile has a new package in it, or - you specifically asking for a version that does not aligned with the one currently in the lock file. That's something I would rarely do manually, as generated .lock file is never enough - I mostly would like to deploy the changes into my virtualenv.

pipenv sync - Install packages based on the Pipfile.lock specification (and pop an error if there is no .lock file). Because the recommendation is to push your .lock file into the repo, It something I would do when I clone a new app (so I can grab the needed dependencies into my virtualenv).

pipenv update - This runs lock with specific flag that upgrades ALL the dependencies (unless again the Pipfile specifically implies version number). After I have a new .lock file, it sync those changes. I would use it when I want to upgrades all my packages in one go.

pipenv update package_name - Same as the above, just for a specific package and not all of them. For example, I would like to upgrade my requests library version, and update the .lock file accordingly.

pipenv install - Calling the "normal" lock option (that does not update anything that is already specific in the .lock file, and then sync. It useful when I have a new project with new pipfile, where you want to generate lock for it and deploy the virtualenv before you start working.

pipenv install package_name - Manually add a new package to pipfile and then again generated a new "normal" lock and then sync.

tl;dr - It's not clear now if lock create a new .lock file by grabbing the latest versions, or modify an already exists .lock file based on the delta. Both are probably needed.

Hopefully it makes sense :-)

amit-traiana commented May 3, 2018

Yes. I think sync should only sync your virtualenv according to the spec defined in the lock file and that's it. Here's what makes sense to me:

pipenv lock - Create a new .lock file or modify you current .lock file if your pipfile has a new package in it, or - you specifically asking for a version that does not aligned with the one currently in the lock file. That's something I would rarely do manually, as generated .lock file is never enough - I mostly would like to deploy the changes into my virtualenv.

pipenv sync - Install packages based on the Pipfile.lock specification (and pop an error if there is no .lock file). Because the recommendation is to push your .lock file into the repo, It something I would do when I clone a new app (so I can grab the needed dependencies into my virtualenv).

pipenv update - This runs lock with specific flag that upgrades ALL the dependencies (unless again the Pipfile specifically implies version number). After I have a new .lock file, it sync those changes. I would use it when I want to upgrades all my packages in one go.

pipenv update package_name - Same as the above, just for a specific package and not all of them. For example, I would like to upgrade my requests library version, and update the .lock file accordingly.

pipenv install - Calling the "normal" lock option (that does not update anything that is already specific in the .lock file, and then sync. It useful when I have a new project with new pipfile, where you want to generate lock for it and deploy the virtualenv before you start working.

pipenv install package_name - Manually add a new package to pipfile and then again generated a new "normal" lock and then sync.

tl;dr - It's not clear now if lock create a new .lock file by grabbing the latest versions, or modify an already exists .lock file based on the delta. Both are probably needed.

Hopefully it makes sense :-)

@techalchemy

This comment has been minimized.

Show comment
Hide comment
@techalchemy

techalchemy May 3, 2018

Member

Sync is not supposed to lock. Why would we want that? The behaviors were separated very explicitly.

Member

techalchemy commented May 3, 2018

Sync is not supposed to lock. Why would we want that? The behaviors were separated very explicitly.

@ncoghlan

This comment has been minimized.

Show comment
Hide comment
@ncoghlan

ncoghlan May 3, 2018

Member

Aye, pipenv sync should never modify the lock file - it's meant to be strictly "make my environment match the lock file". If there's no lock file, it should just error out (if the lock file is out of date, printing a warning is fine, but the command should still work).

Member

ncoghlan commented May 3, 2018

Aye, pipenv sync should never modify the lock file - it's meant to be strictly "make my environment match the lock file". If there's no lock file, it should just error out (if the lock file is out of date, printing a warning is fine, but the command should still work).

@amit-traiana

This comment has been minimized.

Show comment
Hide comment
@amit-traiana

amit-traiana May 3, 2018

A bug maybe? That's how I re-produce it:

  1. Have a pipfile and pipefile.lock that are aligned.
  2. Add package to the pipfile (for example requests = "*")
  3. Run pipfile sync

The above updated the .lock file with the requests package information. I'm using version 11.10.1. If that's indeed now the proper behaviour, I'll open a separate bug.

amit-traiana commented May 3, 2018

A bug maybe? That's how I re-produce it:

  1. Have a pipfile and pipefile.lock that are aligned.
  2. Add package to the pipfile (for example requests = "*")
  3. Run pipfile sync

The above updated the .lock file with the requests package information. I'm using version 11.10.1. If that's indeed now the proper behaviour, I'll open a separate bug.

uranusjr added a commit that referenced this issue May 3, 2018

Check lockfile exists on sync, and don't update it
Matching the behaviour mentioned in:
#1463 (comment)

uranusjr added a commit that referenced this issue May 3, 2018

Check lockfile exists on sync, and don't update it
Matching the behaviour mentioned in:
#1463 (comment)

uranusjr added a commit that referenced this issue May 3, 2018

Check lockfile exists on sync, and don't update it
Matching the behaviour mentioned in:
#1463 (comment)

uranusjr added a commit that referenced this issue May 3, 2018

Check lockfile exists on sync, and don't update it
Matching the behaviour mentioned in:
#1463 (comment)

techalchemy added a commit that referenced this issue May 3, 2018

Check lockfile exists on sync, and don't update it
Matching the behaviour mentioned in:
#1463 (comment)
@amit-traiana

This comment has been minimized.

Show comment
Hide comment
@amit-traiana

amit-traiana May 7, 2018

Thanks for the quick fix. Will be testing it when a new version is released :)

amit-traiana commented May 7, 2018

Thanks for the quick fix. Will be testing it when a new version is released :)

@twig-openlearning

This comment has been minimized.

Show comment
Hide comment
@twig-openlearning

twig-openlearning May 9, 2018

Can I request that the docs be updated with command descriptions asap?

There's a bit of guess-work required to figure out what they each do, and hence I ended up where when trying to figure out what "sync" does.

twig-openlearning commented May 9, 2018

Can I request that the docs be updated with command descriptions asap?

There's a bit of guess-work required to figure out what they each do, and hence I ended up where when trying to figure out what "sync" does.

@gsemet

This comment has been minimized.

Show comment
Hide comment
@gsemet

gsemet May 9, 2018

Contributor

Maybe orienting the documentation toward a classic workflow example with correspondance between old and new behavior:

  • project bootstrap (?)
  • install for dev
  • add a dependency
  • update a dependency
  • update all dependencies (I usually does that every week, so I do not think ‘pipenv update’ should be deprecated)
  • install for prod (ie remove dev env, could be useful for Travis), maybe explicitly how to install within a docker
Contributor

gsemet commented May 9, 2018

Maybe orienting the documentation toward a classic workflow example with correspondance between old and new behavior:

  • project bootstrap (?)
  • install for dev
  • add a dependency
  • update a dependency
  • update all dependencies (I usually does that every week, so I do not think ‘pipenv update’ should be deprecated)
  • install for prod (ie remove dev env, could be useful for Travis), maybe explicitly how to install within a docker
@uranusjr

This comment has been minimized.

Show comment
Hide comment
@uranusjr

uranusjr May 9, 2018

Member

Contributions are always welcomed :) Here’s my typical workflow:

  • pipenv --python=... to bootstrap
  • pipenv install (or install --dev depending on the project)
  • Manage packages by either
    • pipenv install/uninstall [--dev] <package> to manage packages
    • Manually update Pipfile and then run pipenv install [--dev] without arguments
  • On another machine, run pipenv sync [--dev] after pulling in changes to keep the local env up-to-date
  • pipenv update to update locked dependencies from time to time to keep them modern
    • Other machines would see the lockfile update when pulling in changes, and run pipenv sync [--dev]
  • Prod can either be treated as “just another computer” and simply use the usual workflow, or use the --deploy flag to install/sync globally (without a virtual environment).
Member

uranusjr commented May 9, 2018

Contributions are always welcomed :) Here’s my typical workflow:

  • pipenv --python=... to bootstrap
  • pipenv install (or install --dev depending on the project)
  • Manage packages by either
    • pipenv install/uninstall [--dev] <package> to manage packages
    • Manually update Pipfile and then run pipenv install [--dev] without arguments
  • On another machine, run pipenv sync [--dev] after pulling in changes to keep the local env up-to-date
  • pipenv update to update locked dependencies from time to time to keep them modern
    • Other machines would see the lockfile update when pulling in changes, and run pipenv sync [--dev]
  • Prod can either be treated as “just another computer” and simply use the usual workflow, or use the --deploy flag to install/sync globally (without a virtual environment).
@techalchemy

This comment has been minimized.

Show comment
Hide comment
@techalchemy

techalchemy May 9, 2018

Member

@gsemet nobody is deprecating update. Why are you raising old conversations about things that have been put to rest? But your entire post can be summarized with pipenv install [--dev] [package] and pipenv update

Member

techalchemy commented May 9, 2018

@gsemet nobody is deprecating update. Why are you raising old conversations about things that have been put to rest? But your entire post can be summarized with pipenv install [--dev] [package] and pipenv update

@gsemet

This comment has been minimized.

Show comment
Hide comment
@gsemet

gsemet May 9, 2018

Contributor

My bad, I certainly misunderstood something

Contributor

gsemet commented May 9, 2018

My bad, I certainly misunderstood something

@frostming

This comment has been minimized.

Show comment
Hide comment
@frostming

frostming May 21, 2018

Contributor

@twig-openlearning It was already made possible by #1846, but obviously readthedocs is still using the old dependencies somehow.

Please update the doc dependencies.

Contributor

frostming commented May 21, 2018

@twig-openlearning It was already made possible by #1846, but obviously readthedocs is still using the old dependencies somehow.

Please update the doc dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment