chore(scoop): remove stale draft manifests; bucket repo is authoritative#2767
Conversation
The Scoop draft generator at tools/distribution-drafts/scoop/ produced two .json manifests intended to be copied into wheels-dev/scoop-wheels at bucket setup time. After the inline-JDK rework in scoop-wheels@3f22250 (May 18), the bucket grew its own self-hosted autoupdate workflow and diverged from this draft on every meaningful axis: - inline JDK as 5th URL vs. external `depends: java/openjdk21` - real auto-bumped hashes vs. placeholder zeros - self-hosted autoupdate workflow vs. Scoop built-in `autoupdate` block - JAVA_HOME fallback + direct `java -jar` invocation in wrapper (scoop-wheels#6) vs. `call lucli.bat` here The parent README originally framed this directory as a "useful diff signal during release reviews" -- in practice nobody acted on the divergence between May 18 and today (two snapshot releases later), and the stale drafts only added a misleading second source-of-truth. wheels#2765 surfaced the gap when a user hit a wrapper bug that had already been quietly fixed-then-undone in the divergence. Delete the scoop subdirectory and trim the two scoop rows + the keep-or-remove paragraph from the parent README. Bucket is now the unambiguous home; structural changes happen there directly. No CI consumed these files -- release.yml only references linux-packages/apt-repo/yum-repo from this tree. scoop-wheels#6 (the wrapper fix) and scoop-wheels#7 (tracking) remain valid in the bucket repo. Closes wheels#2765 Signed-off-by: Peter Amiri <peter@alurium.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: PR #2767 removes the stale tools/distribution-drafts/scoop/ subdirectory (5 files, ~1050 lines) and trims the matching rows and rationale paragraph from the parent README. The change is correct, the rationale is solid, and nothing in CI or release.yml references the deleted paths. Approving.
Correctness
The diff does exactly what the PR description says. A quick cross-check confirms no CI workflow touches the deleted paths:
grep -r "distribution-drafts/scoop" .github/— no matches.release.ymlonly referenceslinux-packages/,apt-repo/,yum-repo/from this tree.- The parent README update is self-consistent: the two scoop table rows are removed and the new section accurately explains why the bucket repo is now authoritative.
Docs
tools/distribution-drafts/README.md is updated in the same commit and the prose is clear. No user-facing framework behaviour changed, so no .ai/wheels/ or web/sites/guides/ pages need updating.
Commits
Single commit: chore(scoop): remove stale draft manifests; bucket repo is authoritative
- Type
choreis valid percommitlint.config.js. Scopescoopis a fine optional qualifier. - Subject is 55 chars, sentence-case, no ALL-CAPS. Body fully explains the divergence timeline and closes the originating issue.
- DCO
Signed-off-by: Peter Amiri <peter@alurium.com>is present.
One heads-up
mergeStateStatus is DIRTY, meaning the chore/remove-stale-scoop-drafts branch has conflicts with the current develop HEAD (likely because some recent hotfix commits landed on main out-of-order). A git rebase origin/develop before merge should resolve this cleanly — the deleted files have not been touched on develop, so the rebase should be trivial.
…-scoop-drafts Signed-off-by: Peter Amiri <peter@alurium.com> # Conflicts: # tools/distribution-drafts/scoop/build-manifests.py # tools/distribution-drafts/scoop/wheels-be.json # tools/distribution-drafts/scoop/wheels.json
|
Resolved merge conflict with #2766 (merged into develop while this PR was open). The conflict was strategic, not just textual — #2766 took the "keep the drafts and patch them to match the wrapper fix" approach to the same root cause (#2765). Per discussion, the project's preferred end state is a clean slate where the live
Net diff vs develop: 12 files, +13 / −1296. Closes #2765 |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: PR #2767 started as a five-file deletion of the stale Scoop draft manifests, then gained a develop merge that pulled in four docs updates, a CHANGELOG consolidation, and the deletion of ScoopWrapperSpec.cfc. The core deletion is correct and the docs changes are internally consistent; there are no correctness, cross-engine, or security findings. Commenting with two minor observations before merge.
Correctness
The deletions are clean. A cross-check confirms no CI workflow references the removed paths; release.yml only touches linux-packages/, apt-repo/, and yum-repo/ from this tree.
The develop merge (head commit d6b456b5) resolved three conflicts in the files being deleted — the conflict markers are gone, the files are absent, and the merge commit is consistent with the PR intent.
The docs updates across v4-0-0 and v4-0-1-snapshot are symmetric and consistent with the CHANGELOG entry: the canonical bucket now bundles OpenJDK 21 at %~dp0jdk-21.0.2 (described as inline rather than via depends: java/openjdk21), so removing the scoop bucket add java prerequisite step is correct.
Tests
vendor/wheels/tests/specs/cli/ScoopWrapperSpec.cfc is deleted. The spec was correct to remove — every it block opens with:
expect(fileExists(t.path)).toBeTrue("Missing file: " & t.path);Keeping it with the draft JSONs gone would fail the entire cli/ suite bundle immediately. Deletion is the only option.
Worth noting: this removes the only in-repo regression guard against the two bugs from issue 2765 (the call lucli.bat pre-parser crash and the dead JAVA_HOME=%~dp0share\jdk assignment). The canonical bucket is now the sole source of truth for wrapper correctness; there is no equivalent regression spec in the bucket repo. This is an accepted trade-off, but future bucket maintainers lose that safety net silently.
Docs
The four installation.mdx / installing.mdx changes correctly drop the scoop bucket add java step and update the prose from "installs OpenJDK 21 as a Scoop dependency" to "bundles OpenJDK 21 inline." Both v4-0-0 and v4-0-1-snapshot copies are updated in the same diff, which is correct.
Commits
7c35239d—chore(scoop): remove stale draft manifests; bucket repo is authoritative— valid type, optional scope, 55-char subject, exemplary body (divergence timeline, no-CI-consumers confirmation, bucket rationale). DCOSigned-off-by: Peter Amiri <peter@alurium.com>present.d6b456b5— merge commit, standard subject, DCO sign-off present.
One pre-merge housekeeping note
The PR description was written against the original five-file deletion commit and was not updated after the develop merge. The actual diff now includes ScoopWrapperSpec.cfc, four docs files, and a CHANGELOG edit that are not mentioned in the description. This is cosmetic — the diff and commits tell the full story — but updating the PR body to reflect the full scope before merge would help anyone reading the PR summary after the fact.
Wheels Bot — Reviewer B (round 1)A's review is substantiated and catches the important angles (CI non-consumption, test deletion rationale, docs symmetry). One factual inconsistency in the CHANGELOG was missed; everything else checks out. SycophancyNone detected. A chose False positivesNone detected.
Missed issues
The diff only rewrites line 40 (the scoop wrapper bug description). It leaves untouched the bullet added by #2761 (context line in the diff):
This PR removes Fix: remove or replace the #2761 docs bullet in Verdict alignmentA's ConvergenceNot aligned. A needs to address the stale #2761 CHANGELOG bullet that contradicts the docs revert in this PR. Once that line is removed or amended, the PR is clean and can merge. No code, no cross-engine concerns, no test coverage gaps beyond what A already noted. |
There was a problem hiding this comment.
Wheels Bot — Reviewer A response (round 1)
B caught a real issue I missed: the #2761 CHANGELOG bullet is left untouched by this PR, and after merge it directly contradicts what the docs now say. I concede that finding and update my verdict to request-changes. My other observations (CI non-consumption, test deletion rationale, docs symmetry) were confirmed accurate by B and stand unchanged.
Conceded points
- CHANGELOG.md #2761 bullet contradiction. The diff removes
scoop bucket add javafrom the four.mdxinstall files (confirmed: all four- scoop bucket add javaremovals are visible in the diff). The untouched context line in the diff reads: "Docs: Windows install steps instart-here/installing.mdxandcommand-line-tools/installation.mdxnow call outscoop bucket add javaas a prerequisite." After merge, the release notes simultaneously say that step was added and the actual docs omit it. A reader following CHANGELOG guidance against the live bucket would hit the sameCouldn't find manifest for 'openjdk21'error that bullet was written to prevent. The fix is to remove or replace that bullet — the net docs state for this release does not includescoop bucket add java.
Defended points
- CI non-consumption.
release.ymland related workflows reference onlylinux-packages/,apt-repo/,yum-repo/from this tree — nodistribution-drafts/scoop/reference. The deleted paths are not consumed by any CI step. B confirmed this; no change to position. ScoopWrapperSpec.cfcdeletion. Everyitblock opens withexpect(fileExists(t.path)).toBeTrue(...)against paths undertools/distribution-drafts/scoop/. Keeping the spec with those files gone would crash the entirecli/bundle on first compile. Deletion is the only viable option; the trade-off (loss of regression guard) is accepted and noted. B confirmed this; no change to position.- Docs symmetry. Both v4-0-0 and v4-0-1-snapshot copies of
installing.mdx/installation.mdxare updated in the same diff. The inline-JDK description is consistent across all four changed files. B confirmed this; no change to position.
Updated verdict
Request changes: the #2761 CHANGELOG bullet must be removed or amended (the net docs state for this release does not include scoop bucket add java) before the PR is clean to merge. All other findings are resolved.
Summary
tools/distribution-drafts/scoop/(5 files, ~1050 lines): thebuild-manifests.pygenerator,validate.pystatic checker, and the two.jsondrafts plus their README.tools/distribution-drafts/README.mdtable and replace the "keep-or-remove for drift signal" paragraph with a brief note explaining why scoop is no longer drafted here.Rationale captured in the commit message; short version: the live
wheels-dev/scoop-wheelsbucket has had its own self-hosted autoupdate workflow sincescoop-wheels@3f22250(May 18) and diverged from this draft on every meaningful axis (inline JDK, real hashes, autoupdate strategy, and now the wrapper fix from scoop-wheels#6). The parent README originally framed keeping the drafts as a "useful diff signal" — empirically that signal was ignored for two snapshot releases, and the stale drafts only confused the recent debug session that surfaced as wheels#2765.Test plan
grep -r distribution-drafts/scooponly matchesbuild-manifests.pyitself and the parent README).release.ymlonly referenceslinux-packages/,apt-repo/,yum-repo/from this tree.wheels-dev/scoop-wheels#6(the wrapper fix) is already merged and verified end-to-end on a clean reinstall (see comment thread on wheels#2765).tools/distribution-drafts/README.mdstill scans cleanly — only the scoop entries removed.Closes #2765