From 2f5ee7579f00660282dd161a0b79690f4a9c865d Mon Sep 17 00:00:00 2001 From: Damilola Olufemi Date: Thu, 3 Oct 2024 11:29:59 +0100 Subject: [PATCH] feat: handle gradle wrapper files for windows (#292) * feat: handle gradle wrapper files for windows * feat: updated test wrapper tests * feat: add test for generateWrapperProcessArgs * fix: lint files --- lib/index.ts | 44 +++++++++++++++---- lib/sub-process.ts | 6 +-- .../with-lock-file/dep-graph.json | 2 +- test/system/darwin.test.ts | 4 +- test/system/plugin.test.ts | 17 ++++++- test/system/windows.test.ts | 41 ++++++++++------- 6 files changed, 82 insertions(+), 32 deletions(-) diff --git a/lib/index.ts b/lib/index.ts index fb872bd..60b2bc7 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -396,11 +396,13 @@ function getVersionBuildInfo( export async function getGradleVersion( root: string, command: string, + args?: string[], ): Promise { debugLog('`gradle -v` command run: ' + command); let gradleVersionOutput = '[COULD NOT RUN gradle -v]'; + const completeArgs = args ? args.concat(['-v']) : ['-v']; try { - gradleVersionOutput = await subProcess.execute(command, ['-v'], { + gradleVersionOutput = await subProcess.execute(command, completeArgs, { cwd: root, }); } catch (_) { @@ -409,6 +411,22 @@ export async function getGradleVersion( return gradleVersionOutput; } +export function generateWrapperProcessArgs( + commandPath: string, + args: string[], +): { command: string; args: string[] } { + let parseArgs: string[] = []; + let command = commandPath; + const isWinLocal = /^win/.test(os.platform()); + if (isWinLocal && command !== 'gradle') { + command = 'cmd.exe'; + parseArgs.push('/c'); + parseArgs.push(commandPath); + } + parseArgs = parseArgs.concat(args); + return { command, args: parseArgs }; +} + async function getAllDepsWithPlugin( root: string, targetFile: string, @@ -427,12 +445,15 @@ async function getAllDepsWithPlugin( gradleVersion, ); - const fullCommandText = 'gradle command: ' + command + ' ' + args.join(' '); - debugLog('Executing ' + fullCommandText); + const { command: wrapperedCommand, args: wrapperArgs } = + generateWrapperProcessArgs(command, args); + const fullCommandText = + 'gradle command: ' + wrapperedCommand + ' ' + wrapperArgs.join(' '); + debugLog('Executing ' + fullCommandText); const stdoutText = await subProcess.execute( - command, - args, + wrapperedCommand, + wrapperArgs, { cwd: root }, printIfEcho, ); @@ -467,7 +488,13 @@ async function getAllDeps( snykHttpClient: SnykHttpClient, ): Promise { const command = getCommand(root, targetFile); - const gradleVersion = await getGradleVersion(root, command); + const { command: wrapperedCommand, args: wrapperArgs } = + generateWrapperProcessArgs(command, []); + const gradleVersion = await getGradleVersion( + root, + wrapperedCommand, + wrapperArgs, + ); if (gradleVersion.match(/Gradle 1/)) { throw new Error('Gradle 1.x is not supported'); } @@ -636,7 +663,6 @@ function toCamelCase(input: string) { function getCommand(root: string, targetFile: string) { const isWinLocal = /^win/.test(os.platform()); // local check, can be stubbed in tests - const quotLocal = isWinLocal ? '"' : "'"; const wrapperScript = isWinLocal ? 'gradlew.bat' : './gradlew'; // try to find a sibling wrapper script first let pathToWrapper = path.resolve( @@ -645,12 +671,12 @@ function getCommand(root: string, targetFile: string) { wrapperScript, ); if (fs.existsSync(pathToWrapper)) { - return quotLocal + pathToWrapper + quotLocal; + return pathToWrapper; } // now try to find a wrapper in the root pathToWrapper = path.resolve(root, wrapperScript); if (fs.existsSync(pathToWrapper)) { - return quotLocal + pathToWrapper + quotLocal; + return pathToWrapper; } return 'gradle'; } diff --git a/lib/sub-process.ts b/lib/sub-process.ts index 82f2004..fcd9f42 100644 --- a/lib/sub-process.ts +++ b/lib/sub-process.ts @@ -1,6 +1,6 @@ import * as childProcess from 'child_process'; import debugModule = require('debug'); -import { quoteAll } from 'shescape'; +import { escapeAll } from 'shescape'; const debugLogging = debugModule('snyk-gradle-plugin'); @@ -12,7 +12,7 @@ export function execute( perLineCallback?: (s: string) => Promise, ): Promise { const spawnOptions: childProcess.SpawnOptions = { - shell: true, + shell: false, env: { ...process.env }, }; if (options?.cwd) { @@ -22,7 +22,7 @@ export function execute( spawnOptions.env = { ...process.env, ...options.env }; } - args = quoteAll(args, spawnOptions); + args = escapeAll(args, spawnOptions); // Before spawning an external process, we look if we need to restore the system proxy configuration, // which overides the cli internal proxy configuration. diff --git a/test/fixtures-with-wrappers/with-lock-file/dep-graph.json b/test/fixtures-with-wrappers/with-lock-file/dep-graph.json index e4e6007..73bc767 100644 --- a/test/fixtures-with-wrappers/with-lock-file/dep-graph.json +++ b/test/fixtures-with-wrappers/with-lock-file/dep-graph.json @@ -144,4 +144,4 @@ } ] } -} +} \ No newline at end of file diff --git a/test/system/darwin.test.ts b/test/system/darwin.test.ts index 692b251..e9cfc55 100644 --- a/test/system/darwin.test.ts +++ b/test/system/darwin.test.ts @@ -34,7 +34,7 @@ test('darwin without wrapper invokes gradle directly', async () => { test('darwin with wrapper invokes wrapper script', async () => { await expect(inspect(rootWithWrapper, 'build.gradle')).rejects.toThrow(); expect(subProcessExecSpy.mock.calls[0][0]).toBe( - "'" + path.join(rootWithWrapper, 'gradlew') + "'", + path.join(rootWithWrapper, 'gradlew'), ); }); @@ -43,6 +43,6 @@ test('darwin with wrapper in root invokes wrapper script', async () => { inspect(subWithWrapper, path.join('app', 'build.gradle')), ).rejects.toThrow(); expect(subProcessExecSpy.mock.calls[0][0]).toBe( - "'" + path.join(subWithWrapper, 'gradlew') + "'", + path.join(subWithWrapper, 'gradlew'), ); }); diff --git a/test/system/plugin.test.ts b/test/system/plugin.test.ts index 15044d0..7e43346 100644 --- a/test/system/plugin.test.ts +++ b/test/system/plugin.test.ts @@ -2,7 +2,8 @@ import * as fs from 'fs'; import * as path from 'path'; import * as depGraphLib from '@snyk/dep-graph'; import { fixtureDir } from '../common'; -import { inspect } from '../../lib'; +import { generateWrapperProcessArgs, inspect } from '../../lib'; +import * as os from 'os'; const rootNoWrapper = fixtureDir('no wrapper'); const withInitScript = fixtureDir('with-init-script'); @@ -212,3 +213,17 @@ test('repeated transitive lines not pruned if verbose graph', async () => { }); expect(result.dependencyGraph?.equals(expected)).toBe(true); }); + +test('generateWrapperProcessArgs should return gradle is wrapper is not used', () => { + const result = generateWrapperProcessArgs('gradle', ['-v']); + expect(result).toEqual({ command: 'gradle', args: ['-v'] }); +}); +test('generateWrapperProcessArgs should return wrapped command is os is windows ', () => { + const platformMock = jest.spyOn(os, 'platform'); + platformMock.mockReturnValue('win32'); + const result = generateWrapperProcessArgs('foo/bar/gradlew.bat', ['-v']); + expect(result).toEqual({ + command: 'cmd.exe', + args: ['/c', 'foo/bar/gradlew.bat', '-v'], + }); +}); diff --git a/test/system/windows.test.ts b/test/system/windows.test.ts index f1c764d..b07a4be 100644 --- a/test/system/windows.test.ts +++ b/test/system/windows.test.ts @@ -9,6 +9,7 @@ const rootWithWrapper = fixtureDir('with-wrapper'); const subWithWrapper = fixtureDir('with-wrapper-in-root'); let subProcessExecSpy; let platformMock; +const isWinLocal = /^win/.test(os.platform()); beforeAll(() => { platformMock = jest.spyOn(os, 'platform'); @@ -24,23 +25,31 @@ afterAll(() => { afterEach(() => { jest.clearAllMocks(); }); +if (isWinLocal) { + test('windows with wrapper in root invokes wrapper bat', async () => { + await expect( + inspect(subWithWrapper, path.join('app', 'build.gradle')), + ).rejects.toThrow(); + expect(subProcessExecSpy.mock.calls[0]).toEqual([ + 'cmd.exe', + ['/c', `${subWithWrapper}\\gradlew.bat`, '-v'], + { + cwd: `${subWithWrapper}`, + }, + ]); + }); -test('windows with wrapper in root invokes wrapper bat', async () => { - await expect( - inspect(subWithWrapper, path.join('app', 'build.gradle')), - ).rejects.toThrow(); - expect(subProcessExecSpy.mock.calls[0][0]).toBe( - '"' + path.join(subWithWrapper, 'gradlew.bat') + '"', - ); -}); - -test('windows with wrapper invokes wrapper bat', async () => { - await expect(inspect(rootWithWrapper, 'build.gradle')).rejects.toThrow(); - expect(subProcessExecSpy.mock.calls[0][0]).toBe( - '"' + path.join(rootWithWrapper, 'gradlew.bat') + '"', - ); -}); - + test('windows with wrapper invokes wrapper bat', async () => { + await expect(inspect(rootWithWrapper, 'build.gradle')).rejects.toThrow(); + expect(subProcessExecSpy.mock.calls[0]).toEqual([ + 'cmd.exe', + ['/c', `${rootWithWrapper}\\gradlew.bat`, '-v'], + { + cwd: `${rootWithWrapper}`, + }, + ]); + }); +} test('windows without wrapper invokes gradle directly', async () => { await expect(inspect(rootNoWrapper, 'build.gradle')).rejects.toThrow(); expect(subProcessExecSpy.mock.calls[0][0]).toBe('gradle');