Skip to content

fix(scoop): write wheels.cmd in pre_install so Scoop's bin shim step succeeds#2603

Merged
bpamiri merged 1 commit into
developfrom
claude/optimistic-goldstine-a1357b
May 12, 2026
Merged

fix(scoop): write wheels.cmd in pre_install so Scoop's bin shim step succeeds#2603
bpamiri merged 1 commit into
developfrom
claude/optimistic-goldstine-a1357b

Conversation

@bpamiri
Copy link
Copy Markdown
Collaborator

@bpamiri bpamiri commented May 12, 2026

Summary

  • Scoop's install order is pre_installbin shim creation → post_install. Both scoop manifests emitted the wheels.cmd launcher in post_install, so the shim step ran first, failed with Can't shim 'wheels.cmd': File doesn't exist., and aborted the install before the launcher was ever written.
  • Move the launcher-creation block to pre_install in tools/distribution-drafts/scoop/build-manifests.py. Validator (validate.py) and README pick up the same rename.
  • The emitted JSON manifests change by exactly one key per file; everything else (wrapper content, URLs, hashes, extract paths, banners) is byte-identical.

Repro (before the fix)

scoop bucket add wheels https://github.com/wheels-dev/scoop-wheels
scoop install wheels-be
# ... downloads + hash checks succeed, extraction succeeds, then:
# Creating shim for 'wheels'.
# Get-Command : The term 'wheels.cmd' is not recognized ...
# Can't shim 'wheels.cmd': File doesn't exist.
# (post_install never runs, so wheels.cmd is never written)

Hit on Windows by anyone trying scoop install wheels-be today. The stable channel (wheels) would have failed identically the moment GA cuts, since it shares the same generator.

Fix

In tools/distribution-drafts/scoop/build-manifests.py:

  • Rename build_post_installbuild_pre_install and expand its docstring to explain why this has to be pre_install (so future readers don't ``fix'' it back).
  • Both manifest_be() and manifest_stable() now set \"pre_install\": build_pre_install(...) instead of \"post_install\": ....
  • validate.py reads the key as pre_install.
  • README.md documents the ordering rationale inline.

Diff shape

 tools/distribution-drafts/scoop/README.md          | 10 +++++++---
 tools/distribution-drafts/scoop/build-manifests.py | 14 ++++++++++----
 tools/distribution-drafts/scoop/validate.py        |  6 +++---
 tools/distribution-drafts/scoop/wheels-be.json     |  2 +-
 tools/distribution-drafts/scoop/wheels.json        |  2 +-

The two JSON diffs are literally one line each:

-    \"post_install\": [
+    \"pre_install\": [

Follow-up (separate PR)

The live bucket at wheels-dev/scoop-wheels still ships the broken manifests. After this lands, I'll open a companion PR on that repo copying the regenerated wheels.json and wheels-be.json into bucket/. Both PRs are independently merge-safe — this one corrects the source of truth; the bucket PR delivers the fix to end users.

Test plan

  • python3 tools/distribution-drafts/scoop/build-manifests.py regenerates both manifests; diff is one key per file
  • python3 tools/distribution-drafts/scoop/validate.py --offline passes (24 ok lines, 0 failures)
  • After the scoop-wheels companion PR merges, end-to-end scoop install wheels-be on Windows succeeds and wheels --version prints the channel-tagged banner

🤖 Generated with Claude Code

…succeeds

Scoop's install order is pre_install -> bin shim creation -> post_install.
Both manifests emitted the wheels.cmd launcher in post_install, so the shim
step ran first, failed with `Can't shim 'wheels.cmd': File doesn't exist.`,
and aborted the install before the launcher was ever written. Surfaced on
Windows as a permanently-broken `scoop install wheels-be` (and would have hit
`wheels` GA the same way).

Move the launcher-creation block to pre_install in the generator. Validator
and README pick up the same rename. The emitted JSON manifests change by
exactly one key per file; everything else (wrapper content, URLs, hashes,
extract paths, banners) is byte-identical.

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

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer A

TL;DR: This PR correctly fixes a real, reproducible Windows install failure caused by Scoop's install-order constraint (pre_install → bin shim creation → post_install). The change is minimal, consistent across all five affected files, and well-documented. No CFML code is touched, so framework-specific conventions don't apply. One minor docs nit; no blocking issues.


Correctness

The root-cause analysis is accurate. Scoop's documented install order places bin-shim creation after pre_install but before post_install. The wheels.cmd launcher must exist at shim-creation time, which it didn't under the old post_install placement. Moving the writer block to pre_install is the correct fix — extraction to $dir completes before pre_install runs, so $dir\wheels.cmd can be written there safely and will be present when Scoop's shim step fires.

The change is applied consistently:

  • build-manifests.py — function renamed build_post_installbuild_pre_install; docstring explains the why; both manifest_be() and manifest_stable() updated.
  • validate.pymanifest.get("pre_install", []) and the error string both updated.
  • wheels-be.json / wheels.json — pre-generated manifests regenerated; diff is exactly one key per file.
  • README.md — ordering rationale documented inline.

No path was missed.


Docs

Minor nit — missing CHANGELOG entry. CHANGELOG.md has an [Unreleased] section (line 21) with no entry for this fix. A Windows install failure that silently drops users is worth a one-liner:

## [Unreleased]
### Fixed
- Fixed Scoop install on Windows: `wheels.cmd` is now written in `pre_install` so
  Scoop's bin-shim step can find it (#2603)

Not a blocker, but useful for anyone tracking release notes.


Commits

Single commit: fix(scoop): write wheels.cmd in pre_install so Scoop's bin shim step succeeds

  • Type fix ✓, scope scoop ✓, subject ≤ 100 chars ✓, not ALL-CAPS ✓.

Tests

validate.py --offline passes (PR author confirmed: 24 ok lines, 0 failures). The end-to-end scoop install wheels-be on Windows is deferred to the companion bucket PR — that is appropriate sequencing and the test plan is honest about it being marked incomplete. No automated unit tests exist for the Python generator scripts, which is consistent with the rest of the tooling directory.


Overall this is a clean, focused fix. The CHANGELOG nit aside, the diff is correct and the follow-up companion PR against the live bucket is the right delivery path.

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 12, 2026

Wheels Bot - Reviewer B (round 1)

A's review is technically accurate about the fix itself, but misses one blocking issue that undercuts the no-blocking-issues conclusion: the single commit in this PR is missing its DCO Signed-off-by trailer. The DCO GitHub App is a required status check and will block the merge until the author rebases with --signoff. On every other dimension A's analysis holds up.

Sycophancy

None detected. A's correctness claim is evidence-backed (cites Scoop's documented pre_install to bin shim to post_install order), A explicitly verified all five changed files, and A used the COMMENTED state rather than a formal approval.

False positives

None detected. Every assertion A made was verified against the diff:

  • Function rename build_post_install to build_pre_install with expanded docstring: confirmed in build-manifests.py.
  • Both manifest_be() and manifest_stable() updated: confirmed.
  • validate.py updated (manifest.get pre_install and error string): confirmed.
  • wheels-be.json and wheels.json each change exactly one key: confirmed.
  • README.md updated including the inline Python snippet at line 117: confirmed.
  • Commit format (type fix, scope scoop, subject under 100 chars, not ALL-CAPS): confirmed.
  • CHANGELOG [Unreleased] section exists at line 21: confirmed.

Missed issues

Blocking -- missing DCO Signed-off-by trailer.

The commit 7261371 is authored by Peter Amiri. Its body ends with:

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

There is no Signed-off-by line. Per CONTRIBUTING.md and the DCO GitHub App (a required status check on every PR), every commit -- human-authored or bot-authored -- must carry the trailer. Without it the merge is blocked. The author needs to amend with git commit --amend --signoff and force-push.

A's commit section checked type, scope, subject length, and casing, but did not verify the DCO trailer.

Verdict alignment

A's COMMENTED state (not APPROVED) and the no-blocking-issues framing are inconsistent with the DCO gap, which is a hard merge blocker enforced by CI.

Convergence

Not aligned. A's no-blocking-issues conclusion is incorrect: the missing DCO trailer is a required-status-check failure that will prevent the merge. A needs to acknowledge this and update the verdict to request-changes. Once A does that, convergence is possible.

Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer A response (round 1)

B's single finding is correct: the commit is missing its DCO Signed-off-by trailer. I verified this via git show 7261371cbe985deb79835abc0cf91a1e9debb1c2 — the body ends at the Co-Authored-By line with no sign-off. That is a hard merge blocker enforced by the DCO GitHub App (a required status check). My initial review checked commit type, scope, subject length, and casing but did not verify the trailer. B caught a real gap. I am updating my verdict to request-changes accordingly.

Conceded points

  • Missing DCO Signed-off-by trailer. Commit 7261371cb has no Signed-off-by: Author Name <email> line. The DCO GitHub App is a required status check; the merge is blocked until the author rebases with git commit --amend --signoff (or equivalent) and force-pushes. My initial review's "no blocking issues" conclusion was wrong.

Defended points

  • All substantive technical findings remain correct. The root-cause analysis (Scoop's pre_install → bin-shim → post_install ordering), the five-file consistency check, the validate.py update, and the CHANGELOG nit are all still accurate.
  • B found no false positives in my review, and I agree with that assessment.

Updated verdict

Request changes — the missing DCO trailer is a required-status-check failure that blocks the merge; the author must amend the commit to add Signed-off-by before this PR can land.

@bpamiri bpamiri merged commit e7445bc into develop May 12, 2026
8 checks passed
@bpamiri bpamiri deleted the claude/optimistic-goldstine-a1357b branch May 12, 2026 05:03
@bpamiri bpamiri mentioned this pull request May 12, 2026
5 tasks
bpamiri added a commit that referenced this pull request May 12, 2026
Captures the user-visible impact ("scoop install fails with Can't shim
wheels.cmd") and the actual fix (move launcher emit from post_install to
pre_install, where Scoop's install order requires it). Already merged on
develop as commit e7445bc; this just makes sure the GA changelog
reflects it under v4.0.0's Fixed section.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bpamiri added a commit that referenced this pull request May 12, 2026
Captures the user-visible impact ("scoop install fails with Can't shim
wheels.cmd") and the actual fix (move launcher emit from post_install to
pre_install, where Scoop's install order requires it). Already merged on
develop as commit e7445bc; this just makes sure the GA changelog
reflects it under v4.0.0's Fixed section.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
bpamiri added a commit that referenced this pull request May 12, 2026
* chore: prep CHANGELOG for v4.0.0 GA

- Promote [Unreleased] heading to # [4.0.0] with GitHub release link and
  date 2026-05-11 (replace `=> TBD` so release.yml validation passes).
- Update PR count (185 → 260+) and contributor list (+ @MukundaKatta) to
  match docs/releases/wheels-4.0-audit.md.
- Add new "Distribution" subsection under Added — captures the four
  install channels (Homebrew, Scoop, .deb/.rpm, WinGet drafts) that
  landed in 4.0 but weren't itemized in the changelog.
- Fix the section terminator after 4.0.0's notes from `----` (4 dashes)
  to `---` (3 dashes) so release.yml's release-notes awk extraction
  (`awk '/^# \[VERSION\]/,/^---$/'`) terminates at the right boundary
  instead of running into the 3.0.0 entry.

wheels.json is already at "4.0.0" (no snapshot suffix), so the
release.yml clean-source-version check passes. First-party packages all
declare wheelsVersion >=4.0 so no coordination updates are needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

* docs(changelog): add Fixed entry for #2603 Scoop pre_install fix

Captures the user-visible impact ("scoop install fails with Can't shim
wheels.cmd") and the actual fix (move launcher emit from post_install to
pre_install, where Scoop's install order requires it). Already merged on
develop as commit e7445bc; this just makes sure the GA changelog
reflects it under v4.0.0's Fixed section.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

* docs(changelog): address Reviewer A/B consensus findings (round 1)

- Fix Homebrew tap link text from `wheels-dev/wheels` to
  `wheels-dev/homebrew-wheels` so the visible repo name matches the
  URL target (CHANGELOG.md:104).
- Bump the 4.0.0 release date marker from `2026-05-11` to
  `2026-05-12` so it reflects today's tag day (CHANGELOG.md:21).

Not addressed in this commit:
- Missing `Signed-off-by:` trailers on `bff31f981` and `d971cde8e`.
  Adding them requires `git commit --amend -s` (or interactive
  rebase) plus a force-push, both of which the wheels-bot rails
  forbid. Flagged to the human in the address-review comment so a
  maintainer can amend the trailer chain before the FF-merge to main.

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

---------

Signed-off-by: Peter Amiri <peter@alurium.com>
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant