Skip to content

update_lib hard dependency resolver #6817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 3 commits into from
Jan 20, 2026
Merged

update_lib hard dependency resolver #6817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7bd50b2
update_lib __init__ handling
youknowone Jan 20, 2026
9b7b53c
dependency
youknowone Jan 20, 2026
24c1b88
Auto-format: ruff format
github-actions[bot] Jan 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/commands/upgrade-pylib.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ If during the upgrade process you encounter any of the following issues with `sc

**STOP the upgrade and report the issue first.** Describe:
1. What you were trying to do
- Library name
- The full command executed (e.g. python scripts/update_lib quick cpython/Lib/$ARGUMENTS.py)
- Library name
- The full command executed (e.g. python scripts/update_lib quick cpython/Lib/$ARGUMENTS.py)
2. What went wrong or what's missing
3. Expected vs actual behavior

Expand Down
27 changes: 26 additions & 1 deletion scripts/update_lib/auto_mark.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@
from update_lib.path import test_name_from_path


class TestRunError(Exception):
"""Raised when test run fails entirely (e.g., import error, crash)."""

pass


@dataclass
class Test:
name: str = ""
Expand Down Expand Up @@ -58,7 +64,8 @@ def run_test(test_name: str, skip_build: bool = False) -> TestResult:

result = subprocess.run(
cmd + ["-m", "test", "-v", "-u", "all", "--slowest", test_name],
capture_output=True,
stdout=subprocess.PIPE, # Capture stdout for parsing
stderr=None, # Let stderr pass through to terminal
text=True,
)
return parse_results(result)
Expand Down Expand Up @@ -236,6 +243,9 @@ def _is_super_call_only(func_node: ast.FunctionDef | ast.AsyncFunctionDef) -> bo
return False
if not isinstance(call.func, ast.Attribute):
return False
# Verify the method name matches
if call.func.attr != func_node.name:
return False
super_call = call.func.value
if not isinstance(super_call, ast.Call):
return False
Expand Down Expand Up @@ -487,6 +497,14 @@ def auto_mark_file(
print(f"Running test: {test_name}")

results = run_test(test_name, skip_build=skip_build)

# Check if test run failed entirely (e.g., import error, crash)
if not results.tests_result:
raise TestRunError(
f"Test run failed for {test_name}. "
f"Output: {results.stdout[-500:] if results.stdout else '(no output)'}"
)

contents = test_path.read_text(encoding="utf-8")

all_failing_tests, unexpected_successes, error_messages = collect_test_changes(
Expand Down Expand Up @@ -577,6 +595,13 @@ def auto_mark_directory(

results = run_test(test_name, skip_build=skip_build)

# Check if test run failed entirely (e.g., import error, crash)
if not results.tests_result:
raise TestRunError(
f"Test run failed for {test_name}. "
f"Output: {results.stdout[-500:] if results.stdout else '(no output)'}"
)

total_added = 0
total_removed = 0
total_regressions = 0
Expand Down
56 changes: 44 additions & 12 deletions scripts/update_lib/copy_lib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,12 @@
import sys


def copy_lib(
def _copy_single(
src_path: pathlib.Path,
lib_path: pathlib.Path,
verbose: bool = True,
) -> None:
"""
Copy library file or directory from CPython.

Args:
src_path: Source path (e.g., cpython/Lib/dataclasses.py or cpython/Lib/json)
verbose: Print progress messages
"""
from update_lib.path import parse_lib_path

lib_path = parse_lib_path(src_path)

"""Copy a single file or directory."""
# Remove existing file/directory
if lib_path.exists():
if lib_path.is_dir():
Expand All @@ -46,6 +37,7 @@ def copy_lib(
if src_path.is_dir():
if verbose:
print(f"Copying directory: {src_path} -> {lib_path}")
lib_path.parent.mkdir(parents=True, exist_ok=True)
shutil.copytree(src_path, lib_path)
else:
if verbose:
Expand All @@ -54,6 +46,46 @@ def copy_lib(
shutil.copy2(src_path, lib_path)


def copy_lib(
src_path: pathlib.Path,
verbose: bool = True,
) -> None:
"""
Copy library file or directory from CPython.

Also copies additional files if defined in DEPENDENCIES table.

Args:
src_path: Source path (e.g., cpython/Lib/dataclasses.py or cpython/Lib/json)
verbose: Print progress messages
"""
from update_lib.deps import get_lib_paths
from update_lib.path import parse_lib_path

# Extract module name and cpython prefix from path
path_str = str(src_path).replace("\\", "/")
Copy link
Contributor

@ShaharNaveh ShaharNaveh Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I look at this again, wouldn't as_posix() would also do this?

Copy link
Member Author

@youknowone youknowone Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once tried, but not simply replacable.

if "/Lib/" in path_str:
cpython_prefix, after_lib = path_str.split("/Lib/", 1)
# Get module name (first component, without .py)
name = after_lib.split("/")[0]
if name.endswith(".py"):
name = name[:-3]
else:
# Fallback: just copy the single file
lib_path = parse_lib_path(src_path)
_copy_single(src_path, lib_path, verbose)
return
Comment on lines +65 to +77
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fallback path handling is currently unreachable; nested Lib paths mis-resolve module names.

When /Lib/ isn’t present, parse_lib_path will always raise, so the “fallback” can’t work. Also, deriving the name from the first component after /Lib/ mis-resolves nested paths like cpython/Lib/test/libregrtest (name becomes test), which can copy the wrong module. Consider copying nested paths directly (or reverse-mapping via DEPENDENCIES) and making the non-/Lib/ fallback explicit.

🛠️ Proposed fix (direct-copy for nested paths + explicit fallback) 
     if "/Lib/" in path_str:
         cpython_prefix, after_lib = path_str.split("/Lib/", 1)
+        # If a deeper path is provided (e.g., Lib/test/libregrtest),
+        # copy it directly instead of resolving by top-level module name.
+        if "/" in after_lib:
+            lib_path = parse_lib_path(src_path)
+            _copy_single(src_path, lib_path, verbose)
+            return
         # Get module name (first component, without .py)
         name = after_lib.split("/")[0]
         if name.endswith(".py"):
             name = name[:-3]
     else:
-        # Fallback: just copy the single file
-        lib_path = parse_lib_path(src_path)
-        _copy_single(src_path, lib_path, verbose)
-        return
+        # Fallback: allow direct Lib/... paths
+        if path_str.startswith("Lib/"):
+            lib_path = pathlib.Path(path_str)
+            _copy_single(src_path, lib_path, verbose)
+            return
+        raise ValueError(f"Path must contain '/Lib/' or start with 'Lib/' (got: {src_path})")
🤖 Prompt for AI Agents 
In `@scripts/update_lib/copy_lib.py` around lines 65 - 77, The code that parses
src_path via path_str and splits on "/Lib/" mis-handles nested module paths and
makes the "else" fallback unreachable because parse_lib_path always raises;
update the logic in copy_lib.py so that when "/Lib/" is present you preserve the
full after_lib (not just its first path component) and derive the destination
accordingly (e.g., use the full nested path portion from after_lib to compute
lib_path), and when "/Lib/" is not present call parse_lib_path inside a
try/except and on exception perform an explicit direct copy using _copy_single
(or consult DEPENDENCIES mapping) instead of assuming parse_lib_path will
succeed; refer to variables/path pieces path_str, cpython_prefix, after_lib,
name, and functions parse_lib_path and _copy_single to locate the changes.


# Get all paths to copy from DEPENDENCIES table
all_src_paths = get_lib_paths(name, cpython_prefix)

# Copy each file
for src in all_src_paths:
if src.exists():
lib_path = parse_lib_path(src)
_copy_single(src, lib_path, verbose)


def main(argv: list[str] | None = None) -> int:
parser = argparse.ArgumentParser(
description=__doc__,
Expand Down
Loading