Add command line flag to avoid overwriting grpc wheels (#318)

* Add command line flag that avoid overwite of grpc wheel

Add a flag --skip_exists that can let us avoid overiting a grpc wheel
built by home assistant builder with a grpc wheel from pypi.

* Use old python3.8 typing

* Make comparisions using set to avoid ordering issues

* Address lint errors found by tox

* Address black format issues

* Revert change to combine arguments to make code review easier

* Apply black format chagnes

* Remove --skip_exists and always use --skip_binary to remove local existing wheels

Author: allenporter

builder/__main__.py

@@ -15,6 +15,7 @@
     check_available_binary,
     create_wheels_folder,
     create_wheels_index,
+    remove_local_wheels,
 )
 from builder.pip import (
     build_wheels_local,
@@ -186,6 +187,17 @@ def builder(
             run_auditwheel(wheels_dir)
 
         fix_wheels_name(wheels_dir)
+
+        if skip_binary != ":none:":
+            # Some wheels that already exist should not be overwritten in case we replace with
+            # a wheel that came from pypi rather than a wheel built from source with extra flags.
+            # When --skip-binary and --skip-exists are set a wheel is only built from source once.
+            packages = extract_packages(requirement, requirement_diff)
+            constraints = parse_requirements(constraint) if constraint else []
+            remove_local_wheels(
+                wheels_index, skip_binary.split(","), packages + constraints, wheels_dir
+            )
+
         if not test:
             run_upload(upload, output, remote)
 

builder/infra.py

@@ -1,7 +1,7 @@
 """Create folder structure for index."""
 from pathlib import Path
 import re
-from typing import List, Set
+from typing import List, Set, Dict
 
 import requests
 
@@ -23,6 +23,29 @@ def create_wheels_index(base_index: str) -> str:
     return f"{base_index}/{alpine_version()}/{build_arch()}/"
 
 
+def create_package_map(packages: List[str]) -> Dict[str, str]:
+    """Create a dictionary from package base name to package and version string."""
+    results = {}
+    for package in packages.copy():
+        find = _RE_REQUIREMENT.match(package)
+        if not find:
+            continue
+        package = find["package"]
+        version = find["version"]
+        results[package] = f"{package}-{version}"
+    return results
+
+
+def check_existing_packages(index: str, package_map: Dict[str, str]) -> Set[str]:
+    """Return the set of package names that already exist in the index."""
+    available_data = requests.get(index, allow_redirects=True).text
+    found: Set[str] = set({})
+    for (binary, package) in package_map.items():
+        if package in available_data:
+            found.add(binary)
+    return found
+
+
 def check_available_binary(
     index: str, skip_binary: str, packages: List[str], constraints: List[str]
 ) -> str:
@@ -31,33 +54,50 @@ def check_available_binary(
         return skip_binary
 
     list_binary = skip_binary.split(",")
-    available_data = requests.get(index, allow_redirects=True).text
-
-    list_needed: Set[str] = set()
-    for binary in list_binary:
-        for package in packages + constraints:
-            if not package.startswith(binary):
-                continue
 
-            # Check more details
-            find = _RE_REQUIREMENT.match(package)
-            if not find:
-                raise ValueError(f"package requirement malformed: {package}")
+    # Map of package basename to the desired package version
+    package_map = create_package_map(packages + constraints)
 
-            # Check full name
-            if binary != find["package"]:
-                continue
-
-            # Process packages
-            name = f"{binary}-{find['version']}"
-            if name in available_data:
-                continue
-
-            # Ignore binary
-            print(f"Ignore Binary {package}: {name}", flush=True)
-            list_needed.add(binary)
+    # View of package map limited to packages in --skip-binary
+    binary_package_map = {}
+    for binary in list_binary:
+        if not (package := package_map.get(binary)):
+            print(
+                f"Skip binary '{binary}' not in packages/constraints; Can't determine desired version",
+                flush=True,
+            )
+            continue
+        binary_package_map[binary] = package
+
+    print(f"Checking if binaries already exist for packages {binary_package_map}")
+    list_found: Set[str] = check_existing_packages(index, binary_package_map)
+    print(f"Packages already exist: {list_found}")
+    list_needed = binary_package_map.keys() - list_found
 
     # Generate needed list of skip binary
     if not list_needed:
         return ":none:"
+
+    print(f"Will force binary build for {list_needed}")
     return ",".join(list_needed)
+
+
+def remove_local_wheels(
+    index: str,
+    skip_exists: List[str],
+    packages: List[str],
+    wheels_dir: Path,
+) -> str:
+    """Remove existing wheels if they already exist in the index to avoid syncing."""
+    package_map = create_package_map(packages)
+    binary_package_map = {
+        name: package_map[name] for name in skip_exists if name in package_map
+    }
+    print(f"Checking if binaries already exist for packages {binary_package_map}")
+    exists = check_existing_packages(index, binary_package_map)
+    for binary in exists:
+        package = binary_package_map[binary]
+        print(f"Found existing wheels for {binary}, removing local copy {package}")
+        for wheel in wheels_dir.glob(f"{package}-*.whl"):
+            print(f"Removing local wheel {wheel}")
+            wheel.unlink()

tests/test_infra.py

@@ -1,8 +1,7 @@
-"""Tests for infra module.
-
-pip module."""
+"""Tests for infra module."""
 import pytest
-from builder.infra import check_available_binary
+from pathlib import Path
+from builder.infra import check_available_binary, remove_local_wheels
 from unittest.mock import patch
 
 
@@ -147,3 +146,65 @@ def test_check_available_binary_for_missing_constraint() -> None:
         )
         == "grpcio"
     )
+
+
+@pytest.fixture
+def tmppath(tmpdir):
+    return Path(tmpdir)
+
+
+def test_remove_local_wheel(tmppath: Path) -> None:
+    """Test removing an existing wheel."""
+
+    p = tmppath / "google_cloud_pubsub-2.9.0-py2.py3-none-any.whl"
+    p.touch()
+    p = tmppath / "grpcio-1.31.0-cp39-none-any.whl"
+    p.touch()
+    assert {p.name for p in tmppath.glob("*.whl")} == {
+        "grpcio-1.31.0-cp39-none-any.whl",
+        "google_cloud_pubsub-2.9.0-py2.py3-none-any.whl",
+    }
+
+    remove_local_wheels(
+        TEST_INDEX_URL,
+        skip_exists=["grpcio"],
+        packages=[
+            "google_cloud_pubsub==2.9.0",
+            "grpcio==1.31.0",  # Exists in index
+        ],
+        wheels_dir=tmppath,
+    )
+
+    # grpc is removed
+    assert {p.name for p in tmppath.glob("*.whl")} == {
+        "google_cloud_pubsub-2.9.0-py2.py3-none-any.whl",
+    }
+
+
+def test_remove_local_wheel_preserves_newer(tmppath: Path) -> None:
+    """Test that the wheel is preserved when newer than in the index."""
+
+    p = tmppath / "google_cloud_pubsub-2.9.0-py2.py3-none-any.whl"
+    p.touch()
+    p = tmppath / "grpcio-1.43.0-cp39-none-any.whl"
+    p.touch()
+    assert {p.name for p in tmppath.glob("*.whl")} == {
+        "grpcio-1.43.0-cp39-none-any.whl",
+        "google_cloud_pubsub-2.9.0-py2.py3-none-any.whl",
+    }
+
+    remove_local_wheels(
+        TEST_INDEX_URL,
+        skip_exists=["grpcio"],
+        packages=[
+            "google_cloud_pubsub==2.9.0",
+            "grpcio==1.43.0",  # Newer than index
+        ],
+        wheels_dir=tmppath,
+    )
+
+    # grpc is removed
+    assert {p.name for p in tmppath.glob("*.whl")} == {
+        "grpcio-1.43.0-cp39-none-any.whl",
+        "google_cloud_pubsub-2.9.0-py2.py3-none-any.whl",
+    }