[UEPR-535] Combine add-repo.sh and CI generator scripts from #434 and #505#571
Open
cwillisf wants to merge 2 commits into
Open
[UEPR-535] Combine add-repo.sh and CI generator scripts from #434 and #505#571cwillisf wants to merge 2 commits into
cwillisf wants to merge 2 commits into
Conversation
Pure rename, no content changes. Preserves git blame for the shared helpers across the upcoming rewrite that adapts this script from "build the whole monorepo" to "add one repo at a time".
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates legacy monorepo-import and CI-generation tooling into a single canonical repo-import script plus a new GitHub Actions workflow updater, removing the prior CircleCI-era generator and templates.
Changes:
- Add
scripts/add-repo.shto import an external Scratch Foundation repo intopackages/<repo>/, rewire workspace deps, update root workspaces order, and optionally regenerate CI files. - Add
scripts/update-gha-workflows.tsto regenerate.github/path-filters.ymland incrementally extend.github/workflows/publish.ymlwith missing workspace publish steps. - Remove obsolete scripts/templates (
scripts/build-monorepo.sh,scripts/build-gha-workflows.ts,scripts/workspace-template.yml) and rewirenpm run refresh-gh-workflowaccordingly.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/add-repo.sh | New canonical script for importing a repo into the monorepo and performing dependency/workspace/CI fixups. |
| scripts/update-gha-workflows.ts | New generator/updater for path filters and incremental publish workflow step insertion. |
| package.json | Points refresh-gh-workflow to the new generator; removes legacy script entry. |
| .github/path-filters.yml | Updates generated header and workspace ordering to match new generator output. |
| .gitignore | Replaces legacy monorepo tmp artifacts with add-repo tmp/error outputs. |
| scripts/build-monorepo.sh | Removed legacy monorepo build/import script. |
| scripts/build-gha-workflows.ts | Removed CircleCI-era workflow generator. |
| scripts/workspace-template.yml | Removed template used by the old generator. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cwillisf
force-pushed
the
feat/canonical-add-repo-script
branch
from
9232234 to
abfdab8
Compare
Replaces the script work of two prior PRs that independently solved "add an existing scratchfoundation repo to the monorepo": scripts/add-repo.sh — collapses add-to-monorepo.sh (#434) and add-repo.sh (#505) into a single canonical entry point. Takes the CLI shape and auto-CI-regen from #505 and the source-branch auto-detect (develop -> main -> master), normalization (sort-package-json), and clean-tree pre-flight from #434. scripts/update-gha-workflows.ts — replaces the CircleCI-era build-gha-workflows.ts and workspace-template.yml. Regenerates .github/path-filters.yml and incrementally updates publish.yml. Open review comments on #505 are addressed: - sed -i portability bug: rewrites now use perl -pi -e (works on macOS, NixOS, Ubuntu/Debian/Arch, WSL). Perl is added to the explicit prereq check next to git-filter-repo, sponge, and jq. - Arg parser now rejects flag-stealing: --source-branch --org foo errors instead of silently using --org as the branch name. - npm-install failures hard-fail by default. --continue-on-error opts into the prior soft-fail behavior; failures are still logged to add-repo.errors.log when continuing. Scoped to per-dep package.json rewrite failures in the rewire step; the final lockfile install always hard-fails by design. - The submodules-aware branch in move_repository_subdirectory keeps a TODO comment marking that it has never been exercised against a real submodules-bearing repo and should be run carefully when one lands. Cross-workspace dep handling: rewires both bare-name and already-prefixed @scratch/<name> deps in the new package to exact-pinned current monorepo versions. Matches the existing convention in scratch-gui/scratch-vm/ scratch-render. npm's workspace: protocol would be the ideal here but npm does not actually support it (npm/cli#8845 - EUNSUPPORTEDPROTOCOL at install time). Workspaces array ordering: the new package is inserted just after the latest existing workspace it depends on, so 'npm run --workspaces build' builds its deps first. Falls back to position 0 (prepend) when the new package has no monorepo deps. This is a "last dep wins" heuristic, not a full topological sort; it relies on the existing workspaces array already being in valid build order. A real topo sort can be added later (e.g. by extending update-gha-workflows.ts) if it ever becomes necessary. bash 3.2 compatibility: macOS ships bash 3.2 at /bin/bash, which lacks the 'mapfile' builtin. The find/grep output is collected via a 'while IFS= read -r' loop so the script runs on macOS as well as Linux. Bare-clone alternates path: bare clones store the alternates file at objects/info/alternates, not .git/objects/info/alternates. The previous path was a no-op; the working disconnection comes from --dissociate in the clone command. Path corrected so the line is defensive belt-and- braces rather than dead code. update-gha-workflows.ts: throws with a clear error if npm query returns a workspace not present in the root "workspaces" array (previously would silently misorder via indexOf returning -1). The comment in resolveWorkspaces is corrected to reflect that the function returns Map insertion order, with the caller re-sorting into declared order. The npm query exec call now requests a 64 MiB maxBuffer; Node's default 1 MiB is fine for the current repo size but would silently truncate as the monorepo grows. Path anchoring: BUILD_TMP, BUILD_CACHE, and any user-supplied --cache-dir value are resolved to absolute paths up front. A CWD-relative default would have put add-repo.tmp outside the repo (and outside the .gitignore pattern) if the script were run from a subdirectory of the monorepo. No repo additions are included; this PR is script-tooling only.
cwillisf
force-pushed
the
feat/canonical-add-repo-script
branch
from
abfdab8 to
3fdfd68
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves
UEPR-535 (script work)
Proposed Changes
scripts/add-repo.sh, a single canonical script for adding a scratchfoundation repository into the monorepo. Combinesadd-to-monorepo.shfrom Addscratch-storageto the monorepo #434 andadd-repo.shfrom Feat/uepr 535 introduce script for migrating packages to monorepo #505.scripts/update-gha-workflows.tsfrom Feat/uepr 535 introduce script for migrating packages to monorepo #505 to regenerate.github/path-filters.ymland incrementally update.github/workflows/publish.yml. Remove the CircleCI-erascripts/build-gha-workflows.tsandscripts/workspace-template.yml. Updatepackage.json'srefresh-gh-workflowto point at the new file.sed -iportability: in-place rewrites now useperl -pi -e, which works on macOS, NixOS, Ubuntu, Debian, Arch, and WSL.--source-branch --org foo).npm installfailures hard-fail by default.--continue-on-erroropts into the prior log-and-continue behavior. Failures are still recorded inadd-repo.errors.log.move_repository_subdirectorycarries aTODOnoting it has never been exercised against a real submodules-bearing repo.@scratch/prefix. Pin to the current monorepo version, matching the existing convention in scratch-gui, scratch-vm, and scratch-render. (workspace:*would be the ideal here, but npm/cli#8845 reportsEUNSUPPORTEDPROTOCOLat install time.)workspacesarray right after the latest existing workspace it depends on, sonpm run --workspaces buildbuilds dependencies first.mapfile(bash 4+) with awhile IFS= read -rloop.Reason for Changes
#434 and #505 each had real strengths and an opposite cross-platform bug. Neither merged. Combining them now produces a single tool that is strictly better than either alone, before the next monorepo addition is attempted.
This PR supersedes the script work in both #434 and #505. It does not include #434's scratch-storage merge; that can be re-done on top of this PR using the canonical script.
Test Coverage
Local end-to-end runs on Linux (NixOS) and macOS with
scratch-paintas the import target:packages/scratch-paintinserted into rootworkspacesafterscratch-svg-renderer(correct build order).npm lsreports noinvalid:entries; all cross-workspace deps dedupe to workspace versions.npm run buildsucceeds forscratch-svg-renderer.npm run lint --workspace=@scratch/scratch-paintreports 0 errors (155 warnings are pre-existing in scratch-paint's source).npm run builddoes not currently succeed forscratch-paint. Target repositories may need preparation work in their own toolchains before being added to the monorepo; in scratch-paint's case the webpack/babel configuration needs to be updated to handle modern JS in built workspace dependencies. Other target repos may need different preparation. This is upstream of the script's job.Local dogfood state was discarded after verification; this PR contains script-only changes.