Skip to content

Commit c03530d

Browse files
committed
warn before overwriting files owned by previously installed wheels
Before installing a wheel, look for other distributions that have also written files into the same top level directory. Warn if any of the same files will be modified by the new wheel. Addresses #4625 Signed-off-by: Doug Hellmann <[email protected]>
1 parent daff811 commit c03530d

File tree

3 files changed

+156
-0
lines changed

3 files changed

+156
-0
lines changed

news/4625.bugfix

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Add a FutureWarning when installing a wheel will overwrite a file
2+
owned by another distribution.

src/pip/_internal/operations/install/wheel.py

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from pip._vendor import pkg_resources
2424
from pip._vendor.distlib.scripts import ScriptMaker
2525
from pip._vendor.distlib.util import get_export_entry
26+
from pip._vendor.packaging.utils import canonicalize_name
2627
from pip._vendor.six import StringIO
2728

2829
from pip._internal.exceptions import InstallationError
@@ -283,6 +284,98 @@ def make(self, specification, options=None):
283284
return super(PipScriptMaker, self).make(specification, options)
284285

285286

287+
def _get_file_owners(
288+
lib_dir, # type: str
289+
top_level, # type: str
290+
ignore_name, # type: str
291+
):
292+
# type: (...) -> Dict[str, List[str]]
293+
"""Return dict mapping distributions to files under a top level directory
294+
295+
Look through lib_dir for distributions that own files under the
296+
top_level directory specified. Skip distributions that match the
297+
ignore_name. Return a dict mapping filenames to the distributions
298+
that own them.
299+
300+
"""
301+
file_owners = {} # type: Dict[str, List[str]]
302+
existing_env = pkg_resources.Environment([lib_dir])
303+
canonical_name = canonicalize_name(ignore_name)
304+
for existing_dist_name in existing_env:
305+
for d in existing_env[existing_dist_name]:
306+
if canonicalize_name(d.project_name) == canonical_name:
307+
continue
308+
309+
existing_info_dir = os.path.join(
310+
lib_dir,
311+
'{}-{}.dist-info'.format(d.project_name, d.version),
312+
)
313+
314+
top_level_path = os.path.join(existing_info_dir, 'top_level.txt')
315+
if not os.path.exists(top_level_path):
316+
continue
317+
with open(top_level_path, 'r') as f:
318+
existing_top_level = f.read().strip()
319+
if existing_top_level != top_level:
320+
continue
321+
322+
record_path = os.path.join(existing_info_dir, 'RECORD')
323+
if not os.path.exists(record_path):
324+
continue
325+
with open(record_path, **csv_io_kwargs('r')) as record_file:
326+
for row in csv.reader(record_file):
327+
file_owners.setdefault(row[0], []).append(str(d))
328+
329+
return file_owners
330+
331+
332+
def _report_file_owner_conflicts(
333+
lib_dir, # type: str
334+
name, # type: str
335+
source_dir, # type: str
336+
info_dir, # type: str
337+
):
338+
# type: (...) -> None
339+
"""Report files owned by other distributions that are being overwritten.
340+
341+
Scan the lib_dir for distributions that own files under the same
342+
top level directory as the wheel being installed and report any
343+
files owned by those other distributions that are going to be
344+
overwritten.
345+
346+
"""
347+
installing_top_level_path = os.path.join(
348+
source_dir, info_dir, 'top_level.txt')
349+
if not os.path.exists(installing_top_level_path):
350+
# We cannot determine the top level directory, so there is no
351+
# point in continuing.
352+
return
353+
354+
with open(installing_top_level_path, 'r') as fd:
355+
installing_top_level = fd.read().strip()
356+
files_from_other_owners = _get_file_owners(
357+
lib_dir, installing_top_level, name)
358+
if not files_from_other_owners:
359+
# Nothing else owns files under this top level directory, so
360+
# we don't need to scan the source.
361+
return
362+
363+
for dir, subdirs, files in os.walk(source_dir):
364+
basedir = dir[len(source_dir):].lstrip(os.path.sep)
365+
for f in files:
366+
partial_src = os.path.join(basedir, f)
367+
if partial_src not in files_from_other_owners:
368+
# There are no other owners for this file.
369+
continue
370+
destfile = os.path.join(lib_dir, basedir, f)
371+
for owner in files_from_other_owners[partial_src]:
372+
warnings.warn(
373+
'Overwriting or removing {} for {} '
374+
'which is also owned by {}'.format(
375+
destfile, name, owner),
376+
FutureWarning)
377+
378+
286379
def install_unpacked_wheel(
287380
name, # type: str
288381
wheeldir, # type: str
@@ -415,6 +508,13 @@ def clobber(
415508
changed = fixer(destfile)
416509
record_installed(srcfile, destfile, changed)
417510

511+
# Look for packages containing files that are already using the
512+
# same toplevel directory as the wheel we are installing but that
513+
# have a different dist name. Do this before calling clobber(), so
514+
# that when the warning is eventually changed to a hard error no
515+
# partial installation occurs.
516+
_report_file_owner_conflicts(lib_dir, name, source, info_dir)
517+
418518
clobber(source, lib_dir, True)
419519

420520
dest_info_dir = os.path.join(lib_dir, info_dir)

tests/functional/test_install_wheel.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,3 +587,57 @@ def test_wheel_install_fails_with_badly_encoded_metadata(script):
587587
assert "Error decoding metadata for" in result.stderr
588588
assert "simple-0.1.0-py2.py3-none-any.whl" in result.stderr
589589
assert "METADATA" in result.stderr
590+
591+
592+
def test_report_file_owner_conflicts(script, tmpdir):
593+
"""
594+
Test installing from a wheel that wants to overwrite
595+
files owned by another wheel to ensure that a warning
596+
is reported and that the second wheel does overwrite
597+
the first.
598+
"""
599+
from tests.lib import TestFailure
600+
601+
make_wheel_with_file(
602+
name="wheel1",
603+
version="1.0",
604+
extra_files={
605+
'thetop/a.py': '# from wheel1',
606+
},
607+
extra_metadata_files={
608+
'top_level.txt': 'thetop',
609+
},
610+
).save_to_dir(tmpdir)
611+
612+
make_wheel_with_file(
613+
name="wheel2",
614+
version="2.0",
615+
extra_files={
616+
'thetop/a.py': '# from wheel2',
617+
},
618+
extra_metadata_files={
619+
'top_level.txt': 'thetop',
620+
},
621+
).save_to_dir(tmpdir)
622+
623+
result = script.pip(
624+
'install', 'wheel1', 'wheel2', '--no-index',
625+
'--find-links', tmpdir,
626+
)
627+
628+
# FIXME: Both wheels should be installed, for now. In the future
629+
# when the warning is changed to a hard error this part of the
630+
# test needs to be updated to show that only wheel1 is installed.
631+
with pytest.raises(TestFailure):
632+
result.assert_installed('wheel1')
633+
result.assert_installed('wheel2')
634+
635+
# FIXME: When the warning is changed to a hard error, wheel2 won't
636+
# be installed and the file contents will be those from wheel1.
637+
thetop = script.site_packages_path / 'thetop'
638+
assert (thetop / 'a.py').read_text() == '# from wheel2'
639+
640+
full_path = thetop / 'a.py'
641+
msg = 'Overwriting or removing {} for {} which is also owned by {}'.format(
642+
full_path, 'wheel2', 'wheel1 1.0')
643+
assert msg in result.stderr

0 commit comments

Comments
 (0)