Skip to content

feat(pm): add phase timings and install summary#2813

Merged
xusd320 merged 7 commits intonextfrom
feat/pm-install-progress-summary
Apr 20, 2026
Merged

feat(pm): add phase timings and install summary#2813
xusd320 merged 7 commits intonextfrom
feat/pm-install-progress-summary

Conversation

@elrrrrrrr
Copy link
Copy Markdown
Contributor

@elrrrrrrr elrrrrrrr commented Apr 20, 2026

Summary

Addresses two UX gaps reported during dependency install:

  1. The post-resolve clone stage runs clean_deps first (validates & prunes stale node_modules), during which pos doesn't advance and the bar looks frozen on large trees. Now the spinner message reads validating node_moduleslinking packages so the user sees which phase is active.
  2. The scripts phase had no timing info. Each ✓ line now appends a dimmed [elapsed], and a two-line summary prints at the end of install:
    ✓ 7766/7766 package-lock.json resolved [0.2s]
    ✓ 3550/3550 node_modules cloned [2.6s]
    ✓ 10/10 scripts executed [3.2s]
    
    + 513 added · 3017 reused
    

Implementation notes

  • cloner::clone_package now returns Result<bool> (true = freshly materialized, false = valid existing dst reused). Outcomes are accumulated into two process-global atomics exposed via a CloneStats snapshot + Sub impl.
  • InstallService::install() snapshots clone_stats() at entry and prints the delta; this keeps nested installs (global install flows) reporting only their own work.
  • finish_progress_bar now takes Option<Duration> — one function instead of two + a trampoline. format_elapsed_time is built on a new shared fmt_duration core.

Test plan

  • cargo fmt
  • cargo clippy -p utoo-pm --all-targets -- -D warnings --no-deps
  • cargo test -p utoo-pm --bin utoo (cloner/install/logger tests all green)
  • Manual smoke on a large workspace to confirm the summary numbers and timings look sensible

Surface the clean/validate step in the progress bar so the clone phase
doesn't appear frozen on large trees, append elapsed time to each ✓
line (resolve / cloned / scripts), and print a two-line summary at the
end of install:

  + 513 added · 3017 reused
  timing  resolve 0.2s  link 2.6s  scripts 3.2s  total 6.0s

Track fresh vs reused clones via a shared CloneStats snapshot in
cloner, diffed against a baseline taken at install entry so nested
installs (e.g. global install) report their own delta.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the package installation process by adding detailed timing and statistics tracking. It introduces the ability to measure the duration of key phases such as dependency resolution, package linking, and script execution, while also tracking the counts of freshly cloned versus reused packages. A new installation summary is printed upon completion to provide users with these performance metrics. I have no feedback to provide.

elrrrrrrr and others added 6 commits April 20, 2026 15:34
Each ✓ line already shows its phase elapsed time, so the trailing
`timing resolve … link … scripts … total …` line was duplicating that
info. Keep only the counts line:

  + 513 added · 3017 reused

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a third counter alongside added/reused so CI output stays
meaningful even when node_modules/ is always fresh (everything is
"added") but the tarball cache is persisted across runs.

  + 513 added · 3017 reused · 123 downloaded

- added:      node_modules/ dirs freshly materialized
- reused:     node_modules/ dirs already valid, kept as-is
- downloaded: tarballs fetched from the registry this run

downloaded is tracked at the download_to_cache OnceMap closure, so
each unique name@version counts at most once.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ensure_package_lock is a lock-loading helper, not a UI layer — it
shouldn't own progress-bar lifecycle. Move start/finish into the
one caller that was relying on it (cmd::rebuild) and timestamp the
phase there, matching cmd::deps.

Also fill in the elapsed-time suffix for the `ut deps` command,
which was previously passing `None`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous reuse_count tracked "node_modules/<pkg> was already valid
and skipped", which is not what pnpm users expect under "reused" — pnpm
uses reused for "tarball served from local cache, no network".

Drop the node_modules-reuse counter and redefine the three counts to
match pnpm:

  + 513 added · 3017 reused · 123 downloaded

- added:      linked into node_modules/ this run
- reused:     tarball served from local cache (no network)
- downloaded: tarball fetched from the registry

Simplifies cloner.rs: clone_package still returns bool but only to tell
clone_package_once whether to bump CLONE_COUNT; there is no paired
REUSE_COUNT anymore. Tarball counters now live in downloader.rs
alongside the OnceMap they key off of.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@elrrrrrrr elrrrrrrr added the A-Pkg Manager Area: Package Manager label Apr 20, 2026
@elrrrrrrr elrrrrrrr marked this pull request as ready for review April 20, 2026 11:21
@xusd320 xusd320 merged commit 1ae2939 into next Apr 20, 2026
27 checks passed
@xusd320 xusd320 deleted the feat/pm-install-progress-summary branch April 20, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Pkg Manager Area: Package Manager

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants