From 766bfbedc053904ff25b47582e53e4b8a42d7484 Mon Sep 17 00:00:00 2001 From: nlf Date: Wed, 14 Apr 2021 07:53:32 -0700 Subject: [PATCH 1/3] fix: do not use a shell for git commands --- lib/opts.js | 1 + test/opts.js | 3 +++ 2 files changed, 4 insertions(+) diff --git a/lib/opts.js b/lib/opts.js index 6db9e9a..144e0a3 100644 --- a/lib/opts.js +++ b/lib/opts.js @@ -7,5 +7,6 @@ const gitEnv = { module.exports = (opts = {}) => ({ stdioString: true, ...opts, + shell: false, env: opts.env || { ...gitEnv, ...process.env } }) diff --git a/test/opts.js b/test/opts.js index 65ac299..4287aca 100644 --- a/test/opts.js +++ b/test/opts.js @@ -7,6 +7,9 @@ const gitEnv = { t.match(gitOpts().env, gitEnv, 'got the git defaults we want') +t.equal(gitOpts().shell, false, 'shell defaults to false') +t.equal(gitOpts({ shell: '/bin/bash' }).shell, false, 'shell cannot be overridden') + t.test('does not override', t => { const { GIT_ASKPASS, GIT_SSH_COMMAND } = process.env t.teardown(() => { From e69549f4d1516b06afb0804067df82afb2ce041e Mon Sep 17 00:00:00 2001 From: nlf Date: Wed, 14 Apr 2021 08:11:39 -0700 Subject: [PATCH 2/3] fix: remove path escaping since we do not need it when not using a shell --- lib/clone.js | 8 ++++---- lib/utils.js | 13 ------------- lib/which.js | 3 +-- test/clone.js | 5 ++++- 4 files changed, 9 insertions(+), 20 deletions(-) diff --git a/lib/clone.js b/lib/clone.js index 0b88c79..1fea5ac 100644 --- a/lib/clone.js +++ b/lib/clone.js @@ -24,7 +24,7 @@ const { basename, resolve } = require('path') const revs = require('./revs.js') const spawn = require('./spawn.js') -const { isWindows, escapePath } = require('./utils.js') +const { isWindows } = require('./utils.js') const pickManifest = require('npm-pick-manifest') const fs = require('fs') @@ -112,7 +112,7 @@ const branch = (repo, revDoc, target, opts) => { '-b', revDoc.ref, repo, - escapePath(target, opts), + target, '--recurse-submodules' ] if (maybeShallow(repo, opts)) { args.push('--depth=1') } @@ -125,7 +125,7 @@ const plain = (repo, revDoc, target, opts) => { const args = [ 'clone', repo, - escapePath(target, opts), + target, '--recurse-submodules' ] if (maybeShallow(repo, opts)) { args.push('--depth=1') } @@ -151,7 +151,7 @@ const unresolved = (repo, ref, target, opts) => { // can't do this one shallowly, because the ref isn't advertised // but we can avoid checking out the working dir twice, at least const lp = isWindows(opts) ? ['--config', 'core.longpaths=true'] : [] - const cloneArgs = ['clone', '--mirror', '-q', repo, escapePath(target + '/.git', opts)] + const cloneArgs = ['clone', '--mirror', '-q', repo, target + '/.git'] const git = (args) => spawn(args, { ...opts, cwd: target }) return mkdirp(target) .then(() => git(cloneArgs.concat(lp))) diff --git a/lib/utils.js b/lib/utils.js index 82610a7..fcd9578 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,16 +1,3 @@ -const { basename } = require('path') - const isWindows = opts => (opts.fakePlatform || process.platform) === 'win32' -// wrap the target in quotes for Windows when using cmd(.exe) as a shell to -// avoid clone failures for paths with spaces -const escapePath = (gitPath, opts) => { - const isCmd = opts.shell && (basename(opts.shell.toLowerCase(), '.exe') === 'cmd') - if (isWindows(opts) && isCmd && !gitPath.startsWith('"')) { - return `"${gitPath}"` - } - return gitPath -} - -exports.escapePath = escapePath exports.isWindows = isWindows diff --git a/lib/which.js b/lib/which.js index d4e1131..a2f690e 100644 --- a/lib/which.js +++ b/lib/which.js @@ -1,4 +1,3 @@ -const { escapePath } = require('./utils.js') const which = require('which') let gitPath @@ -13,5 +12,5 @@ module.exports = (opts = {}) => { if (!gitPath || opts.git === false) { return Object.assign(new Error('No git binary found in $PATH'), { code: 'ENOGIT' }) } - return escapePath(gitPath, opts) + return gitPath } diff --git a/test/clone.js b/test/clone.js index 1941df7..e8caa52 100644 --- a/test/clone.js +++ b/test/clone.js @@ -253,5 +253,8 @@ if ((process.platform) === 'win32') { ) } else { t.test('cloning to folder with spaces with cmd as the shell not on windows', t => - t.rejects(clone(join(regularRepo, '.git'), 'HEAD', clonedRepoSpaces2, { fakePlatform: 'win32', shell: 'cmd' }))) + clone(join(regularRepo, '.git'), 'HEAD', clonedRepoSpaces2, { fakePlatform: 'win32', shell: 'cmd' }) + .then(() => revs(regularRepo)) + .then((r) => revs(clonedRepoSpaces2).then((r2) => t.same(Object.keys(r.shas), Object.keys(r2.shas)))) + ) } From f48dc34b31ff824f1d37764bc098b6318122662a Mon Sep 17 00:00:00 2001 From: nlf Date: Wed, 14 Apr 2021 08:15:31 -0700 Subject: [PATCH 3/3] chore: run lint as posttest --- .github/workflows/ci.yml | 4 ---- package.json | 3 ++- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2845337..4257e03 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,10 +64,6 @@ jobs: git config --global user.name "Your Name" npm test -- --no-coverage --timeout 60 - - name: Run linting - run: | - npm run lint - ################################################################################ # Run coverage check # diff --git a/package.json b/package.json index c949936..cdc9d0b 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,8 @@ "prepublishOnly": "git push origin --follow-tags", "preversion": "npm test", "snap": "tap", - "test": "tap" + "test": "tap", + "posttest": "npm run lint" }, "tap": { "check-coverage": true,