pythongh-136264: Fix --relative-paths for PEP 739's build-details.json (python#138510)

* KeyError is not raised for defaultdict
* Fix relative paths on different drives on Windows
* Add a round-trip test

Co-authored-by: Itamar Oren <[email protected]>

Author: AA-Turner

Lib/test/test_build_details.py

@@ -1,12 +1,34 @@
+import importlib
 import json
 import os
+import os.path
 import sys
 import sysconfig
 import string
 import unittest
+from pathlib import Path
 
 from test.support import is_android, is_apple_mobile, is_wasm32
 
+BASE_PATH = Path(
+    __file__,  # Lib/test/test_build_details.py
+    '..',  # Lib/test
+    '..',  # Lib
+    '..',  # <src/install dir>
+).resolve()
+MODULE_PATH = BASE_PATH / 'Tools' / 'build' / 'generate-build-details.py'
+
+try:
+    # Import "generate-build-details.py" as "generate_build_details"
+    spec = importlib.util.spec_from_file_location(
+        "generate_build_details", MODULE_PATH
+    )
+    generate_build_details = importlib.util.module_from_spec(spec)
+    sys.modules["generate_build_details"] = generate_build_details
+    spec.loader.exec_module(generate_build_details)
+except (FileNotFoundError, ImportError):
+    generate_build_details = None
+
 
 class FormatTestsBase:
     @property
@@ -31,16 +53,15 @@ def key(self, name):
             value = value[part]
         return value
 
-    def test_parse(self):
-        self.data
-
     def test_top_level_container(self):
         self.assertIsInstance(self.data, dict)
         for key, value in self.data.items():
             with self.subTest(key=key):
-                if key in ('schema_version', 'base_prefix', 'base_interpreter', 'platform'):
+                if key in ('schema_version', 'base_prefix', 'base_interpreter',
+                           'platform'):
                     self.assertIsInstance(value, str)
-                elif key in ('language', 'implementation', 'abi', 'suffixes', 'libpython', 'c_api', 'arbitrary_data'):
+                elif key in ('language', 'implementation', 'abi', 'suffixes',
+                             'libpython', 'c_api', 'arbitrary_data'):
                     self.assertIsInstance(value, dict)
 
     def test_base_prefix(self):
@@ -71,15 +92,20 @@ def test_language_version_info(self):
         self.assertEqual(len(value), sys.version_info.n_fields)
         for part_name, part_value in value.items():
             with self.subTest(part=part_name):
-                self.assertEqual(part_value, getattr(sys.version_info, part_name))
+                sys_version_value = getattr(sys.version_info, part_name)
+                self.assertEqual(part_value, sys_version_value)
 
     def test_implementation(self):
+        impl_ver = sys.implementation.version
         for key, value in self.key('implementation').items():
             with self.subTest(part=key):
                 if key == 'version':
-                    self.assertEqual(len(value), len(sys.implementation.version))
+                    self.assertEqual(len(value), len(impl_ver))
                     for part_name, part_value in value.items():
-                        self.assertEqual(getattr(sys.implementation.version, part_name), part_value)
+                        self.assertFalse(isinstance(sys.implementation.version, dict))
+                        getattr(sys.implementation.version, part_name)
+                        sys_implementation_value = getattr(impl_ver, part_name)
+                        self.assertEqual(sys_implementation_value, part_value)
                 else:
                     self.assertEqual(getattr(sys.implementation, key), value)
 
@@ -99,15 +125,16 @@ class CPythonBuildDetailsTests(unittest.TestCase, FormatTestsBase):
     def location(self):
         if sysconfig.is_python_build():
             projectdir = sysconfig.get_config_var('projectbase')
-            with open(os.path.join(projectdir, 'pybuilddir.txt')) as f:
+            pybuilddir = os.path.join(projectdir, 'pybuilddir.txt')
+            with open(pybuilddir, encoding='utf-8') as f:
                 dirname = os.path.join(projectdir, f.read())
         else:
             dirname = sysconfig.get_path('stdlib')
         return os.path.join(dirname, 'build-details.json')
 
     @property
     def contents(self):
-        with open(self.location, 'r') as f:
+        with open(self.location, 'r', encoding='utf-8') as f:
             return f.read()
 
     @needs_installed_python
@@ -147,5 +174,64 @@ def test_c_api(self):
         self.assertTrue(os.path.exists(os.path.join(value['pkgconfig_path'], f'python-{version}.pc')))
 
 
+@unittest.skipIf(
+    generate_build_details is None,
+    "Failed to import generate-build-details"
+)
+@unittest.skipIf(os.name != 'posix', 'Feature only implemented on POSIX right now')
+@unittest.skipIf(is_wasm32, 'Feature not available on WebAssembly builds')
+class BuildDetailsRelativePathsTests(unittest.TestCase):
+    @property
+    def build_details_absolute_paths(self):
+        data = generate_build_details.generate_data(schema_version='1.0')
+        return json.loads(json.dumps(data))
+
+    @property
+    def build_details_relative_paths(self):
+        data = self.build_details_absolute_paths
+        generate_build_details.make_paths_relative(data, config_path=None)
+        return data
+
+    def test_round_trip(self):
+        data_abs_path = self.build_details_absolute_paths
+        data_rel_path = self.build_details_relative_paths
+
+        self.assertEqual(data_abs_path['base_prefix'],
+                         data_rel_path['base_prefix'])
+
+        base_prefix = data_abs_path['base_prefix']
+
+        top_level_keys = ('base_interpreter',)
+        for key in top_level_keys:
+            self.assertEqual(key in data_abs_path, key in data_rel_path)
+            if key not in data_abs_path:
+                continue
+
+            abs_rel_path = os.path.join(base_prefix, data_rel_path[key])
+            abs_rel_path = os.path.normpath(abs_rel_path)
+            self.assertEqual(data_abs_path[key], abs_rel_path)
+
+        second_level_keys = (
+            ('libpython', 'dynamic'),
+            ('libpython', 'dynamic_stableabi'),
+            ('libpython', 'static'),
+            ('c_api', 'headers'),
+            ('c_api', 'pkgconfig_path'),
+
+        )
+        for part, key in second_level_keys:
+            self.assertEqual(part in data_abs_path, part in data_rel_path)
+            if part not in data_abs_path:
+                continue
+            self.assertEqual(key in data_abs_path[part],
+                             key in data_rel_path[part])
+            if key not in data_abs_path[part]:
+                continue
+
+            abs_rel_path = os.path.join(base_prefix, data_rel_path[part][key])
+            abs_rel_path = os.path.normpath(abs_rel_path)
+            self.assertEqual(data_abs_path[part][key], abs_rel_path)
+
+
 if __name__ == '__main__':
     unittest.main()

Tools/build/generate-build-details.py

@@ -55,7 +55,7 @@ def generate_data(schema_version: str) -> collections.defaultdict[str, Any]:
     data['language']['version'] = sysconfig.get_python_version()
     data['language']['version_info'] = version_info_to_dict(sys.version_info)
 
-    data['implementation'] = vars(sys.implementation)
+    data['implementation'] = vars(sys.implementation).copy()
     data['implementation']['version'] = version_info_to_dict(sys.implementation.version)
     # Fix cross-compilation
     if '_multiarch' in data['implementation']:
@@ -104,7 +104,7 @@ def generate_data(schema_version: str) -> collections.defaultdict[str, Any]:
         data['abi']['extension_suffix'] = sysconfig.get_config_var('EXT_SUFFIX')
 
         # EXTENSION_SUFFIXES has been constant for a long time, and currently we
-        # don't have a better information source to find the  stable ABI suffix.
+        # don't have a better information source to find the stable ABI suffix.
         for suffix in importlib.machinery.EXTENSION_SUFFIXES:
             if suffix.startswith('.abi'):
                 data['abi']['stable_abi_suffix'] = suffix
@@ -133,33 +133,51 @@ def generate_data(schema_version: str) -> collections.defaultdict[str, Any]:
 def make_paths_relative(data: dict[str, Any], config_path: str | None = None) -> None:
     # Make base_prefix relative to the config_path directory
     if config_path:
-        data['base_prefix'] = os.path.relpath(data['base_prefix'], os.path.dirname(config_path))
+        data['base_prefix'] = relative_path(data['base_prefix'],
+                                            os.path.dirname(config_path))
+    base_prefix = data['base_prefix']
+
     # Update path values to make them relative to base_prefix
-    PATH_KEYS = [
+    PATH_KEYS = (
         'base_interpreter',
         'libpython.dynamic',
         'libpython.dynamic_stableabi',
         'libpython.static',
         'c_api.headers',
         'c_api.pkgconfig_path',
-    ]
+    )
     for entry in PATH_KEYS:
-        parent, _, child = entry.rpartition('.')
+        *parents, child = entry.split('.')
         # Get the key container object
         try:
             container = data
-            for part in parent.split('.'):
+            for part in parents:
                 container = container[part]
+            if child not in container:
+                raise KeyError(child)
             current_path = container[child]
         except KeyError:
             continue
         # Get the relative path
-        new_path = os.path.relpath(current_path, data['base_prefix'])
+        new_path = relative_path(current_path, base_prefix)
         # Join '.' so that the path is formated as './path' instead of 'path'
         new_path = os.path.join('.', new_path)
         container[child] = new_path
 
 
+def relative_path(path: str, base: str) -> str:
+    if os.name != 'nt':
+        return os.path.relpath(path, base)
+
+    # There are no relative paths between drives on Windows.
+    path_drv, _ = os.path.splitdrive(path)
+    base_drv, _ = os.path.splitdrive(base)
+    if path_drv.lower() == base_drv.lower():
+        return os.path.relpath(path, base)
+
+    return path
+
+
 def main() -> None:
     parser = argparse.ArgumentParser(exit_on_error=False)
     parser.add_argument('location')
@@ -186,8 +204,9 @@ def main() -> None:
         make_paths_relative(data, args.config_file_path)
 
     json_output = json.dumps(data, indent=2)
-    with open(args.location, 'w') as f:
-        print(json_output, file=f)
+    with open(args.location, 'w', encoding='utf-8') as f:
+        f.write(json_output)
+        f.write('\n')
 
 
 if __name__ == '__main__':