Skip to content

Commit

Permalink
fix(core): if git hash-object reports fewer hashes than requested - l…
Browse files Browse the repository at this point in the history
…og one of the missing files (#10134)

When executing parallel targets using e.g. run-commands missing entries in .gitignore
may lead to
temp files being passed to "git hash-object"
which when git executes have been deleted
resulting in
mismatch between number of files requested and hashes returned.
git reports the error and this PR
will make sure that report is passed
in the error thrown.
This will make problem resolution a lot
faster.

ISSUES CLOSED: #9946
  • Loading branch information
jonhamm committed May 6, 2022
1 parent 0cc7042 commit 0ec1a62
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 16 deletions.
29 changes: 26 additions & 3 deletions packages/nx/src/hasher/git-hasher.spec.ts
@@ -1,7 +1,7 @@
import { dirSync } from 'tmp';
import { mkdirSync, removeSync } from 'fs-extra';
import { execSync } from 'child_process';
import { getFileHashes } from './git-hasher';
import { mkdirSync, removeSync } from 'fs-extra';
import { dirSync } from 'tmp';
import { getFileHashes, getGitHashForBatch } from './git-hasher';

describe('git-hasher', () => {
let dir: string;
Expand Down Expand Up @@ -173,4 +173,27 @@ describe('git-hasher', () => {
function run(command: string) {
return execSync(command, { cwd: dir, stdio: ['pipe', 'pipe', 'pipe'] });
}

it('should hash two simple files', async () => {
const files = ['a.txt', 'b.txt'];
run(`echo AAA > a.txt`);
run(`echo BBB > b.txt`);
const hashes = await getGitHashForBatch(files, dir);
expect([...hashes.keys()]).toEqual(files);
});

it('should fail when file deleted', async () => {
const files = ['a.txt', 'b.txt'];
run(`echo AAA > a.txt`);
try {
const hashes = await getGitHashForBatch(files, dir);
expect(false).toBeTruthy();
} catch (err: any) {
expect(err instanceof Error).toBeTruthy();
const error = err as Error;
expect(error.message).toMatch(
/Passed 2 file paths to Git to hash, but received 1 hashes\.\n *fatal:.*b\.txt.*No such file or directory\n/
);
}
});
});
40 changes: 27 additions & 13 deletions packages/nx/src/hasher/git-hasher.ts
Expand Up @@ -40,17 +40,17 @@ export async function getGitHashForFiles(
return { hashes: res, deleted };
}

async function getGitHashForBatch(filesToHash: string[], path) {
export async function getGitHashForBatch(filesToHash: string[], path) {
const res: Map<string, string> = new Map<string, string>();
const hashStdout = await spawnProcess(
const { stdout: hashStdout, stderr: hashStderr } = await spawnProcess(
'git',
['hash-object', ...filesToHash],
path
);
const hashes: string[] = hashStdout.split('\n').filter((s) => !!s);
if (hashes.length !== filesToHash.length) {
throw new Error(
`Passed ${filesToHash.length} file paths to Git to hash, but received ${hashes.length} hashes.`
`Passed ${filesToHash.length} file paths to Git to hash, but received ${hashes.length} hashes.\n${hashStderr}`
);
}
for (let i = 0; i < hashes.length; i++) {
Expand All @@ -67,7 +67,7 @@ function getActualFilesToHash(
): { filesToHash: string[]; deleted: string[] } {
const filesToHash = [];
const deleted = [];
for (let file of potentialFilesToHash) {
for (const file of potentialFilesToHash) {
if (fileExists(joinPathFragments(path, file))) {
filesToHash.push(file);
} else {
Expand All @@ -77,28 +77,42 @@ function getActualFilesToHash(
return { filesToHash, deleted };
}

async function spawnProcess(command: string, args: string[], cwd: string) {
async function spawnProcess(
command: string,
args: string[],
cwd: string
): Promise<{ code: number; stdout: string; stderr: string }> {
const cp = spawn(command, args, {
windowsHide: true,
shell: false,
cwd,
});
let stdout = '';
for await (const data of cp.stdout) {
let stderr = '';
cp.stdout.on('data', (data) => {
stdout += data;
}
return stdout;
});
cp.stderr.on('data', (data) => {
stderr += data;
});
return new Promise((resolve) => {
cp.on('close', (code) => {
resolve({ code, stdout, stderr });
});
});
}

async function getStagedFiles(path: string) {
const staged = await spawnProcess(
const { stdout: staged } = await spawnProcess(
'git',
['ls-files', '-s', '-z', '--exclude-standard', '.'],
path
);
const res = new Map();
for (let line of staged.split('\0')) {
if (!line) continue;
for (const line of staged.split('\0')) {
if (!line) {
continue;
}
const [_, hash, __, ...fileParts] = line.split(/\s/);
const fileName = fileParts.join(' ');
res.set(fileName, hash);
Expand All @@ -107,7 +121,7 @@ async function getStagedFiles(path: string) {
}

async function getUnstagedFiles(path: string) {
const unstaged = await spawnProcess(
const { stdout: unstaged } = await spawnProcess(
'git',
['ls-files', '-m', '-z', '--exclude-standard', '.'],
path
Expand All @@ -117,7 +131,7 @@ async function getUnstagedFiles(path: string) {
}

async function getUntrackedFiles(path: string) {
const untracked = await spawnProcess(
const { stdout: untracked } = await spawnProcess(
'git',
['ls-files', '--other', '-z', '--exclude-standard', '.'],
path
Expand Down

1 comment on commit 0ec1a62

@vercel
Copy link

@vercel vercel bot commented on 0ec1a62 May 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

nx-dev – ./

nx-dev-nrwl.vercel.app
nx-dev-git-master-nrwl.vercel.app
nx-five.vercel.app
nx.dev

Please sign in to comment.