diff --git a/packages/rstack/src/cli/commandHelp.ts b/packages/rstack/src/cli/commandHelp.ts index 2b6398e5..a844a1b4 100644 --- a/packages/rstack/src/cli/commandHelp.ts +++ b/packages/rstack/src/cli/commandHelp.ts @@ -498,6 +498,7 @@ const HELP_DEFINITIONS = { { title: 'Options', items: [ + ['-f, --force', 'Install despite an existing Git hooks setup'], [ '--hooks-dir ', 'Specify hooks directory relative to the Git repository root', diff --git a/packages/rstack/src/setup/index.ts b/packages/rstack/src/setup/index.ts index ef597323..5b37d61d 100644 --- a/packages/rstack/src/setup/index.ts +++ b/packages/rstack/src/setup/index.ts @@ -7,6 +7,7 @@ export const runSetupCLI = async (args: string[]): Promise => { const { values } = parseArgs({ args, options: { + force: { type: 'boolean', short: 'f' }, help: { type: 'boolean', short: 'h' }, 'hooks-dir': { type: 'string', multiple: true }, }, @@ -28,15 +29,47 @@ export const runSetupCLI = async (args: string[]): Promise => { return; } - const result = installHooks({ hooksDir }); + const result = installHooks({ force: values.force, hooksDir }); - if (result.status === 'installed' || result.status === 'unchanged') { + if (result.status === 'installed') { + // Warn when `--force` preserves an existing hooks setup but makes it inactive. + if (result.inactiveHooks) { + const { hooks, path, restore } = result.inactiveHooks; + const hooksMessage = hooks.length + ? `: ${color.yellow(hooks.join(', '))}` + : ''; + logger.warn( + `The previous Git hooks path "${color.yellow(path)}" is now inactive${hooksMessage}.`, + ); + + if (restore === 'unset') { + logger.info( + `The existing files were preserved and will become active again if ${color.yellow('core.hooksPath')} is unset.`, + ); + } else { + logger.info( + `The existing files were preserved. Set ${color.yellow('core.hooksPath')} back to this path to use them again.`, + ); + } + } + return; + } + + if (result.status === 'unchanged') { return; } if (result.status === 'skipped') { if (result.message) { logger.warn(`Git hooks setup skipped: ${color.yellow(result.message)}.`); + if ( + result.reason === 'existing-git-hooks' || + result.reason === 'hooks-path-conflict' + ) { + logger.info( + `To continue, run ${color.yellow('rs setup --force')}. Existing hook files will be preserved but become inactive.`, + ); + } return; } diff --git a/packages/rstack/src/setup/install.ts b/packages/rstack/src/setup/install.ts index eae7c186..92a65842 100644 --- a/packages/rstack/src/setup/install.ts +++ b/packages/rstack/src/setup/install.ts @@ -17,9 +17,16 @@ const gitignore = '*\n'; type InstallHooksOptions = { cwd?: string; + force?: boolean; hooksDir?: string; }; +type InactiveHooks = { + hooks: string[]; + path: string; + restore: 'configure' | 'unset'; +}; + type FailedInstallResult = { status: 'failed'; reason: string; @@ -33,7 +40,7 @@ type SkippedInstallResult = { }; type InstallResult = - | { status: 'installed'; hooksPath: string } + | { status: 'installed'; hooksPath: string; inactiveHooks?: InactiveHooks } | { status: 'unchanged'; hooksPath: string } | SkippedInstallResult | FailedInstallResult; @@ -252,6 +259,7 @@ const findExistingHooks = (directory: string): string[] => export const installHooks = ({ cwd = process.cwd(), + force = false, hooksDir = defaultHooksDir, }: InstallHooksOptions = {}): InstallResult => { if (process.env.RSTACK_HOOKS === '0') { @@ -282,16 +290,24 @@ export const installHooks = ({ effectiveHooksDirectory, defaultHooksDirectory, ); + let inactiveHooks: InactiveHooks | undefined; if (!hooksPathMatches && !usesDefaultHooks) { const activeOwner = readOwner(effectiveHooksDirectory); if (!activeOwner) { - return skip( - 'hooks-path-conflict', - `Git hooks are already configured at "${displayPath(gitRoot, effectiveHooksDirectory)}"`, - ); - } - if (activeOwner !== projectPath) { + if (!force) { + return skip( + 'hooks-path-conflict', + `Git hooks are already configured at "${displayPath(gitRoot, effectiveHooksDirectory)}"`, + ); + } + + inactiveHooks = { + hooks: findExistingHooks(effectiveHooksDirectory), + path: displayPath(gitRoot, effectiveHooksDirectory), + restore: 'configure', + }; + } else if (activeOwner !== projectPath) { return ownerConflict(activeOwner); } } @@ -299,10 +315,18 @@ export const installHooks = ({ if (usesDefaultHooks) { const existingHooks = findExistingHooks(defaultHooksDirectory); if (existingHooks.length > 0) { - return skip( - 'existing-git-hooks', - `existing Git hooks were found: ${existingHooks.join(', ')}`, - ); + if (!force) { + return skip( + 'existing-git-hooks', + `existing Git hooks were found: ${existingHooks.join(', ')}`, + ); + } + + inactiveHooks = { + hooks: existingHooks, + path: displayPath(gitRoot, defaultHooksDirectory), + restore: 'unset', + }; } } @@ -360,5 +384,9 @@ export const installHooks = ({ ); } - return { status: 'installed', hooksPath }; + return { + status: 'installed', + hooksPath, + ...(inactiveHooks ? { inactiveHooks } : {}), + }; }; diff --git a/packages/rstack/tests/cli/setup/__snapshots__/index.test.ts.snap b/packages/rstack/tests/cli/setup/__snapshots__/index.test.ts.snap index d238e172..71351935 100644 --- a/packages/rstack/tests/cli/setup/__snapshots__/index.test.ts.snap +++ b/packages/rstack/tests/cli/setup/__snapshots__/index.test.ts.snap @@ -9,6 +9,7 @@ Usage: Install Git hooks in the current repository Options: + -f, --force Install despite an existing Git hooks setup --hooks-dir Specify hooks directory relative to the Git repository root -h, --help Display this help message " diff --git a/packages/rstack/tests/cli/setup/index.test.ts b/packages/rstack/tests/cli/setup/index.test.ts index 0780fd4a..93bda958 100644 --- a/packages/rstack/tests/cli/setup/index.test.ts +++ b/packages/rstack/tests/cli/setup/index.test.ts @@ -1,5 +1,6 @@ import { spawnSync } from 'node:child_process'; import { + chmodSync, existsSync, mkdirSync, mkdtempSync, @@ -34,6 +35,16 @@ const runSetup = (args: string[], runCwd: string = cwd) => env, }); +const runSetupSuccessfully = (args: string[], runCwd: string = cwd): string => { + const result = runSetup(args, runCwd); + if (result.status !== 0) { + throw new Error( + result.stderr || result.error?.message || `Exited with ${result.status}`, + ); + } + return `${result.stdout}${result.stderr}`; +}; + beforeEach(() => { cwd = mkdtempSync(path.join(import.meta.dirname, 'test-temp-rstack setup ')); env = { @@ -108,6 +119,65 @@ test('installs hooks silently without loading Rstack config', ({ expect(execCli('setup', { cwd, env })).toBe(''); }); +test('guides and forces setup while preserving existing hooks', ({ + expect, +}) => { + initRepository(); + const existingHook = path.join(cwd, '.git', 'hooks', 'pre-commit'); + writeFileSync( + existingHook, + "#!/usr/bin/env sh\nprintf 'ran\\n' > old-hook-ran\n", + ); + chmodSync(existingHook, 0o755); + + const skippedOutput = runSetupSuccessfully([]); + expect(skippedOutput).toContain( + 'Git hooks setup skipped: existing Git hooks were found: pre-commit.', + ); + expect(skippedOutput).toContain( + 'To continue, run rs setup --force. Existing hook files will be preserved but become inactive.', + ); + + const forcedOutput = runSetupSuccessfully(['--force']); + expect(forcedOutput).toContain( + 'The previous Git hooks path ".git/hooks" is now inactive: pre-commit.', + ); + expect(forcedOutput).toContain( + 'The existing files were preserved and will become active again if core.hooksPath is unset.', + ); + expect(git(['config', '--local', '--get', 'core.hooksPath'])).toBe(hooksPath); + + git(['hook', 'run', 'pre-commit']); + expect(existsSync(path.join(cwd, 'old-hook-ran'))).toBe(false); + + git(['config', '--local', '--unset', 'core.hooksPath']); + git(['hook', 'run', 'pre-commit']); + expect(existsSync(path.join(cwd, 'old-hook-ran'))).toBe(true); + + expect(runSetupSuccessfully(['-f'])).toContain( + 'The previous Git hooks path ".git/hooks" is now inactive: pre-commit.', + ); +}); + +test('reports how to restore a replaced hooks path', ({ expect }) => { + initRepository(); + const existingDirectory = path.join(cwd, '.husky', '_'); + mkdirSync(existingDirectory, { recursive: true }); + writeFileSync( + path.join(existingDirectory, 'pre-commit'), + '#!/usr/bin/env sh\n', + ); + git(['config', '--local', 'core.hooksPath', '.husky/_']); + + const output = runSetupSuccessfully(['--force']); + expect(output).toContain( + 'The previous Git hooks path ".husky/_" is now inactive: pre-commit.', + ); + expect(output).toContain( + 'The existing files were preserved. Set core.hooksPath back to this path to use them again.', + ); +}); + test('installs root-relative hooks and reports owner conflicts', ({ execCli, expect, @@ -126,9 +196,7 @@ test('installs root-relative hooks and reports owner conflicts', ({ ); expect(existsSync(path.join(cwd, 'custom hooks', '_', 'runner'))).toBe(true); - const conflict = runSetup(['--hooks-dir', 'custom hooks'], docs); - expect(conflict.status).toBe(0); - expect(`${conflict.stdout}${conflict.stderr}`).toContain( + expect(runSetupSuccessfully(['--hooks-dir', 'custom hooks'], docs)).toContain( 'Git hooks are already managed by Rstack project "frontend"', ); }); diff --git a/packages/rstack/tests/setup/install.test.ts b/packages/rstack/tests/setup/install.test.ts index f91b2b45..4a32fa80 100644 --- a/packages/rstack/tests/setup/install.test.ts +++ b/packages/rstack/tests/setup/install.test.ts @@ -137,8 +137,12 @@ test('reports Git configuration failures without changing hooksPath', () => { }); }); -test('does not replace another Git hooks path', () => { +test('requires force to replace another Git hooks path', () => { withRepository((cwd) => { + const existingDirectory = path.join(cwd, '.husky', '_'); + const existingHook = path.join(existingDirectory, 'pre-commit'); + mkdirSync(existingDirectory, { recursive: true }); + writeFileSync(existingHook, '#!/usr/bin/env sh\n'); runGit(cwd, ['config', '--local', 'core.hooksPath', '.husky/_']); expect(installHooks({ cwd })).toMatchObject({ @@ -149,10 +153,24 @@ test('does not replace another Git hooks path', () => { '.husky/_', ); expect(existsSync(path.join(cwd, hooksPath))).toBe(false); + + expect(installHooks({ cwd, force: true })).toEqual({ + status: 'installed', + hooksPath, + inactiveHooks: { + hooks: ['pre-commit'], + path: '.husky/_', + restore: 'configure', + }, + }); + expect(runGit(cwd, ['config', '--local', '--get', 'core.hooksPath'])).toBe( + hooksPath, + ); + expect(readFileSync(existingHook, 'utf8')).toBe('#!/usr/bin/env sh\n'); }); }); -test('does not bypass existing Git hooks', () => { +test('requires force to bypass existing Git hooks', () => { withRepository((cwd) => { const existingHook = path.join(cwd, '.git', 'hooks', 'pre-commit'); writeFileSync(existingHook, '#!/usr/bin/env sh\n'); @@ -165,6 +183,54 @@ test('does not bypass existing Git hooks', () => { expect( git(cwd, ['config', '--local', '--get', 'core.hooksPath']).status, ).toBe(1); + + expect(installHooks({ cwd, force: true })).toEqual({ + status: 'installed', + hooksPath, + inactiveHooks: { + hooks: ['pre-commit'], + path: '.git/hooks', + restore: 'unset', + }, + }); + expect(runGit(cwd, ['config', '--local', '--get', 'core.hooksPath'])).toBe( + hooksPath, + ); expect(readFileSync(existingHook, 'utf8')).toBe('#!/usr/bin/env sh\n'); }); }); + +test('force does not replace hooks owned by another Rstack project', () => { + withRepository((cwd) => { + const frontend = path.join(cwd, 'frontend'); + const docs = path.join(cwd, 'docs'); + mkdirSync(frontend); + mkdirSync(docs); + + expect(installHooks({ cwd: frontend }).status).toBe('installed'); + expect(installHooks({ cwd: docs, force: true })).toMatchObject({ + status: 'skipped', + reason: 'owned-by-another-project', + }); + expect(readFileSync(path.join(cwd, hooksPath, '.owner'), 'utf8')).toBe( + 'frontend\n', + ); + }); +}); + +test('force does not replace an invalid Rstack hooks directory', () => { + withRepository((cwd) => { + const directory = path.join(cwd, hooksPath); + mkdirSync(directory, { recursive: true }); + writeFileSync(path.join(directory, '.owner'), 'invalid'); + + expect(installHooks({ cwd, force: true })).toMatchObject({ + status: 'skipped', + reason: 'hooks-directory-conflict', + }); + expect( + git(cwd, ['config', '--local', '--get', 'core.hooksPath']).status, + ).toBe(1); + expect(existsSync(path.join(directory, 'runner'))).toBe(false); + }); +});