Adapt pip and tests to merged metadata

Author: msimacek

graalpython/com.oracle.graal.python.test/src/tests/test_patched_pip.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2023, 2023, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # The Universal Permissive License (UPL), Version 1.0
@@ -97,7 +97,7 @@ def setUp(self):
             shutil.copytree(self.venv_template_dir, self.venv_dir, symlinks=True)
             self.patch_dir = Path(tempfile.mkdtemp()).resolve()
             self.pip_env = os.environ.copy()
-            self.pip_env['PIPLOADER_PATCHES_BASE_DIRS'] = str(self.patch_dir)
+            self.pip_env['PIP_GRAALPY_PATCHES_URL'] = str(self.patch_dir)
             self.index_dir = Path(tempfile.mkdtemp()).resolve()
 
         def tearDown(self):
@@ -106,18 +106,16 @@ def tearDown(self):
             shutil.rmtree(self.index_dir, ignore_errors=True)
 
         def prepare_config(self, name, rules):
-            package_dir = self.patch_dir / name
-            package_dir.mkdir(exist_ok=True)
             toml_lines = []
             for rule in rules:
-                toml_lines.append('[[rules]]')
+                toml_lines.append(f'[[{name}.rules]]')
                 for k, v in rule.items():
                     if not k.startswith('$'):
                         toml_lines.append(f'{k} = {v!r}')
                 if patch := rule.get('patch'):
-                    with open(package_dir / patch, 'w') as f:
+                    with open(self.patch_dir / patch, 'w') as f:
                         f.write(PATCH_TEMPLATE.format(rule.get('$patch-text', 'Patched')))
-            with open(package_dir / 'metadata.toml', 'w') as f:
+            with open(self.patch_dir / 'metadata.toml', 'w') as f:
                 f.write('\n'.join(toml_lines))
 
         def build_package(self, name, version):

graalpython/lib-graalpython/patches/README.md

@@ -1,15 +1,12 @@
-This directory contains patches applied by pip when installing packages. There is a directory for each package that
-contains patches and optionally a configuration file that can specify rules for matching patches to package versions and
-can also influence pip version selection mechanism.
-
-Configuration files are named `metadata.toml` and can contain the following:
+This directory contains patches applied by pip when installing packages. There is a `metadata.toml` configuration file that specifies rules for patching and
+can also influence pip version selection mechanism. It can contain the following:
 
 ```toml
-# The file defines an array of tables (dicts) named `patches`. The patch selection process iterates it and picks the
-# first patch one that matches in version and dist type.
+# The file defines a dict whose keys are package names. The sub-key 'rules' specifies a list of patching rules.
+# The patch selection process iterates it and picks the first patch one that matches in version and dist type.
 # The next entry will apply to a wheel foo-1.0.0
-[[rules]]
-# Optional. Relative path to a patch file. May be omitted when the entry just specifies `install-priority`
+[[foo.rules]]
+# Optional. Relative path to a patch file. May be omitted
 patch = 'foo-1.0.0.patch'
 # Required if 'patch' is specified. SPDX license expression for the package (allows parentheses, 'AND', 'OR', 'WITH').
 # Allowed licenses are enumerated in mx.graalpython/verify_patches.py
@@ -33,8 +30,8 @@ subdir = 'src'
 # priority to 0, the version will not be shown in the suggestion list we display when we didn't find an applicable patch
 install-priority = 1
 
-# The next entry will apply to all other artifacts of foo
-[[patches]]
+# The next entry will apply to all other versions of foo that didn't get matched by the previous rule
+[[foo.rules]]
 patch = 'foo.patch'
 license = 'MIT'
 ```

graalpython/lib-graalpython/patches/pip-23.2.1.patch

@@ -102,10 +102,10 @@ index a8cd133..20dd1e6 100644
          # file in .data maps to same location as file in wheel root).
 diff --git a/pip/_internal/utils/graalpy.py b/pip/_internal/utils/graalpy.py
 new file mode 100644
-index 0000000..b456846
+index 0000000..2a67f4b
 --- /dev/null
 +++ b/pip/_internal/utils/graalpy.py
-@@ -0,0 +1,214 @@
+@@ -0,0 +1,209 @@
 +# ATTENTION: GraalPy uses existence of this module to verify that it is
 +# running a patched pip in pip_hook.py
 +import os
@@ -120,10 +120,10 @@ index 0000000..b456846
 +from pip._vendor.packaging.specifiers import SpecifierSet
 +from pip._vendor.packaging.version import VERSION_PATTERN
 +
-+PATCHES_BASE_DIRS = [os.path.join(__graalpython__.core_home, "patches")]
-+if hasattr(__graalpython__, "tdebug"):
-+    PATCHES_BASE_DIRS += os.environ.get('PIPLOADER_PATCHES_BASE_DIRS', "").split(",")
-+
++METADATA_PATH = os.environ.get(
++    'PIP_GRAALPY_METADATA',
++    os.path.join(__graalpython__.core_home, 'patches', 'metadata.toml'),
++)
 +DISABLE_PATCHING = os.environ.get('PIP_GRAALPY_DISABLE_PATCHING', '').lower() in ('true', '1')
 +DISABLE_VERSION_SELECTION = os.environ.get('PIP_GRAALPY_DISABLE_VERSION_SELECTION', '').lower() in ('true', '1')
 +
@@ -133,25 +133,16 @@ index 0000000..b456846
 +
 +
 +class PatchRepository:
-+    def __init__(self, base_dirs):
++    def __init__(self, metadata_path):
 +        self._repository = {}
-+        for base_dir in base_dirs:
-+            for package_dir in Path(base_dir).iterdir():
-+                denormalized_name = package_dir.name
-+                normalized_name = normalize_name(denormalized_name)
-+                metadata = {}
-+                if (metadata_path := package_dir / 'metadata.toml').is_file():
-+                    with open(metadata_path, 'rb') as f:
-+                        metadata = tomli.load(f)
-+                    metadata.setdefault('rules', [])
-+                    for rule in metadata['rules']:
-+                        if 'patch' in rule:
-+                            rule['patch'] = package_dir / rule['patch']
-+                self._repository[normalized_name] = metadata
++        self.metadata_path = Path(metadata_path)
++        if self.metadata_path.exists():
++            with open(self.metadata_path, 'rb') as f:
++                self._repository = {normalize_name(name): metadata for name, metadata in tomli.load(f).items()}
 +
 +    def get_rules(self, name):
 +        if metadata := self._repository.get(normalize_name(name)):
-+            return metadata['rules']
++            return metadata.get('rules')
 +
 +    def get_add_sources(self, name):
 +        if metadata := self._repository.get(normalize_name(name)):
@@ -185,14 +176,17 @@ index 0000000..b456846
 +                    continue
 +                return rule
 +
++    def resolve_patch(self, patch_path):
++        return self.metadata_path.parent / patch_path
++
 +
 +__PATCH_REPOSITORY = None
 +
 +
 +def get_patch_repository():
 +    global __PATCH_REPOSITORY
 +    if not __PATCH_REPOSITORY:
-+        __PATCH_REPOSITORY = PatchRepository(PATCHES_BASE_DIRS)
++        __PATCH_REPOSITORY = PatchRepository(METADATA_PATH)
 +    return __PATCH_REPOSITORY
 +
 +
@@ -246,6 +240,7 @@ index 0000000..b456846
 +            location = os.path.join(location, subdir)
 +    if rule:
 +        if patch := rule.get('patch'):
++            patch = repository.resolve_patch(patch)
 +            print(f"Patching package {name} using {patch}")
 +            exe = '.exe' if os.name == 'nt' else ''
 +            try:

mx.graalpython/verify_patches.py

@@ -38,7 +38,6 @@
 # SOFTWARE.
 import argparse
 import re
-import sys
 from pathlib import Path
 
 from pip._vendor import tomli  # pylint: disable=no-name-in-module
@@ -63,44 +62,50 @@
 RULE_KEYS = frozenset({'version', 'patch', 'license', 'subdir', 'dist-type', 'install-priority'})
 
 
-def validate_metadata(package_dir, metadata):
-    if unexpected_keys := set(metadata) - SECTIONS:
-        assert False, f"Unexpected top-level metadata keys: {unexpected_keys}"
+def validate_metadata(patches_dir):
+    with open(patches_dir / 'metadata.toml', 'rb') as f:
+        all_metadata = tomli.load(f)
     patches = set()
-    if rules := metadata.get('rules'):
-        for rule in rules:
-            if unexpected_keys := set(rule) - RULE_KEYS:
-                assert False, f"Unexpected rule keys: {unexpected_keys}"
-            if patch := rule.get('patch'):
-                patch_path = package_dir / patch
-                assert patch_path.is_file(), f"Patch file does not exists: {patch_path}"
-                patches.add(patch_path)
-                license_id = rule.get('license')
-                assert license_id, f"'license' not specified for patch {patch}"
-                license_id = re.sub(r'[()]', ' ', license_id)
-                for part in re.split(f'AND|OR', license_id):
-                    part = part.strip()
-                    if ' WITH ' in part:
-                        part, exception = re.split(r'\s+WITH\s+', part, 1)
-                        assert exception in ALLOWED_WITH_CLAUSES, \
-                            f"License WITH clause {exception} not in allowed list of clauses: {', '.join(ALLOWED_WITH_CLAUSES)}"
-                    assert part in ALLOWED_LICENSES, \
-                        f"License {part} not in allowed list of licenses: {', '.join(ALLOWED_LICENSES)}"
-            if install_priority := rule.get('install-priority'):
-                assert isinstance(install_priority, int), "'rules.install_priority' must be an int"
-            if dist_type := rule.get('dist-type'):
-                assert dist_type in ('wheel', 'sdist'), "'rules.dist_type' must be on of 'wheel', 'sdist'"
-            if version := rule.get('version'):
-                # Just try that it doesn't raise
-                SpecifierSet(version)
-    for file in package_dir.iterdir():
-        assert file.name == 'metadata.toml' or file in patches, f"Dangling file in patch directory: {file}"
-    if add_sources := metadata.get('add-sources'):
-        for add_source in add_sources:
-            if unexpected_keys := set(add_source) - {'version', 'url'}:
-                assert False, f"Unexpected add_source keys: {unexpected_keys}"
-            assert add_source.get('version'), f"Missing 'add_sources.version' key in {package_dir}"
-            assert add_source.get('url'), f"Missing 'add_sources.url' key in {package_dir}"
+    for package, metadata in all_metadata.items():
+        try:
+            if unexpected_keys := set(metadata) - SECTIONS:
+                assert False, f"Unexpected top-level metadata keys: {unexpected_keys}"
+            if rules := metadata.get('rules'):
+                for rule in rules:
+                    if unexpected_keys := set(rule) - RULE_KEYS:
+                        assert False, f"Unexpected rule keys: {unexpected_keys}"
+                    if patch := rule.get('patch'):
+                        patch_path = patches_dir / patch
+                        assert patch_path.is_file(), f"Patch file does not exists: {patch_path}"
+                        patches.add(patch_path)
+                        license_id = rule.get('license')
+                        assert license_id, f"'license' not specified for patch {patch}"
+                        license_id = re.sub(r'[()]', ' ', license_id)
+                        for part in re.split(f'AND|OR', license_id):
+                            part = part.strip()
+                            if ' WITH ' in part:
+                                part, exception = re.split(r'\s+WITH\s+', part, 1)
+                                assert exception in ALLOWED_WITH_CLAUSES, \
+                                    f"License WITH clause {exception} not in allowed list of clauses: {', '.join(ALLOWED_WITH_CLAUSES)}"
+                            assert part in ALLOWED_LICENSES, \
+                                f"License {part} not in allowed list of licenses: {', '.join(ALLOWED_LICENSES)}"
+                    if install_priority := rule.get('install-priority'):
+                        assert isinstance(install_priority, int), "'rules.install_priority' must be an int"
+                    if dist_type := rule.get('dist-type'):
+                        assert dist_type in ('wheel', 'sdist'), "'rules.dist_type' must be on of 'wheel', 'sdist'"
+                    if version := rule.get('version'):
+                        # Just try that it doesn't raise
+                        SpecifierSet(version)
+            if add_sources := metadata.get('add-sources'):
+                for add_source in add_sources:
+                    if unexpected_keys := set(add_source) - {'version', 'url'}:
+                        assert False, f"Unexpected add_source keys: {unexpected_keys}"
+                    assert add_source.get('version'), "Missing 'add_sources.version' key"
+                    assert add_source.get('url'), "Missing 'add_sources.url' key"
+        except Exception as e:
+            raise AssertionError(f"{package}: {e}")
+    for file in patches_dir.iterdir():
+        assert not file.name.endswith('patch') or file in patches, f"Dangling patch file: {file}"
 
 
 def main():
@@ -109,20 +114,7 @@ def main():
 
     args = parser.parse_args()
 
-    errors = []
-    for package_dir in args.patches_dir.iterdir():
-        if package_dir.is_dir():
-            try:
-                if (metadata_path := package_dir / 'metadata.toml').is_file():
-                    with open(metadata_path, 'rb') as f:
-                        metadata = tomli.load(f)
-                        validate_metadata(package_dir, metadata)
-                else:
-                    assert False, f"Patch directory without metadata: {package_dir}"
-            except AssertionError as e:
-                errors.append(f"\t{package_dir.name}: {e}")
-    if errors:
-        sys.exit("Patch metadata validation failed:\n" + '\n'.join(errors))
+    validate_metadata(args.patches_dir)
 
 
 if __name__ == '__main__':

scripts/repack-bundled-wheels.sh

@@ -77,4 +77,4 @@ patch_wheel() {
     rm -rf "$tmpdir"
 }
 
-patch_wheel pip graalpython/lib-graalpython/patches/pip/pip-23.2.1.patch
+patch_wheel pip graalpython/lib-graalpython/patches/pip-23.2.1.patch