-
Notifications
You must be signed in to change notification settings - Fork 2
fix(restore): install APM and unpack via apm CLI to avoid dirtying workspace #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,13 +87,40 @@ export async function run(): Promise<void> { | |
| } | ||
| } | ||
|
|
||
| // RESTORE MODE: extract bundle, skip APM installation entirely. | ||
| // RESTORE MODE: install APM, then extract via `apm unpack`. | ||
|
||
| // Directory was already created above (actionOwnsDir = true for bundle mode). | ||
| // | ||
| // Why install APM in restore mode: | ||
| // `apm unpack` honors the bundle contract — it copies only files listed in | ||
| // the lockfile's `deployed_files` (primitives + apm_modules) and never | ||
| // writes `apm.lock.yaml` / `apm.yml` to `working-directory`. The previous | ||
| // "skip install" optimization forced extractBundle through its raw | ||
| // `tar xzf --strip-components=1` fallback, which dumped the *entire* | ||
| // bundle — including lockfile and apm.yml — into working-directory. | ||
| // When working-directory was a git checkout (the default | ||
| // `${{ github.workspace }}`), those tracked files became dirty and any | ||
| // subsequent `git checkout` (e.g. gh-aw's pull_request_target PR-branch | ||
| // checkout) aborted with: | ||
| // error: Your local changes to the following files would be | ||
| // overwritten by checkout: apm.lock.yaml | ||
| // See microsoft/apm-action#26. | ||
| // | ||
| // The install is tool-cached (see installer.ts), so this adds at most a | ||
| // single small download per runner — negligible vs. the cost of a typical | ||
| // agent job, and we get bundle integrity verification for free. | ||
| if (bundleInput) { | ||
| await ensureApmInstalled(); | ||
|
|
||
| const bundlePath = await resolveLocalBundle(bundleInput, resolvedDir); | ||
| core.info(`Restoring bundle: ${bundlePath}`); | ||
| const result = await extractBundle(bundlePath, resolvedDir); | ||
| const verifiedMsg = result.verified ? ' (verified)' : ' (unverified — install APM for integrity checks)'; | ||
| // Restore mode now installs APM up-front, so the verified `apm unpack` | ||
| // path is the expected outcome. The unverified branch only runs if APM | ||
| // install failed transiently and extractBundle fell through to its tar | ||
| // fallback — point operators at the install logs, not at re-installing. | ||
| const verifiedMsg = result.verified | ||
| ? ' (verified)' | ||
| : ' (unverified — APM install did not complete; see earlier install logs)'; | ||
| core.info(`Restored ${result.files} file(s)${verifiedMsg}`); | ||
|
|
||
| const primitivesPath = path.join(resolvedDir, '.github'); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
README now documents that
bundlerestore installs APM and unpacks viaapm unpack, butaction.ymlstill describesbundleas “Skips APM installation entirely.” Please updateaction.ymlto match so Marketplace/input docs aren’t misleading.