Verify dependencies before uploading a package (#1)

Co-authored-by: Ivan Levkivskyi <ilevkivskyi@dropbox.com>

Author: ilevkivskyi

.github/workflows/check_scripts.yml

@@ -13,7 +13,7 @@ jobs:
       - uses: actions/checkout@v2
       - run: |
           pip install mypy
-          mypy -p scripts
+          mypy -p scripts -p tests
 
   tests:
     name: Run integration and unit tests
@@ -36,4 +36,4 @@ jobs:
         run: |
           pip install pytest toml requests setuptools wheel
           cd main
-          python -m pytest tests/*
+          python -m pytest tests

.github/workflows/force_update.yml

@@ -34,4 +34,4 @@ jobs:
           TWINE_PASSWORD: ${{ secrets.TYPESHED_BOT_API_TOKEN }}
         run: |
           cd main
-          python -m scripts.upload_some ../typeshed ${{ github.event.inputs.distribution }}
+          python -m scripts.upload_some ../typeshed ${{ github.event.inputs.distribution }} data/uploaded_packages.txt

.github/workflows/update_stubs.yml

@@ -34,7 +34,7 @@ jobs:
           TWINE_PASSWORD: ${{ secrets.TYPESHED_BOT_API_TOKEN }}
         run: |
           cd main
-          python -m scripts.upload_changed ../typeshed $(cat data/last_typeshed_commit.sha1) --dry-run
+          python -m scripts.upload_changed ../typeshed $(cat data/last_typeshed_commit.sha1) data/uploaded_packages.txt --dry-run
           (cd ../typeshed; git rev-parse HEAD) > data/last_typeshed_commit.sha1
           if [ -z "$(git status --porcelain)" ]; then
               exit 0;

data/uploaded_packages.txt

@@ -0,0 +1,9 @@
+types-Routes
+types-attrs
+types-enum34
+types-first
+types-mypy-extensions
+types-six
+types-toml
+types-typed-ast
+types-typing-extensions

scripts/build_wheel.py

@@ -12,6 +12,8 @@
 The types stubs live in https://github.com/python/typeshed/tree/master/stubs,
 all fixes for types and metadata should be contributed there, see
 https://github.com/python/typeshed/blob/master/CONTRIBUTING.md for more details.
+
+This file also contains some helper functions related to wheel validation and upload.
 """
 
 import argparse
@@ -20,8 +22,12 @@
 import shutil
 import tempfile
 import subprocess
+from collections import defaultdict
+from functools import cmp_to_key
 from textwrap import dedent
-from typing import List, Dict, Any, Tuple
+from typing import List, Dict, Any, Tuple, Set
+
+from scripts import get_version
 
 import toml
 
@@ -66,6 +72,11 @@
 """).lstrip()
 
 
+def strip_types_prefix(dependency: str) -> str:
+    assert dependency.startswith("types-"), "Currently only dependencies on stub packages are supported"
+    return dependency[len("types-"):]
+
+
 def find_stub_files(top: str) -> List[str]:
     """Find all stub files for a given package, relative to package root.
 
@@ -165,6 +176,82 @@ def collect_setup_entries(
     return packages, package_data
 
 
+def verify_dependency(typeshed_dir: str, dependency: str, uploaded: str) -> None:
+    """Verify this is a valid dependency, i.e. a stub package uploaded by us."""
+    known_distributions = set(os.listdir(os.path.join(typeshed_dir, THIRD_PARTY_NAMESPACE)))
+    assert ";" not in dependency, "Semicolons in dependencies are not supported"
+    dependency = get_version.strip_dep_version(dependency)
+    assert strip_types_prefix(dependency) in known_distributions, "Only dependencies on typeshed stubs are allowed"
+    with open(uploaded) as f:
+        uploaded_distributions = set(f.read().splitlines())
+
+    msg = f"{dependency} looks like a foreign distribution."
+    uploaded_distributions_lower = [d.lower() for d in uploaded_distributions]
+    if dependency not in uploaded_distributions and dependency.lower() in uploaded_distributions_lower:
+        msg += " Note: list is case sensitive"
+    assert dependency in uploaded_distributions, msg
+
+
+def update_uploaded(uploaded: str, distribution: str) -> None:
+    with open(uploaded) as f:
+        current = set(f.read().splitlines())
+    if f"types-{distribution}" not in current:
+        with open(uploaded, "w") as f:
+            f.writelines(sorted(current | {f"types-{distribution}"}))
+
+
+def make_dependency_map(typeshed_dir: str, distributions: List[str]) -> Dict[str, Set[str]]:
+    """Return relative dependency map among distributions.
+
+    Important: this only includes dependencies *within* the given
+    list of distributions.
+    """
+    result: Dict[str, Set[str]] = {d: set() for d in distributions}
+    for distribution in distributions:
+        data = read_matadata(
+            os.path.join(typeshed_dir, THIRD_PARTY_NAMESPACE, distribution, META)
+        )
+        for dependency in data.get("requires", []):
+            dependency = strip_types_prefix(get_version.strip_dep_version(dependency))
+            if dependency in distributions:
+                result[distribution].add(dependency)
+    return result
+
+
+def transitive_deps(dep_map: Dict[str, Set[str]]) -> Dict[str, Set[str]]:
+    """Propagate dependencies to compute a transitive dependency map.
+
+    Note: this algorithm is O(N**2) in general case, but we don't worry,
+    because N is small (less than 1000). So it will take few seconds at worst,
+    while building/uploading 1000 packages will take minutes.
+    """
+    transitive: Dict[str, Set[str]] = defaultdict(set)
+    for distribution in dep_map:
+        to_add = {distribution}
+        while to_add:
+            new = to_add.pop()
+            extra = dep_map[new]
+            transitive[distribution] |= extra
+            assert distribution not in transitive[distribution], f"Cyclic dependency {distribution} -> {distribution}"
+            to_add |= extra
+    return transitive
+
+
+def sort_by_dependency(dep_map: Dict[str, Set[str]]) -> List[str]:
+    """Sort distributions by dependency order (those depending on nothing appear first)."""
+    trans_map = transitive_deps(dep_map)
+
+    def compare(d1: str, d2: str) -> int:
+        if d1 in trans_map[d2]:
+            return -1
+        if d2 in trans_map[d1]:
+            return 1
+        return 0
+
+    # Return independent packages sorted by name for stability.
+    return sorted(sorted(dep_map), key=cmp_to_key(compare))
+
+
 def generate_setup_file(typeshed_dir: str, distribution: str, increment: str) -> str:
     """Auto-generate a setup.py file for given distribution using a template."""
     base_dir = os.path.join(typeshed_dir, THIRD_PARTY_NAMESPACE, distribution)
@@ -178,17 +265,11 @@ def generate_setup_file(typeshed_dir: str, distribution: str, increment: str) ->
         packages += py2_packages
         package_data.update(py2_package_data)
     version = metadata["version"]
-    requires = metadata.get("requires", [])
-    known_distributions = set(os.listdir(os.path.join(typeshed_dir, THIRD_PARTY_NAMESPACE)))
-    for dependency in requires:
-        assert dependency.startswith("types-"), "Only dependencies on stub packages are allowed"
-        dep_name = dependency[len("types-"):]
-        assert dep_name in known_distributions, "Only dependencies on typeshed stubs are allowed"
     assert version.count(".") == 1, f"Version must be major.minor, not {version}"
     return SETUP_TEMPLATE.format(
         distribution=distribution,
         version=f"{version}.{increment}",
-        requires=requires,
+        requires=metadata.get("requires", []),
         packages=packages,
         package_data=package_data,
     )

scripts/get_version.py

@@ -5,6 +5,9 @@
 If the given version was never uploaded, this will return -1. See
 https://github.com/python/typeshed/blob/master/README.md for details
 on stub versioning.
+
+This file also contains some helper functions related to querying
+distribution information.
 """
 
 import argparse
@@ -34,6 +37,30 @@ def read_base_version(typeshed_dir: str, distribution: str) -> str:
     return data["version"]
 
 
+def strip_dep_version(dependency: str) -> str:
+    """Strip a possible version suffix, e.g. types-six>=0.1.4 -> types-six."""
+    dep_version_pos = len(dependency)
+    for pos, c in enumerate(dependency):
+        if c in "=<>":
+            dep_version_pos = pos
+            break
+    return dependency[:dep_version_pos]
+
+
+def check_exists(distribution: str) -> bool:
+    """Check if any version of this *stub* distribution has ben ever uploaded."""
+    url = URL_TEMPLATE.format(distribution)
+    retry_strategy = Retry(total=RETRIES, status_forcelist=RETRY_ON)
+    with requests.Session() as session:
+        session.mount("https://", HTTPAdapter(max_retries=retry_strategy))
+        resp = session.get(url, timeout=TIMEOUT)
+    if resp.ok:
+        return True
+    if resp.status_code == 404:
+        return False
+    raise ValueError("Error while verifying existence")
+
+
 def main(typeshed_dir: str, distribution: str, version: Optional[str]) -> int:
     """A simple function to get version increment of a third-party stub package.
 

scripts/upload_changed.py

@@ -1,11 +1,12 @@
 """
 Entry point for scheduled GitHub auto-upload action.
 
-This does three things:
+This does following things:
 * Reads the list of stub packages modified since last commit in typeshed
 * Checks what is the current stub version increment for each package on PyPI
 * Bumps the increment, builds and uploads (unless run with --dry-run) each
   new package to PyPI
+* Verifies validity of stub dependencies, and updates known dependencies if needed
 """
 
 import argparse
@@ -17,24 +18,34 @@
 from scripts import get_changed
 
 
-def main(typeshed_dir: str, commit: str, dry_run: bool = False) -> None:
+def main(typeshed_dir: str, commit: str, uploaded: str, dry_run: bool = False) -> None:
     """Upload stub typeshed packages modified since commit."""
     changed = get_changed.main(typeshed_dir, commit)
-    for distribution in changed:
+    # Sort by dependency to prevent depending on foreign distributions.
+    to_upload = build_wheel.sort_by_dependency(
+        build_wheel.make_dependency_map(typeshed_dir, changed)
+    )
+    for distribution in to_upload:
         # Setting base version to None, so it will be read from current METADATA.toml.
         increment = get_version.main(typeshed_dir, distribution, None)
         increment += 1
         temp_dir = build_wheel.main(typeshed_dir, distribution, increment)
         if dry_run:
             print(f"Would upload: {distribution}, increment {increment}")
             continue
+        for dependency in build_wheel.read_matadata(
+            os.path.join(typeshed_dir, build_wheel.THIRD_PARTY_NAMESPACE, distribution)
+        ).get("requires", []):
+            build_wheel.verify_dependency(typeshed_dir, dependency, uploaded)
         subprocess.run(["twine", "upload", os.path.join(temp_dir, "*")], check=True)
+        build_wheel.update_uploaded(uploaded, distribution)
 
 
 if __name__ == "__main__":
     parser = argparse.ArgumentParser()
     parser.add_argument("typeshed_dir", help="Path to typeshed checkout directory")
     parser.add_argument("previous_commit", help="Previous typeshed commit for which we performed upload")
+    parser.add_argument("uploaded", help="Previously uploaded packages to validate dependencies")
     parser.add_argument("--dry-run", action="store_true", help="Should we perform a dry run (don't actually upload)")
     args = parser.parse_args()
-    main(args.typeshed_dir, args.previous_commit, args.dry_run)
+    main(args.typeshed_dir, args.previous_commit, args.uploaded, args.dry_run)

scripts/upload_some.py

@@ -1,10 +1,11 @@
 """
 Entry point for manual GitHub upload action.
 
-This does three things:
+This does following things:
 * Finds all distributions with names that match the pattern provided
 * Checks what is the current stub version increment for each package on PyPI
 * Bumps the increment, builds and uploads the each new package to PyPI
+* Verifies validity of stub dependencies, and updates known dependencies if needed
 """
 
 import argparse
@@ -16,22 +17,33 @@
 from scripts import build_wheel
 
 
-def main(typeshed_dir: str, pattern: str) -> None:
+def main(typeshed_dir: str, pattern: str, uploaded: str) -> None:
     """Force upload typeshed stub packages to PyPI."""
     compiled = re.compile(f"^{pattern}$")  # force exact matches
-    for distribution in os.listdir(os.path.join(typeshed_dir, "stubs")):
-        if not re.match(compiled, distribution):
-            continue
+    matching = [
+        d for d in os.listdir(os.path.join(typeshed_dir, "stubs")) if re.match(compiled, d)
+    ]
+    # Sort by dependency to prevent depending on foreign distributions.
+    to_upload = build_wheel.sort_by_dependency(
+        build_wheel.make_dependency_map(typeshed_dir, matching)
+    )
+    for distribution in to_upload:
         # Setting base version to None, so it will be read from current METADATA.toml.
         increment = get_version.main(typeshed_dir, distribution, version=None)
         increment += 1
+        for dependency in build_wheel.read_matadata(
+            os.path.join(typeshed_dir, build_wheel.THIRD_PARTY_NAMESPACE, distribution)
+        ).get("requires", []):
+            build_wheel.verify_dependency(typeshed_dir, dependency, uploaded)
         temp_dir = build_wheel.main(typeshed_dir, distribution, increment)
         subprocess.run(["twine", "upload", os.path.join(temp_dir, "*")], check=True)
+        build_wheel.update_uploaded(uploaded, distribution)
 
 
 if __name__ == "__main__":
     parser = argparse.ArgumentParser()
     parser.add_argument("typeshed_dir", help="Path to typeshed checkout directory")
-    parser.add_argument("pattern", help="Pattern to select distributions for upload")
+    parser.add_argument("pattern", help="Regular expression to select distributions for upload")
+    parser.add_argument("uploaded", help="Previously uploaded packages to validate dependencies")
     args = parser.parse_args()
-    main(args.typeshed_dir, args.pattern)
+    main(args.typeshed_dir, args.pattern, args.uploaded)

tests/__init__.py

@@ -4,9 +4,11 @@
 a typeshed checkout side by side.
 """
 import os
+import pytest  # type: ignore[import]
 from scripts import get_version, build_wheel
 
 TYPESHED = "../typeshed"
+UPLOADED = "data/uploaded_packages.txt"
 
 
 def test_version() -> None:
@@ -17,9 +19,27 @@ def test_version() -> None:
     assert get_version.main(TYPESHED, "typing-extensions", None) >= 0
 
 
+def test_check_exists() -> None:
+    assert get_version.check_exists("six")
+    assert not get_version.check_exists("nonexistent-distribution")
+
+
 def test_build_wheel() -> None:
     # Check that we can build wheels for all distributions.
     for distribution in os.listdir(os.path.join(TYPESHED, "stubs")):
         tmp_dir = build_wheel.main(TYPESHED, distribution, increment=1)
         assert tmp_dir.endswith("/dist")
         assert list(os.listdir(tmp_dir))  # check it is not empty
+
+
+def test_verify_dependency() -> None:
+    # Check some known dependencies that they verify as valid.
+    build_wheel.verify_dependency(TYPESHED, "types-six", UPLOADED)
+    build_wheel.verify_dependency(TYPESHED, "types-six==0.1.1", UPLOADED)
+    build_wheel.verify_dependency(TYPESHED, "types-typing-extensions", UPLOADED)
+    build_wheel.verify_dependency(TYPESHED, "types-typing-extensions>=3.7", UPLOADED)
+    # Also check couple errors.
+    with pytest.raises(AssertionError):
+        build_wheel.verify_dependency(TYPESHED, "unsupported", UPLOADED)
+    with pytest.raises(AssertionError):
+        build_wheel.verify_dependency(TYPESHED, "types-unknown-xxx", UPLOADED)