Skip to content

Share the single-lock wire and revert envelope across the Poetry, PDM and Pipenv vendored backends #937

Description

[agent] Filed by the scheduled architecture audit routine (ecosystems and formats). Register: register comment.

Kind: refactor. Source: review Part 5.4 ("the pypi_{poetry,pdm,pipenv}.rs backends repeat one skeleton"); register E23.

Problem

The three single-lock PyPI backends each re-implement one skeleton around their format-specific edit:

Step Poetry PDM Pipenv
Target enum {Fresh, InSync} PoetryTarget PdmTarget PipenvTarget
Wire envelope wire_poetry wire_pdm wire_pipenv
Revert prelude revert_poetry revert_pdm revert_pipenv

Every wire function runs the same sequence, in the same order, with copied comments. Only the error-code prefix and the edit in the middle differ:

  1. refuse_symlinked(root, &[LOCK_FILE], "pypi_<flavor>_symlink_unsupported").
  2. check_target_guards(…). On InSync, a defensive pypi_<flavor>_source_already_exists refusal: "already wires {name} to this patch's vendored wheel; nothing to wire". This message is spelled three times.
  3. The format-specific edit.
  4. ensure_unchanged(root, LOCK_FILE, &p.lock_text, "pypi_<flavor>_changed").
  5. LOCK_MEMO.invalidate().
  6. atomic_write_bytes_preserving_mode(…), mapped to pypi_<flavor>_write_failed / "cannot write {LOCK_FILE}: {e}".

Every revert function opens with the same symlink refusal, which returns a literal RevertOutcome { kept_artifact: true, success: false, … }.

The orchestrator then fans the three out by hand: load, guards, the Fresh/InSync mapping, WiringPlan, MetaSlot and revert dispatch, at pypi.rs#L809-L905, #L1277-L1320 and #L1609-L1611.

Drift: none proven in the envelope (read, not executed). The steps match today. The risk is the next safety step, such as the ensure_unchanged race guard, the memo invalidation or the symlink refusal, being added to one or two copies. Each landed in all three separately.

Symptoms / Impact

No open bug. The duplicated envelope is about 150 production lines (3 × ~35 for wire, plus 3 × ~15 for the revert prelude), and the per-flavor test copies grow with it (symlinked_lock_refuses_wire_and_revert_without_writing, lock_changed_during_vendoring_is_refused_before_the_write, wire_preserves_lock_file_mode and *_write_failure_*, ×3 each).

Proposed change

Add one module, vendor/pypi_lock_backend.rs, with:

  • enum LockTarget { Fresh, InSync }, replacing the three per-flavor enums;
  • struct LockFlavor { name: &'static str, lock_file: &'static str }, which derives the exact existing codes (pypi_{name}_symlink_unsupported, _source_already_exists, _changed, _write_failed), so every code string is unchanged;
  • async fn commit_lock(flavor, root, snapshot: &str, new_text: &str, memo: &ParseMemo<_>) -> Result<(), (&'static str, String)>, which runs ensure_unchanged → invalidate → mode-preserving write → error mapping;
  • async fn wire_envelope(flavor, root, guards: impl FnOnce() -> Result<LockTarget, _>) -> Result<(), _>, which runs the symlink refusal, the guards and the defensive InSync refusal;
  • async fn guarded_revert(flavor, root, f) -> RevertOutcome, which runs the symlink prelude and then the flavor's revert.

Then:

  • wire_poetry, wire_pdm and wire_pipenv keep only their edit and meta.
  • Delete: the three target enums, the three defensive InSync refusals, the three write/ensure_unchanged blocks and the three revert preludes.
  • Optionally merge the per-flavor envelope tests into one table test over the three flavors.

Size and scope

Acceptance criteria

  • One LockTarget enum. PoetryTarget, PdmTarget and PipenvTarget are gone.
  • Every pypi_{poetry,pdm,pipenv}_* error code and message is byte-identical. The existing tests that assert codes stay green unchanged.
  • ensure_unchanged, LOCK_MEMO.invalidate and atomic_write_bytes_preserving_mode each appear once across the three backends, in the shared module.
  • All tests in pypi_poetry.rs, pypi_pdm.rs, pypi_pipenv.rs and pypi.rs stay green, including the symlink, FIFO, mode-preservation, concurrent-change, write-failure and in-sync tests.

Dependencies

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)priority:p1refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions