Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/rstack/src/cli/commandHelp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ const HELP_DEFINITIONS = {
{
title: 'Options',
items: [
['-f, --force', 'Install despite an existing Git hooks setup'],
[
'--hooks-dir <path>',
'Specify hooks directory relative to the Git repository root',
Expand Down
37 changes: 35 additions & 2 deletions packages/rstack/src/setup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const runSetupCLI = async (args: string[]): Promise<void> => {
const { values } = parseArgs({
args,
options: {
force: { type: 'boolean', short: 'f' },
help: { type: 'boolean', short: 'h' },
'hooks-dir': { type: 'string', multiple: true },
},
Expand All @@ -28,15 +29,47 @@ export const runSetupCLI = async (args: string[]): Promise<void> => {
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;
}

Expand Down
52 changes: 40 additions & 12 deletions packages/rstack/src/setup/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,7 +40,7 @@ type SkippedInstallResult = {
};

type InstallResult =
| { status: 'installed'; hooksPath: string }
| { status: 'installed'; hooksPath: string; inactiveHooks?: InactiveHooks }
| { status: 'unchanged'; hooksPath: string }
| SkippedInstallResult
| FailedInstallResult;
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -282,27 +290,43 @@ 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) {
Comment thread
chenjiahan marked this conversation as resolved.
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);
}
}

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',
};
}
}

Expand Down Expand Up @@ -360,5 +384,9 @@ export const installHooks = ({
);
}

return { status: 'installed', hooksPath };
return {
status: 'installed',
hooksPath,
...(inactiveHooks ? { inactiveHooks } : {}),
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Usage:
Install Git hooks in the current repository

Options:
-f, --force Install despite an existing Git hooks setup
--hooks-dir <path> Specify hooks directory relative to the Git repository root
-h, --help Display this help message
"
Expand Down
74 changes: 71 additions & 3 deletions packages/rstack/tests/cli/setup/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { spawnSync } from 'node:child_process';
import {
chmodSync,
existsSync,
mkdirSync,
mkdtempSync,
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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,
Expand All @@ -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"',
);
});
Expand Down
70 changes: 68 additions & 2 deletions packages/rstack/tests/setup/install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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');
Expand All @@ -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);
});
});