Skip to content

Commit

Permalink
Sync: Keep installed deps marked as to_install; Let pip sort it out
Browse files Browse the repository at this point in the history
Fixes #896

By including already-installed matching requirements in the to_install set,
dependencies of pinned packages will be installed,
after potentially being uninstalled,
regardless of environment state at time of sync.

The already-installed reqs will still not be reinstalled,
thanks to pip's own handling.

The resulting state is more consistent, predictable, and practical.

Modify tests to expect explicit requirements in the to_install set, to match.
  • Loading branch information
AndydeCleyre authored and webknjaz committed Oct 5, 2022
1 parent eff8476 commit 0cda3f0
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 10 deletions.
5 changes: 1 addition & 4 deletions piptools/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ def diff(
"""
requirements_lut = {diff_key_from_ireq(r): r for r in compiled_requirements}

satisfied = set() # holds keys
to_install = set() # holds InstallRequirement objects
to_uninstall = set() # holds keys

Expand All @@ -159,11 +158,9 @@ def diff(
key = key_from_req(dist)
if key not in requirements_lut or not requirements_lut[key].match_markers():
to_uninstall.add(key)
elif requirements_lut[key].specifier.contains(dist.version):
satisfied.add(key)

for key, requirement in requirements_lut.items():
if key not in satisfied and requirement.match_markers():
if requirement.match_markers():
to_install.add(requirement)

# Make sure to not uninstall any packages that should be ignored
Expand Down
12 changes: 6 additions & 6 deletions tests/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def test_diff_should_do_nothing():
reqs = [] # no requirements

to_install, to_uninstall = diff(reqs, installed)
assert to_install == set()
assert to_install == set(reqs)
assert to_uninstall == set()


Expand All @@ -136,7 +136,7 @@ def test_diff_should_uninstall(fake_dist):
reqs = []

to_install, to_uninstall = diff(reqs, installed)
assert to_install == set()
assert to_install == set(reqs)
assert to_uninstall == {"django"} # no version spec when uninstalling


Expand Down Expand Up @@ -200,7 +200,7 @@ def test_diff_leave_packaging_packages_alone(fake_dist, from_line):
reqs = [from_line("django==1.7")]

to_install, to_uninstall = diff(reqs, installed)
assert to_install == set()
assert to_install == set(reqs)
assert to_uninstall == {"first"}


Expand All @@ -221,7 +221,7 @@ def test_diff_leave_piptools_alone(fake_dist, from_line):
reqs = [from_line("django==1.7")]

to_install, to_uninstall = diff(reqs, installed)
assert to_install == set()
assert to_install == set(reqs)
assert to_uninstall == {"foobar"}


Expand All @@ -242,12 +242,12 @@ def test_diff_with_editable(fake_dist, from_editable):


def test_diff_with_matching_url_versions(fake_dist, from_line):
# if URL version is explicitly provided, use it to avoid reinstalling
# if URL version is explicitly provided, pip itself will avoid reinstalling
installed = [fake_dist("example==1.0")]
reqs = [from_line("file:///example.zip#egg=example==1.0")]

to_install, to_uninstall = diff(reqs, installed)
assert to_install == set()
assert to_install == set(reqs)
assert to_uninstall == set()


Expand Down

0 comments on commit 0cda3f0

Please sign in to comment.