Skip to content

Refactor(depends): collapse duplicated _available / _ensure_* helpers in depends.py #873

@asclearuc

Description

@asclearuc

Problem Statement

packages/server/engine-lib/rocketlib-python/lib/depends.py contains several copy-pasted blocks that have drifted slightly over time and make the bootstrap code harder to maintain. Each time a new build-time dependency is added (e.g. the recent setuptools addition in #871), another near-identical pair of functions is appended.

Concretely, the following patterns are duplicated:

  1. _<module>_available() — 3 identical copies

    Each body is the same importlib.util.find_spec(name) check wrapped in a try/except Exception.

  2. _ensure_<module>() — 4 near-identical copies

    All share the same shape: check → monitorStatus('Installing X…')_run([sys.executable, '-m', 'pip', 'install', X, '--quiet', '--disable-pip-version-check'], check=False) → on non-zero rc error(...) + raise RuntimeError(...) → optional re-verify.

  3. uv pip install argv builder — 2 copies

    Same base argv (--python, -r, --index-strategy unsafe-best-match, --no-build-isolation). Only --dry-run --no-color differs.

  4. Constraints-file gating — 3 copies

    if os.path.exists(constraints_path) and os.path.getsize(constraints_path) > 0:
        uv_args.extend(['-c', './cache/constraints.txt'])

    At depends.py:705-706, depends.py:783-784, depends.py:916-917.

  5. Minor: if not _uv_available(): raise RuntimeError('uv executable not found') is duplicated at depends.py:464-465 and depends.py:685-686. The subprocess.run(..., capture_output=True, text=True, check=False, stdin=subprocess.PIPE, cwd=exe_dir) invocation for uv is also repeated in _compile_constraints and _install_dry_run.

Proposed Solution

Introduce two small helpers that subsume patterns (1) and (2):

def _module_available(name: str) -> bool:
    import importlib.util
    try:
        return importlib.util.find_spec(name) is not None
    except Exception:
        return False

def _ensure_module(name: str, *, check, install_args=None, verify=True):
    if check():
        debug(f'{name} is available')
        return
    monitorStatus(f'Installing {name}...')
    args = install_args or [
        sys.executable, '-m', 'pip', 'install', name,
        '--quiet', '--disable-pip-version-check',
    ]
    result = _run(args, check=False)
    if result.returncode != 0:
        error(f'Failed to install {name}: {result.stderr}')
        raise RuntimeError(f'Failed to install {name}')
    if verify and not check():
        raise RuntimeError(f'{name} installed but not found')
    debug(f'{name} installed successfully')

Then:

  • _ensure_wheel, _ensure_setuptools, _ensure_uv become one-liners.
  • _ensure_pip keeps its custom ensurepip flow but reuses _module_available('pip').

Follow up with patterns (3) and (4):

  • Extract _build_uv_install_argv(requirements_path, constraints_path, *, dry_run=False) that returns the full argv list including the constraints -c flag when applicable.
  • Replace the three constraints-gating snippets with a single _constraints_flags(constraints_path) helper.

Expected savings: ~50–60 lines, with the bonus that adding the next bootstrapped dependency becomes a one-line _ensure_module(...) call.

Alternatives Considered

  • Leave as-is. Each helper is short and readable on its own. Rejected because the duplication has already caused subtle drift (e.g. _ensure_wheel skips the post-install re-verify that _ensure_setuptools and _ensure_uv do — likely an oversight, not a deliberate choice).
  • Class-based bootstrapper (e.g. BootstrapStep). Overkill for 4 dependencies; pure functions match the existing style better.
  • Defer to uv tool install / uv self. Doesn't help here — these helpers run before uv is guaranteed to exist.

Affected Modules

  • server (C++ engine)

Metadata

Metadata

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions