address comments

Author: mbfreder

packages/core/src/shared/sam/build.ts

@@ -29,7 +29,7 @@ import {
 } from './utils'
 import { getConfigFileUri, validateSamBuildConfig } from './config'
 import { runInTerminal } from './processTerminal'
-import { getLogger } from '../logger'
+import { SemVer } from 'semver'
 
 const buildMementoRootKey = 'samcli.build.params'
 export interface BuildParams {
@@ -211,10 +211,13 @@ export async function runBuild(arg?: TreeNode): Promise<SamBuildResult> {
     const projectRoot = params.projectRoot
 
     const defaultFlags: string[] = ['--cached', '--parallel', '--save-params', '--use-container']
+
+    const { path: samCliPath, parsedVersion } = await getSamCliPathAndVersion()
+
     // refactor
     const buildFlags: string[] =
         params.paramsSource === ParamsSource.Specify && params.buildFlags
-            ? resolveBuildFlags(JSON.parse(params.buildFlags))
+            ? await resolveBuildFlags(JSON.parse(params.buildFlags), parsedVersion)
             : await getBuildFlags(params.paramsSource, projectRoot, defaultFlags)
 
     // todo remove
@@ -230,8 +233,6 @@ export async function runBuild(arg?: TreeNode): Promise<SamBuildResult> {
     await updateRecentResponse(buildMementoRootKey, 'global', 'templatePath', templatePath)
 
     try {
-        const { path: samCliPath } = await getSamCliPathAndVersion()
-
         // Create a child process to run the SAM build command
         const buildProcess = new ChildProcess(samCliPath, ['build', ...buildFlags], {
             spawnOptions: await addTelemetryEnvVar({
@@ -282,7 +283,11 @@ function resolveBuildArgConflict(boundArgs: string[]): string[] {
     // }
     return Array.from(boundArgsSet)
 }
-export function resolveBuildFlags(buildFlags: string[]): string[] {
+export async function resolveBuildFlags(buildFlags: string[], samCliVersion: SemVer | null): Promise<string[]> {
+    // const samCliVersion = (await getSamCliPathAndVersion()).parsedVersion
+    if (samCliVersion?.compare('1.133.0') ?? -1 < 0) {
+        return buildFlags
+    }
     if (!buildFlags.includes('--use-container')) {
         buildFlags.push('--no-use-container')
     }

packages/core/src/test/shared/sam/build.test.ts

@@ -27,12 +27,14 @@ import { getProjectRootUri } from '../../../shared/sam/utils'
 import sinon from 'sinon'
 import { createMultiPick, DataQuickPickItem } from '../../../shared/ui/pickerPrompter'
 import * as config from '../../../shared/sam/config'
+import * as utils from '../../../shared/sam/utils'
 import { PrompterTester } from '../wizards/prompterTester'
 import { getWorkspaceFolder, TestFolder } from '../../testUtil'
 import { samconfigCompleteData, validTemplateData } from './samTestUtils'
 import { CloudFormationTemplateRegistry } from '../../../shared/fs/templateRegistry'
 import { getTestWindow } from '../vscode/window'
 import { CancellationError } from '../../../shared/utilities/timeoutUtils'
+import { SemVer } from 'semver'
 
 describe('SAM BuildWizard', async function () {
     const createTester = async (params?: Partial<BuildParams>, arg?: TreeNode | undefined) =>
@@ -232,10 +234,42 @@ describe('SAM build helper functions', () => {
     })
 
     describe('resolveBuildFlags', () => {
-        it('should add --no-use-container if the buildFlags array does not contain --use-container', () => {
+        let sandbox: sinon.SinonSandbox
+        let pathAndVersionStub: sinon.SinonStub<any[], any>
+
+        beforeEach(() => {
+            sandbox = sinon.createSandbox()
+        })
+
+        afterEach(() => {
+            sandbox.restore()
+        })
+
+        it('should add --no-use-container if the buildFlags array does not contain --use-container', async () => {
+            const normalVersion = { path: 'file:///path/to/cli', parsedVersion: new SemVer('1.133.0') }
+            pathAndVersionStub = sandbox.stub().resolves(normalVersion)
+            sandbox.stub(utils, 'getSamCliPathAndVersion').callsFake(pathAndVersionStub)
             const buildFlags: string[] = ['--cached', '--debug', '--parallel']
             const resolvedBuildFlags = ['--cached', '--debug', '--parallel', '--no-use-container']
-            assert.deepStrictEqual(resolvedBuildFlags, resolveBuildFlags(buildFlags))
+            assert.deepEqual(resolvedBuildFlags, await resolveBuildFlags(buildFlags, normalVersion.parsedVersion))
+        })
+
+        it('should return the original buildFlags array if SAM CLI version is less than 1.133', async () => {
+            const lowerVersion = { path: 'file:///path/to/cli', parsedVersion: new SemVer('1.110.0') }
+            const pathAndVersionStub = sandbox.stub().resolves(lowerVersion)
+            sandbox.stub(utils, 'getSamCliPathAndVersion').callsFake(pathAndVersionStub)
+            const buildFlags = ['--cached', '--parallel', '--save-params']
+            const resolvedBuildFlags = await resolveBuildFlags(buildFlags, lowerVersion.parsedVersion)
+            assert.deepEqual(resolvedBuildFlags, buildFlags)
+        })
+
+        it('should not add "--no-use-container" if "--use-container" is already present', async () => {
+            const normalVersion = { path: 'file:///path/to/cli', parsedVersion: new SemVer('1.110.0') }
+            pathAndVersionStub = sandbox.stub().resolves(normalVersion)
+            sandbox.stub(utils, 'getSamCliPathAndVersion').callsFake(pathAndVersionStub)
+            const buildFlags = ['--cached', '--parallel', '--save-params', '--use-container']
+            const resolvedBuildFlags = await resolveBuildFlags(buildFlags, normalVersion.parsedVersion)
+            assert.deepStrictEqual(resolvedBuildFlags, buildFlags)
         })
     })
 })