installer: do not fail on invalid wheels, print only a warning (#7694)

radoering · web-flow · commit c2a7a8d6ea41 · 2023-03-31T21:39:33.000+02:00
diff --git a/src/poetry/installation/executor.py b/src/poetry/installation/executor.py
@@ -207,6 +207,14 @@ def execute(self, operations: list[Operation]) -> int:
 
         for warning in self._yanked_warnings:
             self._io.write_error_line(f"<warning>Warning: {warning}</warning>")
+        for path, issues in self._wheel_installer.invalid_wheels.items():
+            formatted_issues = "\n".join(issues)
+            warning = (
+                f"Validation of the RECORD file of {path.name} failed."
+                " Please report to the maintainers of that package so they can fix"
+                f" their build process. Details:\n{formatted_issues}\n"
+            )
+            self._io.write_error_line(f"<warning>Warning: {warning}</warning>")
 
         return 1 if self._shutdown else 0
 
diff --git a/src/poetry/installation/wheel_installer.py b/src/poetry/installation/wheel_installer.py
@@ -10,6 +10,7 @@
 from installer import install
 from installer.destinations import SchemeDictionaryDestination
 from installer.sources import WheelFile
+from installer.sources import _WheelFileValidationError
 
 from poetry.__version__ import __version__
 from poetry.utils._compat import WINDOWS
@@ -93,12 +94,17 @@ def __init__(self, env: Env) -> None:
             schemes, interpreter=str(self._env.python), script_kind=script_kind
         )
 
+        self.invalid_wheels: dict[Path, list[str]] = {}
+
     def enable_bytecode_compilation(self, enable: bool = True) -> None:
         self._destination.bytecode_optimization_levels = (-1,) if enable else ()
 
     def install(self, wheel: Path) -> None:
         with WheelFile.open(wheel) as source:
-            source.validate_record()
+            try:
+                source.validate_record()
+            except _WheelFileValidationError as e:
+                self.invalid_wheels[wheel] = e.issues
             install(
                 source=source,
                 destination=self._destination.for_source(source),
diff --git a/tests/fixtures/distributions/demo_invalid_record-0.1.0-py2.py3-none-any.whl b/tests/fixtures/distributions/demo_invalid_record-0.1.0-py2.py3-none-any.whl
diff --git a/tests/fixtures/distributions/demo_invalid_record2-0.1.0-py2.py3-none-any.whl b/tests/fixtures/distributions/demo_invalid_record2-0.1.0-py2.py3-none-any.whl
diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py
@@ -148,9 +148,9 @@ def callback(
         )
 
         if not fixture.exists():
-            if name == "demo-0.1.0.tar.gz":
-                fixture = fixture_dir("distributions") / "demo-0.1.0.tar.gz"
-            else:
+            fixture = fixture_dir("distributions") / name
+
+            if not fixture.exists():
                 fixture = (
                     fixture_dir("distributions") / "demo-0.1.0-py2.py3-none-any.whl"
                 )
@@ -337,6 +337,66 @@ def test_execute_prints_warning_for_yanked_package(
         assert error.count("yanked") == 0
 
 
+def test_execute_prints_warning_for_invalid_wheels(
+    config: Config,
+    pool: RepositoryPool,
+    io: BufferedIO,
+    tmp_dir: str,
+    mock_file_downloads: None,
+    env: MockEnv,
+):
+    config.merge({"cache-dir": tmp_dir})
+
+    executor = Executor(env, pool, config, io)
+
+    base_url = "https://files.pythonhosted.org/"
+    wheel1 = "demo_invalid_record-0.1.0-py2.py3-none-any.whl"
+    wheel2 = "demo_invalid_record2-0.1.0-py2.py3-none-any.whl"
+    return_code = executor.execute(
+        [
+            Install(
+                Package(
+                    "demo-invalid-record",
+                    "0.1.0",
+                    source_type="url",
+                    source_url=f"{base_url}/{wheel1}",
+                )
+            ),
+            Install(
+                Package(
+                    "demo-invalid-record2",
+                    "0.1.0",
+                    source_type="url",
+                    source_url=f"{base_url}/{wheel2}",
+                )
+            ),
+        ]
+    )
+
+    warning1 = f"""\
+<warning>Warning: Validation of the RECORD file of {wheel1} failed.\
+ Please report to the maintainers of that package so they can fix their build process.\
+ Details:
+In .*?{wheel1}, demo/__init__.py is not mentioned in RECORD
+In .*?{wheel1}, demo_invalid_record-0.1.0.dist-info/WHEEL is not mentioned in RECORD
+"""
+
+    warning2 = f"""\
+<warning>Warning: Validation of the RECORD file of {wheel2} failed.\
+ Please report to the maintainers of that package so they can fix their build process.\
+ Details:
+In .*?{wheel2}, hash / size of demo_invalid_record2-0.1.0.dist-info/METADATA didn't\
+ match RECORD
+"""
+
+    output = io.fetch_output()
+    error = io.fetch_error()
+    assert return_code == 0, f"\noutput: {output}\nerror: {error}\n"
+    assert re.match(f"{warning1}\n{warning2}", error) or re.match(
+        f"{warning2}\n{warning1}", error
+    ), error
+
+
 def test_execute_shows_skipped_operations_if_verbose(
     config: Config,
     pool: RepositoryPool,
diff --git a/tests/utils/test_cache.py b/tests/utils/test_cache.py
@@ -286,7 +286,9 @@ def test_get_cached_archives_for_link(
     )
 
     assert archives
-    assert set(archives) == set(distributions.glob("demo-0.1.*"))
+    assert set(archives) == set(distributions.glob("*.whl")) | set(
+        distributions.glob("*.tar.gz")
+    )
 
 
 @pytest.mark.parametrize(

Original file line number	Diff line number	Diff line change
`@@ -286,7 +286,9 @@ def test_get_cached_archives_for_link(`
`286`	`286`	`)`
`287`	`287`
`288`	`288`	`assert archives`
`289`		`- assert set(archives) == set(distributions.glob("demo-0.1.*"))`
	`289`	`+ assert set(archives) == set(distributions.glob("*.whl")) \| set(`
	`290`	`+ distributions.glob("*.tar.gz")`
	`291`	`+ )`
`290`	`292`
`291`	`293`
`292`	`294`	`@pytest.mark.parametrize(`