From c07a4e02f30a97045bb8f44ff484a5a41c85a221 Mon Sep 17 00:00:00 2001 From: Johannes Hoppe Date: Wed, 22 Apr 2026 23:27:03 +0200 Subject: [PATCH 1/2] fix: clean up leftover gh-pages branch files (dotfiles, submodules) gh-pages@6.3.0's "Removing files" step calls globby without `dot: true`, so dotfiles (.gitignore, .gitmodules, .github/) and submodule gitlinks from the gh-pages branch are not removed before our dist is copied on top. They then get re-committed and leak into the deploy. Upstream fix tschaub/gh-pages#612 (merged 2025-08-09) adds both `remove: '**/*'` and `dot: true`, but is unreleased as of v6.3.0. We can't reach the globby call from options alone. Workaround: register a `beforeAdd` hook that runs after gh-pages' broken remove + our file copy. The hook asks git what it still has indexed (`git ls-files -z`), diffs against the set of files in our dist, and `git rm`s the leftovers. `git rm` handles submodule gitlinks correctly, so the `build` gitlink from the reporter's scenario is also cleaned up. Skipped when the user opts into `add: true` (additive mode explicitly preserves existing files). Adds a spawn-level regression test in engine.gh-pages-behavior.spec.ts that mocks `git ls-files` to return leftover dotfiles + a submodule gitlink, runs engine.run() end-to-end through real gh-pages, and asserts `git rm` targets the leftovers while leaving dist files alone. Fixes #204 --- src/engine/engine.gh-pages-behavior.spec.ts | 85 +++++++++++++++++++- src/engine/engine.prepare-options-helpers.ts | 82 +++++++++++++++++++ src/engine/engine.ts | 9 ++- src/interfaces.ts | 1 + src/package-lock.json | 4 +- src/package.json | 2 +- 6 files changed, 177 insertions(+), 6 deletions(-) diff --git a/src/engine/engine.gh-pages-behavior.spec.ts b/src/engine/engine.gh-pages-behavior.spec.ts index 67fcdbf..c049528 100644 --- a/src/engine/engine.gh-pages-behavior.spec.ts +++ b/src/engine/engine.gh-pages-behavior.spec.ts @@ -53,7 +53,7 @@ function createMockChildProcess(): Partial { * This is intentional - we want to know about any new git operations. */ const EXPECTED_GIT_COMMANDS = [ - 'clone', 'clean', 'fetch', 'checkout', 'ls-remote', 'reset', + 'clone', 'clean', 'fetch', 'checkout', 'ls-remote', 'ls-files', 'reset', 'rm', 'add', 'config', 'diff-index', 'commit', 'tag', 'push', 'update-ref' ]; @@ -1066,4 +1066,87 @@ describe('gh-pages v6.3.0 - behavioral snapshot', () => { expect(spawnCalls[0]?.args[0]).toBe('clone'); }); }); + + /** + * End-to-end regression for issue #204: gh-pages@6.3.0's "Removing files" step + * skips dotfiles and submodule gitlinks. Our beforeAdd hook should catch them + * via `git ls-files` and `git rm` the leftovers while leaving dist files alone. + */ + describe('submodule / dotfile cleanup regression (issue #204)', () => { + const engine = require('./engine'); + const { cleanupMonkeypatch } = require('./engine.prepare-options-helpers'); + const { logging } = require('@angular-devkit/core'); + + beforeEach(() => { + cleanupMonkeypatch(); + }); + + it('engine.run() should git rm leftover gh-pages branch files not present in dist', async () => { + const repo = 'https://github.com/owner/leftover-test.git'; + currentTestContext.repo = repo; + + const leftoverFiles = [ + '.github/workflows/deploy.yml', + '.gitignore', + '.gitmodules', + 'build' // submodule gitlink + ]; + + // Reconfigure the default mockSpawn so `git ls-files` returns our leftover set. + // Other git commands keep the outer describe's default behavior. + mockSpawn.mockImplementation((cmd: string, args: string[] | undefined, opts: unknown) => { + const capturedArgs = args || []; + spawnCalls.push({ cmd, args: capturedArgs, options: opts }); + + if (cmd === 'git' && capturedArgs[0] && !EXPECTED_GIT_COMMANDS.includes(capturedArgs[0])) { + throw new Error(`Unexpected git command: ${capturedArgs[0]}. Add to whitelist if intentional.`); + } + + const child = createMockChildProcess(); + setImmediate(() => { + let output = ''; + if (cmd === 'git' && capturedArgs[0] === 'ls-files') { + output = leftoverFiles.join('\0') + '\0'; + } else if (cmd === 'git' && capturedArgs[0] === 'config' && + capturedArgs[1] === '--get' && capturedArgs[2]?.startsWith('remote.')) { + output = repo; + } else if (cmd === 'git' && capturedArgs[0] === 'ls-remote') { + output = 'refs/heads/gh-pages'; + } else if (cmd === 'git' && capturedArgs[0] === 'diff-index') { + child.emit!('close', 1); + return; + } + child.stdout!.emit('data', Buffer.from(output)); + child.emit!('close', 0); + }); + return child; + }); + + const options = { + repo, + branch: 'gh-pages', + dotfiles: true, + notfound: false, + nojekyll: false + }; + + await engine.run(basePath, options, new logging.NullLogger()); + + // Our hook should have issued `git ls-files -z`. + const lsFilesCall = spawnCalls.find(c => c.cmd === 'git' && c.args[0] === 'ls-files'); + expect(lsFilesCall).toBeDefined(); + expect(lsFilesCall!.args).toContain('-z'); + + // And a subsequent `git rm` that targets exactly the leftovers. + const rmCalls = spawnCalls.filter(c => c.cmd === 'git' && c.args[0] === 'rm'); + const cleanupCall = rmCalls.find(c => leftoverFiles.every(f => c.args.includes(f))); + expect(cleanupCall).toBeDefined(); + + // Dist files (from the outer beforeAll: index.html, main.js, styles.css, .htaccess) + // must NOT be in the cleanup call — those are files we just copied. + for (const distFile of ['index.html', 'main.js', 'styles.css', '.htaccess']) { + expect(cleanupCall!.args).not.toContain(distFile); + } + }); + }); }); diff --git a/src/engine/engine.prepare-options-helpers.ts b/src/engine/engine.prepare-options-helpers.ts index 9e71451..1847375 100644 --- a/src/engine/engine.prepare-options-helpers.ts +++ b/src/engine/engine.prepare-options-helpers.ts @@ -6,6 +6,8 @@ */ import { logging } from '@angular-devkit/core'; +import * as fs from 'fs/promises'; +import * as path from 'path'; import * as util from 'util'; import { Schema } from '../deploy/schema'; @@ -225,6 +227,86 @@ export async function injectTokenIntoRepoUrl(options: PreparedOptions): Promise< } } +/** + * Minimal subset of the gh-pages internal Git class that our cleanup hook relies on. + * See src/node_modules/gh-pages/lib/git.js — `Git.prototype.exec`, `Git.prototype.rm`, `cwd`, `output`. + */ +export interface GhPagesGit { + cwd: string; + output: string; + exec(...args: string[]): Promise; + rm(files: string[]): Promise; +} + +/** + * Recursively walk a directory and return the set of relative file paths, + * using POSIX separators so the output matches what `git ls-files` prints. + * Honors the same dotfile-inclusion semantics as gh-pages' `options.dotfiles`. + */ +export async function collectDistFiles( + baseDir: string, + includeDotfiles: boolean +): Promise> { + const files = new Set(); + + async function walk(relDir: string): Promise { + const fullDir = relDir ? path.join(baseDir, relDir) : baseDir; + const entries = await fs.readdir(fullDir, { withFileTypes: true }); + for (const entry of entries) { + if (!includeDotfiles && entry.name.startsWith('.')) { + continue; + } + const relPath = relDir ? `${relDir}/${entry.name}` : entry.name; + if (entry.isDirectory()) { + await walk(relPath); + } else if (entry.isFile()) { + files.add(relPath); + } + } + } + + await walk(''); + return files; +} + +/** + * Create a `beforeAdd` hook that removes leftover files from the gh-pages branch + * before gh-pages stages our dist for commit. + * + * Why this exists (issue #204): + * gh-pages@6.3.0's "Removing files" step calls globby without `dot: true`, so + * dotfiles (.gitignore, .gitmodules, .github/…) and submodule gitlinks from the + * gh-pages branch are NOT removed before our dist is copied on top. They then + * get re-committed and leak into the deploy. + * + * Fix: after gh-pages' broken remove + our file copy, ask git what it still has + * indexed (`git ls-files -z`), diff against the set of files in our dist, and + * `git rm` the leftovers. `git rm` correctly handles submodule gitlinks too. + * + * Upstream fix: tschaub/gh-pages#612 (merged 2025-08-09, unreleased as of + * gh-pages@6.3.0). When a release containing that PR lands, this hook becomes + * redundant and can be removed. + */ +export function createCleanupBeforeAddHook( + distDir: string, + dotfiles: boolean, + logger: logging.LoggerApi +): (git: GhPagesGit) => Promise { + return async (git) => { + const distFiles = await collectDistFiles(distDir, dotfiles); + await git.exec('ls-files', '-z'); + const tracked = (git.output || '').split('\0').filter(Boolean); + const toRemove = tracked.filter((f) => !distFiles.has(f)); + if (toRemove.length === 0) { + return; + } + logger.info( + `Removing ${toRemove.length} leftover file(s) from gh-pages branch not in dist: ${toRemove.join(', ')}` + ); + await git.rm(toRemove); + }; +} + /** * Get the remote URL from the git repository * diff --git a/src/engine/engine.ts b/src/engine/engine.ts index 39766da..25b32d1 100644 --- a/src/engine/engine.ts +++ b/src/engine/engine.ts @@ -13,7 +13,8 @@ import { handleUserCredentials, warnDeprecatedParameters, appendCIMetadata, - injectTokenIntoRepoUrl + injectTokenIntoRepoUrl, + createCleanupBeforeAddHook } from './engine.prepare-options-helpers'; export async function run( @@ -194,7 +195,11 @@ async function publishViaGhPages( dotfiles: options.dotfiles, user: options.user, cname: options.cname, - nojekyll: options.nojekyll + nojekyll: options.nojekyll, + // Workaround for gh-pages#612 (unreleased in v6.3.0): clean up leftover + // gh-pages branch files (dotfiles, submodule gitlinks) that the broken + // remove step misses. Skipped when the user opts into additive mode. + beforeAdd: options.add ? undefined : createCleanupBeforeAddHook(dir, options.dotfiles, logger) }; // gh-pages@6 silently absorbs errors via its internal .then(_, onRejected) diff --git a/src/interfaces.ts b/src/interfaces.ts index 46c99d7..e7200ce 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -65,6 +65,7 @@ export interface PublishOptions { cname?: string; add?: boolean; git?: string; + beforeAdd?: (git: unknown) => void | Promise; [key: string]: unknown; // Allow additional gh-pages options } diff --git a/src/package-lock.json b/src/package-lock.json index cdb2895..e5abf7b 100644 --- a/src/package-lock.json +++ b/src/package-lock.json @@ -1,12 +1,12 @@ { "name": "angular-cli-ghpages", - "version": "3.0.3", + "version": "3.0.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "angular-cli-ghpages", - "version": "3.0.3", + "version": "3.0.4", "license": "MIT", "dependencies": { "@angular-devkit/architect": ">=0.1800.0 <0.2200.0", diff --git a/src/package.json b/src/package.json index 759a74b..a630357 100644 --- a/src/package.json +++ b/src/package.json @@ -1,6 +1,6 @@ { "name": "angular-cli-ghpages", - "version": "3.0.3", + "version": "3.0.4", "description": "Deploy your Angular app to GitHub Pages or Cloudflare Pages directly from the Angular CLI (ng deploy)", "main": "index.js", "types": "index.d.ts", From 8152c35b5b95d2ec6c11b0af39ebcae5b2718dc2 Mon Sep 17 00:00:00 2001 From: Johannes Hoppe Date: Wed, 22 Apr 2026 23:38:51 +0200 Subject: [PATCH 2/2] test: add end-to-end real-git integration test for #204 cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior tests covered the cleanup hook at the spawn-mock level only — they verified that the right git commands get issued, but not that the resulting gh-pages commit is actually clean. Add a real-filesystem, real-git test that: 1. Creates a local bare repo. 2. Seeds a gh-pages branch containing the exact conditions from #204: dotfiles (.gitignore, .gitmodules), nested dot-directory contents (.github/workflows/deploy.yml), a submodule gitlink, and a stale non-dot file. 3. Runs engine.run() (no mocks of child_process, fs, or gh-pages). 4. Inspects `git ls-tree -r gh-pages` on the bare repo and asserts the final tree contains ONLY index.html. Also adds a baseline test that calls gh-pages.publish() directly (bypassing our hook) and asserts the dotfiles + submodule DO leak — i.e. the upstream bug is real on the installed gh-pages version. If a future gh-pages release fixes the bug, that baseline test will fail as a clear signal that our workaround can be removed. No production changes; the hook implementation and its PR #206-era callback wrapper are exercised end-to-end by the new tests. --- src/engine/engine.gh-pages-cleanup.spec.ts | 190 +++++++++++++++++++++ 1 file changed, 190 insertions(+) create mode 100644 src/engine/engine.gh-pages-cleanup.spec.ts diff --git a/src/engine/engine.gh-pages-cleanup.spec.ts b/src/engine/engine.gh-pages-cleanup.spec.ts new file mode 100644 index 0000000..c734023 --- /dev/null +++ b/src/engine/engine.gh-pages-cleanup.spec.ts @@ -0,0 +1,190 @@ +/** + * End-to-end integration test for issue #204 — REAL git, REAL gh-pages, REAL filesystem. + * + * No mocks. This test proves the `beforeAdd` cleanup hook actually produces a clean + * gh-pages commit when the branch previously contained dotfiles and submodule gitlinks. + * + * Flow: + * 1. Create a local bare git repo (serves as "remote"). + * 2. Seed a gh-pages branch containing: + * - .github/workflows/deploy.yml (dot-directory, nested) + * - .gitignore (dotfile) + * - .gitmodules (dotfile) + * - a submodule gitlink `build` (mode 160000) + * - stale.html (regular file gh-pages' own remove would catch) + * 3. Run `engine.run()` against the bare repo with a dist containing only index.html. + * 4. `git ls-tree -r gh-pages` on the bare repo → assert ONLY `index.html` landed. + * + * A companion test demonstrates the upstream bug itself by calling `gh-pages.publish()` + * directly (bypassing our hook) and observing the leftovers leak into the commit. That + * test will start failing once tschaub/gh-pages ships PR #612 in a release — which is + * fine; it's the signal that our workaround can be removed. + */ + +import * as path from 'path'; +import * as fs from 'fs/promises'; +import * as os from 'os'; +import { execSync } from 'child_process'; + +import { logging } from '@angular-devkit/core'; + +import * as engine from './engine'; +import { cleanupMonkeypatch } from './engine.prepare-options-helpers'; + +// NO MOCKS — we want real gh-pages behavior end-to-end +const ghPages = require('gh-pages'); + +interface GitOptions { + readonly cwd: string; +} + +function git(args: string, options: GitOptions): string { + return execSync(`git -C "${options.cwd}" ${args}`, { stdio: ['pipe', 'pipe', 'pipe'] }) + .toString() + .trim(); +} + +async function seedGhPagesBranch(workDir: string, bareRepoPath: string): Promise { + await fs.mkdir(workDir, { recursive: true }); + execSync(`git init "${workDir}"`, { stdio: 'pipe' }); + git('config user.email "seed@test.com"', { cwd: workDir }); + git('config user.name "Seed"', { cwd: workDir }); + git('checkout -b gh-pages', { cwd: workDir }); + + // Dotfiles and dot-directory (what gh-pages' broken remove step misses) + await fs.mkdir(path.join(workDir, '.github', 'workflows'), { recursive: true }); + await fs.writeFile(path.join(workDir, '.github', 'workflows', 'deploy.yml'), 'name: deploy\n'); + await fs.writeFile(path.join(workDir, '.gitignore'), 'node_modules\n'); + await fs.writeFile( + path.join(workDir, '.gitmodules'), + '[submodule "build"]\n\tpath = build\n\turl = https://example.invalid/build.git\n' + ); + // A regular non-dot file — gh-pages' own remove step WILL catch this one. + await fs.writeFile(path.join(workDir, 'stale.html'), 'stale\n'); + + git('add .github .gitignore .gitmodules stale.html', { cwd: workDir }); + git('commit -m "seed dotfiles and stale content"', { cwd: workDir }); + + // Add a submodule gitlink without needing a real subrepo. + // update-index --cacheinfo creates the 160000 entry directly in the index. + const seedSha = git('rev-parse HEAD', { cwd: workDir }); + git(`update-index --add --cacheinfo 160000,${seedSha},build`, { cwd: workDir }); + git('commit -m "add submodule gitlink"', { cwd: workDir }); + + git(`remote add origin "${bareRepoPath}"`, { cwd: workDir }); + git('push origin gh-pages', { cwd: workDir }); +} + +describe('end-to-end cleanup regression (issue #204, real git)', () => { + let tempDir: string; + let distDir: string; + let bareRepoPath: string; + let originalEnv: NodeJS.ProcessEnv; + + // Unique suffix so parallel-ish reruns never collide in /tmp. + const testRunId = `${Date.now()}-${Math.random().toString(36).substring(7)}`; + + beforeEach(async () => { + cleanupMonkeypatch(); + + // Isolate from ambient CI envs and tokens so engine.prepareOptions doesn't + // rewrite the repo URL or append CI metadata during tests. + originalEnv = { ...process.env }; + delete process.env.TRAVIS; + delete process.env.CIRCLECI; + delete process.env.GITHUB_ACTIONS; + delete process.env.GH_TOKEN; + delete process.env.PERSONAL_TOKEN; + delete process.env.GITHUB_TOKEN; + + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), `ghp-204-${testRunId}-`)); + + distDir = path.join(tempDir, 'dist'); + await fs.mkdir(distDir, { recursive: true }); + await fs.writeFile(path.join(distDir, 'index.html'), 'fresh dist\n'); + + bareRepoPath = path.join(tempDir, 'bare.git'); + execSync(`git init --bare "${bareRepoPath}"`, { stdio: 'pipe' }); + + await seedGhPagesBranch(path.join(tempDir, 'seed'), bareRepoPath); + + // Clear gh-pages' cache so each test starts from a clean clone. + ghPages.clean(); + }); + + afterEach(async () => { + ghPages.clean(); + cleanupMonkeypatch(); + process.env = originalEnv; + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it('engine.run() produces a gh-pages commit containing ONLY dist files (no dotfiles, no submodule)', async () => { + await engine.run( + distDir, + { + repo: bareRepoPath, + branch: 'gh-pages', + dotfiles: true, + // Keep assertions simple: don't let our own 404.html / .nojekyll machinery + // add files to the expected set. + notfound: false, + nojekyll: false, + name: 'Test', + email: 'test@test.com', + message: 'test deploy' + }, + new logging.NullLogger() + ); + + // Inspect what actually landed on gh-pages in the bare repo. + const tree = git('ls-tree -r gh-pages --name-only', { cwd: bareRepoPath }) + .split('\n') + .filter(Boolean) + .sort(); + + expect(tree).toEqual(['index.html']); + + // Explicit negative checks — makes regressions very readable on failure. + expect(tree).not.toContain('.gitignore'); + expect(tree).not.toContain('.gitmodules'); + expect(tree).not.toContain('.github/workflows/deploy.yml'); + expect(tree).not.toContain('build'); + expect(tree).not.toContain('stale.html'); + }, 30_000); + + it('baseline: without our hook, gh-pages alone leaks dotfiles and submodule gitlinks (demonstrates the upstream bug)', async () => { + // Call gh-pages.publish() directly — no engine.run(), no beforeAdd hook. + // This is exactly what angular-cli-ghpages v3 did before our fix. + await new Promise((resolve, reject) => { + ghPages.publish( + distDir, + { + repo: bareRepoPath, + branch: 'gh-pages', + dotfiles: true, + user: { name: 'Test', email: 'test@test.com' }, + message: 'baseline: unhooked gh-pages publish' + }, + (err: Error | null) => { + if (err) reject(err); + else resolve(); + } + ); + }); + + const tree = git('ls-tree -r gh-pages --name-only', { cwd: bareRepoPath }) + .split('\n') + .filter(Boolean); + + // gh-pages' broken remove step missed all of these: + expect(tree).toContain('.gitignore'); + expect(tree).toContain('.gitmodules'); + expect(tree).toContain('.github/workflows/deploy.yml'); + expect(tree).toContain('build'); + // Our dist file did land: + expect(tree).toContain('index.html'); + // And the non-dot stale file DID get removed by gh-pages' (partial) remove step: + expect(tree).not.toContain('stale.html'); + }, 30_000); +});