From c9e9d1c39fb5b80d3470c1e99afd0de86b681637 Mon Sep 17 00:00:00 2001 From: Cory Miller <13227161+cory-miller@users.noreply.github.com> Date: Tue, 13 Dec 2022 20:52:34 +0000 Subject: [PATCH] Add tests for branch list ambiguous refname --- __test__/git-command-manager.test.ts | 80 ++++++++ dist/index.js | 267 ++++++++++++--------------- src/git-command-manager.ts | 27 +-- 3 files changed, 214 insertions(+), 160 deletions(-) create mode 100644 __test__/git-command-manager.test.ts diff --git a/__test__/git-command-manager.test.ts b/__test__/git-command-manager.test.ts new file mode 100644 index 0000000..6944ff7 --- /dev/null +++ b/__test__/git-command-manager.test.ts @@ -0,0 +1,80 @@ +import * as exec from '@actions/exec' +import * as fshelper from '../lib/fs-helper' +import * as commandManager from '../lib/git-command-manager' + +let git: commandManager.IGitCommandManager +let mockExec = jest.fn() + +describe('git-auth-helper tests', () => { + beforeAll(async () => {}) + + beforeEach(async () => { + jest.spyOn(fshelper, 'fileExistsSync').mockImplementation(jest.fn()) + jest.spyOn(fshelper, 'directoryExistsSync').mockImplementation(jest.fn()) + }) + + afterEach(() => { + jest.restoreAllMocks() + }) + + afterAll(() => {}) + + it('branch list matches', async () => { + mockExec.mockImplementation((path, args, options) => { + console.log(args, options.listeners.stdout) + + if (args.includes('version')) { + options.listeners.stdout(Buffer.from('2.18')) + return 0 + } + + if (args.includes('rev-parse')) { + options.listeners.stdline(Buffer.from('refs/heads/foo')) + options.listeners.stdline(Buffer.from('refs/heads/bar')) + return 0 + } + + return 1 + }) + jest.spyOn(exec, 'exec').mockImplementation(mockExec) + const workingDirectory = 'test' + const lfs = false + git = await commandManager.createCommandManager(workingDirectory, lfs) + + let branches = await git.branchList(false) + + expect(branches).toHaveLength(2) + expect(branches.sort()).toEqual(['foo', 'bar'].sort()) + }) + + it('ambiguous ref name output is captured', async () => { + mockExec.mockImplementation((path, args, options) => { + console.log(args, options.listeners.stdout) + + if (args.includes('version')) { + options.listeners.stdout(Buffer.from('2.18')) + return 0 + } + + if (args.includes('rev-parse')) { + options.listeners.stdline(Buffer.from('refs/heads/foo')) + // If refs/tags/v1 and refs/heads/tags/v1 existed on this repository + options.listeners.errline( + Buffer.from("error: refname 'tags/v1' is ambiguous") + ) + return 0 + } + + return 1 + }) + jest.spyOn(exec, 'exec').mockImplementation(mockExec) + const workingDirectory = 'test' + const lfs = false + git = await commandManager.createCommandManager(workingDirectory, lfs) + + let branches = await git.branchList(false) + + expect(branches).toHaveLength(1) + expect(branches.sort()).toEqual(['foo'].sort()) + }) +}) diff --git a/dist/index.js b/dist/index.js index eb035af..a34daa0 100644 --- a/dist/index.js +++ b/dist/index.js @@ -98,25 +98,6 @@ module.exports = Octokit; "use strict"; -var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) { - if (k2 === undefined) k2 = k; - Object.defineProperty(o, k2, { enumerable: true, get: function() { return m[k]; } }); -}) : (function(o, m, k, k2) { - if (k2 === undefined) k2 = k; - o[k2] = m[k]; -})); -var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (function(o, v) { - Object.defineProperty(o, "default", { enumerable: true, value: v }); -}) : function(o, v) { - o["default"] = v; -}); -var __importStar = (this && this.__importStar) || function (mod) { - if (mod && mod.__esModule) return mod; - var result = {}; - if (mod != null) for (var k in mod) if (k !== "default" && Object.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k); - __setModuleDefault(result, mod); - return result; -}; var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) { function adopt(value) { return value instanceof P ? value : new P(function (resolve) { resolve(value); }); } return new (P || (P = Promise))(function (resolve, reject) { @@ -127,14 +108,11 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge }); }; Object.defineProperty(exports, "__esModule", { value: true }); -exports.findInPath = exports.which = exports.mkdirP = exports.rmRF = exports.mv = exports.cp = void 0; -const assert_1 = __webpack_require__(357); -const childProcess = __importStar(__webpack_require__(129)); -const path = __importStar(__webpack_require__(622)); +const childProcess = __webpack_require__(129); +const path = __webpack_require__(622); const util_1 = __webpack_require__(669); -const ioUtil = __importStar(__webpack_require__(672)); +const ioUtil = __webpack_require__(672); const exec = util_1.promisify(childProcess.exec); -const execFile = util_1.promisify(childProcess.execFile); /** * Copies a file or folder. * Based off of shelljs - https://github.com/shelljs/shelljs/blob/9237f66c52e5daa40458f94f9565e18e8132f5a6/src/cp.js @@ -145,14 +123,14 @@ const execFile = util_1.promisify(childProcess.execFile); */ function cp(source, dest, options = {}) { return __awaiter(this, void 0, void 0, function* () { - const { force, recursive, copySourceDirectory } = readCopyOptions(options); + const { force, recursive } = readCopyOptions(options); const destStat = (yield ioUtil.exists(dest)) ? yield ioUtil.stat(dest) : null; // Dest is an existing file, but not forcing if (destStat && destStat.isFile() && !force) { return; } // If dest is an existing directory, should copy inside. - const newDest = destStat && destStat.isDirectory() && copySourceDirectory + const newDest = destStat && destStat.isDirectory() ? path.join(dest, path.basename(source)) : dest; if (!(yield ioUtil.exists(source))) { @@ -217,22 +195,12 @@ function rmRF(inputPath) { if (ioUtil.IS_WINDOWS) { // Node doesn't provide a delete operation, only an unlink function. This means that if the file is being used by another // program (e.g. antivirus), it won't be deleted. To address this, we shell out the work to rd/del. - // Check for invalid characters - // https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file - if (/[*"<>|]/.test(inputPath)) { - throw new Error('File path must not contain `*`, `"`, `<`, `>` or `|` on Windows'); - } try { - const cmdPath = ioUtil.getCmdPath(); if (yield ioUtil.isDirectory(inputPath, true)) { - yield exec(`${cmdPath} /s /c "rd /s /q "%inputPath%""`, { - env: { inputPath } - }); + yield exec(`rd /s /q "${inputPath}"`); } else { - yield exec(`${cmdPath} /s /c "del /f /a "%inputPath%""`, { - env: { inputPath } - }); + yield exec(`del /f /a "${inputPath}"`); } } catch (err) { @@ -265,7 +233,7 @@ function rmRF(inputPath) { return; } if (isDir) { - yield execFile(`rm`, [`-rf`, `${inputPath}`]); + yield exec(`rm -rf "${inputPath}"`); } else { yield ioUtil.unlink(inputPath); @@ -283,8 +251,7 @@ exports.rmRF = rmRF; */ function mkdirP(fsPath) { return __awaiter(this, void 0, void 0, function* () { - assert_1.ok(fsPath, 'a path argument must be provided'); - yield ioUtil.mkdir(fsPath, { recursive: true }); + yield ioUtil.mkdirP(fsPath); }); } exports.mkdirP = mkdirP; @@ -312,80 +279,62 @@ function which(tool, check) { throw new Error(`Unable to locate executable file: ${tool}. Please verify either the file path exists or the file can be found within a directory specified by the PATH environment variable. Also check the file mode to verify the file is executable.`); } } - return result; } - const matches = yield findInPath(tool); - if (matches && matches.length > 0) { - return matches[0]; + try { + // build the list of extensions to try + const extensions = []; + if (ioUtil.IS_WINDOWS && process.env.PATHEXT) { + for (const extension of process.env.PATHEXT.split(path.delimiter)) { + if (extension) { + extensions.push(extension); + } + } + } + // if it's rooted, return it if exists. otherwise return empty. + if (ioUtil.isRooted(tool)) { + const filePath = yield ioUtil.tryGetExecutablePath(tool, extensions); + if (filePath) { + return filePath; + } + return ''; + } + // if any path separators, return empty + if (tool.includes('/') || (ioUtil.IS_WINDOWS && tool.includes('\\'))) { + return ''; + } + // build the list of directories + // + // Note, technically "where" checks the current directory on Windows. From a toolkit perspective, + // it feels like we should not do this. Checking the current directory seems like more of a use + // case of a shell, and the which() function exposed by the toolkit should strive for consistency + // across platforms. + const directories = []; + if (process.env.PATH) { + for (const p of process.env.PATH.split(path.delimiter)) { + if (p) { + directories.push(p); + } + } + } + // return the first match + for (const directory of directories) { + const filePath = yield ioUtil.tryGetExecutablePath(directory + path.sep + tool, extensions); + if (filePath) { + return filePath; + } + } + return ''; + } + catch (err) { + throw new Error(`which failed with message ${err.message}`); } - return ''; }); } exports.which = which; -/** - * Returns a list of all occurrences of the given tool on the system path. - * - * @returns Promise the paths of the tool - */ -function findInPath(tool) { - return __awaiter(this, void 0, void 0, function* () { - if (!tool) { - throw new Error("parameter 'tool' is required"); - } - // build the list of extensions to try - const extensions = []; - if (ioUtil.IS_WINDOWS && process.env['PATHEXT']) { - for (const extension of process.env['PATHEXT'].split(path.delimiter)) { - if (extension) { - extensions.push(extension); - } - } - } - // if it's rooted, return it if exists. otherwise return empty. - if (ioUtil.isRooted(tool)) { - const filePath = yield ioUtil.tryGetExecutablePath(tool, extensions); - if (filePath) { - return [filePath]; - } - return []; - } - // if any path separators, return empty - if (tool.includes(path.sep)) { - return []; - } - // build the list of directories - // - // Note, technically "where" checks the current directory on Windows. From a toolkit perspective, - // it feels like we should not do this. Checking the current directory seems like more of a use - // case of a shell, and the which() function exposed by the toolkit should strive for consistency - // across platforms. - const directories = []; - if (process.env.PATH) { - for (const p of process.env.PATH.split(path.delimiter)) { - if (p) { - directories.push(p); - } - } - } - // find all matches - const matches = []; - for (const directory of directories) { - const filePath = yield ioUtil.tryGetExecutablePath(path.join(directory, tool), extensions); - if (filePath) { - matches.push(filePath); - } - } - return matches; - }); -} -exports.findInPath = findInPath; function readCopyOptions(options) { const force = options.force == null ? true : options.force; const recursive = Boolean(options.recursive); - const copySourceDirectory = options.copySourceDirectory == null - ? true - : Boolean(options.copySourceDirectory); - return { force, recursive, copySourceDirectory }; + return { force, recursive }; } function cpDirRecursive(sourceDir, destDir, currentDepth, force) { return __awaiter(this, void 0, void 0, function* () { @@ -7441,8 +7390,10 @@ class GitCommandManager { const result = []; // Note, this implementation uses "rev-parse --symbolic-full-name" because the output from // "branch --list" is more difficult when in a detached HEAD state. - // Note, this implementation uses "rev-parse --symbolic-full-name" because there is a bug - // in Git 2.18 that causes "rev-parse --symbolic" to output symbolic full names. + // TODO(https://github.com/actions/checkout/issues/786): this implementation uses + // "rev-parse --symbolic-full-name" because there is a bug + // in Git 2.18 that causes "rev-parse --symbolic" to output symbolic full names. When + // 2.18 is no longer supported, we can switch back to --symbolic. const args = ['rev-parse', '--symbolic-full-name']; if (remote) { args.push('--remotes=origin'); @@ -7468,7 +7419,7 @@ class GitCommandManager { stdline.push(data.toString()); } }; - core.info(`${this.gitPath} ${args.join(' ')}`); + // Suppress the output in order to avoid flooding annotations with innocuous errors. yield this.execGit(args, false, true, listeners); core.debug(`stderr callback is: ${stderr}`); core.debug(`errline callback is: ${errline}`); @@ -7476,17 +7427,17 @@ class GitCommandManager { core.debug(`stdline callback is: ${stdline}`); for (let branch of stdline) { branch = branch.trim(); - if (branch) { - if (branch.startsWith('refs/heads/')) { - branch = branch.substr('refs/heads/'.length); - } - else if (branch.startsWith('refs/remotes/')) { - branch = branch.substr('refs/remotes/'.length); - } - result.push(branch); + if (!branch) { + continue; } + if (branch.startsWith('refs/heads/')) { + branch = branch.substring('refs/heads/'.length); + } + else if (branch.startsWith('refs/remotes/')) { + branch = branch.substring('refs/remotes/'.length); + } + result.push(branch); } - core.info(result.join('\n')); return result; }); } @@ -7763,6 +7714,8 @@ class GitCommandManager { }; result.exitCode = yield exec.exec(`"${this.gitPath}"`, args, options); result.stdout = stdout.join(''); + core.debug(result.exitCode.toString()); + core.debug(result.stdout); return result; }); } @@ -16474,25 +16427,6 @@ module.exports = require("util"); "use strict"; -var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) { - if (k2 === undefined) k2 = k; - Object.defineProperty(o, k2, { enumerable: true, get: function() { return m[k]; } }); -}) : (function(o, m, k, k2) { - if (k2 === undefined) k2 = k; - o[k2] = m[k]; -})); -var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (function(o, v) { - Object.defineProperty(o, "default", { enumerable: true, value: v }); -}) : function(o, v) { - o["default"] = v; -}); -var __importStar = (this && this.__importStar) || function (mod) { - if (mod && mod.__esModule) return mod; - var result = {}; - if (mod != null) for (var k in mod) if (k !== "default" && Object.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k); - __setModuleDefault(result, mod); - return result; -}; var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) { function adopt(value) { return value instanceof P ? value : new P(function (resolve) { resolve(value); }); } return new (P || (P = Promise))(function (resolve, reject) { @@ -16504,9 +16438,9 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge }; var _a; Object.defineProperty(exports, "__esModule", { value: true }); -exports.getCmdPath = exports.tryGetExecutablePath = exports.isRooted = exports.isDirectory = exports.exists = exports.IS_WINDOWS = exports.unlink = exports.symlink = exports.stat = exports.rmdir = exports.rename = exports.readlink = exports.readdir = exports.mkdir = exports.lstat = exports.copyFile = exports.chmod = void 0; -const fs = __importStar(__webpack_require__(747)); -const path = __importStar(__webpack_require__(622)); +const assert_1 = __webpack_require__(357); +const fs = __webpack_require__(747); +const path = __webpack_require__(622); _a = fs.promises, exports.chmod = _a.chmod, exports.copyFile = _a.copyFile, exports.lstat = _a.lstat, exports.mkdir = _a.mkdir, exports.readdir = _a.readdir, exports.readlink = _a.readlink, exports.rename = _a.rename, exports.rmdir = _a.rmdir, exports.stat = _a.stat, exports.symlink = _a.symlink, exports.unlink = _a.unlink; exports.IS_WINDOWS = process.platform === 'win32'; function exists(fsPath) { @@ -16547,6 +16481,49 @@ function isRooted(p) { return p.startsWith('/'); } exports.isRooted = isRooted; +/** + * Recursively create a directory at `fsPath`. + * + * This implementation is optimistic, meaning it attempts to create the full + * path first, and backs up the path stack from there. + * + * @param fsPath The path to create + * @param maxDepth The maximum recursion depth + * @param depth The current recursion depth + */ +function mkdirP(fsPath, maxDepth = 1000, depth = 1) { + return __awaiter(this, void 0, void 0, function* () { + assert_1.ok(fsPath, 'a path argument must be provided'); + fsPath = path.resolve(fsPath); + if (depth >= maxDepth) + return exports.mkdir(fsPath); + try { + yield exports.mkdir(fsPath); + return; + } + catch (err) { + switch (err.code) { + case 'ENOENT': { + yield mkdirP(path.dirname(fsPath), maxDepth, depth + 1); + yield exports.mkdir(fsPath); + return; + } + default: { + let stats; + try { + stats = yield exports.stat(fsPath); + } + catch (err2) { + throw err; + } + if (!stats.isDirectory()) + throw err; + } + } + } + }); +} +exports.mkdirP = mkdirP; /** * Best effort attempt to determine whether a file exists and is executable. * @param filePath file path to check @@ -16643,12 +16620,6 @@ function isUnixExecutable(stats) { ((stats.mode & 8) > 0 && stats.gid === process.getgid()) || ((stats.mode & 64) > 0 && stats.uid === process.getuid())); } -// Get the path of cmd.exe in windows -function getCmdPath() { - var _a; - return (_a = process.env['COMSPEC']) !== null && _a !== void 0 ? _a : `cmd.exe`; -} -exports.getCmdPath = getCmdPath; //# sourceMappingURL=io-util.js.map /***/ }), diff --git a/src/git-command-manager.ts b/src/git-command-manager.ts index 4bce461..5d79dba 100644 --- a/src/git-command-manager.ts +++ b/src/git-command-manager.ts @@ -128,8 +128,7 @@ class GitCommandManager { } } - core.info(`${this.gitPath} ${args.join(' ')}`) - + // Suppress the output in order to avoid flooding annotations with innocuous errors. await this.execGit(args, false, true, listeners) core.debug(`stderr callback is: ${stderr}`) @@ -139,18 +138,18 @@ class GitCommandManager { for (let branch of stdline) { branch = branch.trim() - if (branch) { - if (branch.startsWith('refs/heads/')) { - branch = branch.substr('refs/heads/'.length) - } else if (branch.startsWith('refs/remotes/')) { - branch = branch.substr('refs/remotes/'.length) - } - - result.push(branch) + if (!branch) { + continue } - } - core.info(result.join('\n')) + if (branch.startsWith('refs/heads/')) { + branch = branch.substring('refs/heads/'.length) + } else if (branch.startsWith('refs/remotes/')) { + branch = branch.substring('refs/remotes/'.length) + } + + result.push(branch) + } return result } @@ -462,6 +461,10 @@ class GitCommandManager { result.exitCode = await exec.exec(`"${this.gitPath}"`, args, options) result.stdout = stdout.join('') + + core.debug(result.exitCode.toString()) + core.debug(result.stdout) + return result }