Skip to content

Commit 8be5858

Browse files
authored
fix(appbuilder): pass in the negative version of --use-container when using build quickpick (#6603)
## Problem When users use appbuilder to build their lambda functions, they choose between using their samconfig file or manually selecting the build parameters/flags. The problem is that when the user selects build flags and intentionally doesn't select the ```--use-container``` flag, the command will still be run with --use-container if the samconfig file has ```use_container``` is set to true. ## Solution Whenever the user manually selects the build flags and doesn't select ```--use-container```, we add the negative version of ```--use-container```, which is ```--no-use-container```. This serves as an override if the samconfig file has ```--use-container``` set to true. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent d1758c2 commit 8be5858

File tree

3 files changed

+67
-3
lines changed

3 files changed

+67
-3
lines changed

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
} from './utils'
3030
import { getConfigFileUri, validateSamBuildConfig } from './config'
3131
import { runInTerminal } from './processTerminal'
32+
import { SemVer } from 'semver'
3233

3334
const buildMementoRootKey = 'samcli.build.params'
3435
export interface BuildParams {
@@ -210,10 +211,13 @@ export async function runBuild(arg?: TreeNode): Promise<SamBuildResult> {
210211
const projectRoot = params.projectRoot
211212

212213
const defaultFlags: string[] = ['--cached', '--parallel', '--save-params', '--use-container']
214+
215+
const { path: samCliPath, parsedVersion } = await getSamCliPathAndVersion()
216+
213217
// refactor
214218
const buildFlags: string[] =
215219
params.paramsSource === ParamsSource.Specify && params.buildFlags
216-
? JSON.parse(params.buildFlags)
220+
? await resolveBuildFlags(JSON.parse(params.buildFlags), parsedVersion)
217221
: await getBuildFlags(params.paramsSource, projectRoot, defaultFlags)
218222

219223
// todo remove
@@ -229,8 +233,6 @@ export async function runBuild(arg?: TreeNode): Promise<SamBuildResult> {
229233
await updateRecentResponse(buildMementoRootKey, 'global', 'templatePath', templatePath)
230234

231235
try {
232-
const { path: samCliPath } = await getSamCliPathAndVersion()
233-
234236
// Create a child process to run the SAM build command
235237
const buildProcess = new ChildProcess(samCliPath, ['build', ...buildFlags], {
236238
spawnOptions: await addTelemetryEnvVar({
@@ -281,3 +283,13 @@ function resolveBuildArgConflict(boundArgs: string[]): string[] {
281283
// }
282284
return Array.from(boundArgsSet)
283285
}
286+
export async function resolveBuildFlags(buildFlags: string[], samCliVersion: SemVer | null): Promise<string[]> {
287+
// --no-use-container was not added until v1.133.0
288+
if (samCliVersion?.compare('1.133.0') ?? -1 < 0) {
289+
return buildFlags
290+
}
291+
if (!buildFlags.includes('--use-container')) {
292+
buildFlags.push('--no-use-container')
293+
}
294+
return buildFlags
295+
}

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
createParamsSourcePrompter,
1717
getBuildFlags,
1818
ParamsSource,
19+
resolveBuildFlags,
1920
runBuild,
2021
} from '../../../shared/sam/build'
2122
import { TreeNode } from '../../../shared/treeview/resourceTreeDataProvider'
@@ -26,12 +27,14 @@ import { getProjectRootUri } from '../../../shared/sam/utils'
2627
import sinon from 'sinon'
2728
import { createMultiPick, DataQuickPickItem } from '../../../shared/ui/pickerPrompter'
2829
import * as config from '../../../shared/sam/config'
30+
import * as utils from '../../../shared/sam/utils'
2931
import { PrompterTester } from '../wizards/prompterTester'
3032
import { getWorkspaceFolder, TestFolder } from '../../testUtil'
3133
import { samconfigCompleteData, validTemplateData } from './samTestUtils'
3234
import { CloudFormationTemplateRegistry } from '../../../shared/fs/templateRegistry'
3335
import { getTestWindow } from '../vscode/window'
3436
import { CancellationError } from '../../../shared/utilities/timeoutUtils'
37+
import { SemVer } from 'semver'
3538

3639
describe('SAM BuildWizard', async function () {
3740
const createTester = async (params?: Partial<BuildParams>, arg?: TreeNode | undefined) =>
@@ -229,6 +232,38 @@ describe('SAM build helper functions', () => {
229232
assert.deepStrictEqual(quickPick.items, expectedItems)
230233
})
231234
})
235+
236+
describe('resolveBuildFlags', () => {
237+
let sandbox: sinon.SinonSandbox
238+
beforeEach(() => {
239+
sandbox = sinon.createSandbox()
240+
})
241+
242+
afterEach(() => {
243+
sandbox.restore()
244+
})
245+
246+
it('uses --no-use-container when --use-container is absent', async () => {
247+
const normalVersion = new SemVer('1.133.0')
248+
const buildFlags = ['--cached', '--debug', '--parallel']
249+
const expectedBuildFlags = ['--cached', '--debug', '--parallel', '--no-use-container']
250+
return testResolveBuildFlags(sandbox, normalVersion, buildFlags, expectedBuildFlags)
251+
})
252+
253+
it('preserves buildFlags when SAM CLI version < 1.133', async () => {
254+
const lowerVersion = new SemVer('1.110.0')
255+
const buildFlags = ['--cached', '--parallel', '--save-params']
256+
const expectedBuildFlags = ['--cached', '--parallel', '--save-params']
257+
return testResolveBuildFlags(sandbox, lowerVersion, buildFlags, expectedBuildFlags)
258+
})
259+
260+
it('respects existing --use-container flag', async () => {
261+
const normalVersion = new SemVer('1.110.0')
262+
const buildFlags = ['--cached', '--parallel', '--save-params', '--use-container']
263+
const expectedBuildFlags = ['--cached', '--parallel', '--save-params', '--use-container']
264+
return testResolveBuildFlags(sandbox, normalVersion, buildFlags, expectedBuildFlags)
265+
})
266+
})
232267
})
233268

234269
describe('SAM runBuild', () => {
@@ -551,3 +586,16 @@ describe('SAM runBuild', () => {
551586
})
552587
})
553588
})
589+
590+
function testResolveBuildFlags(
591+
sandbox: sinon.SinonSandbox,
592+
parsedVersion: SemVer,
593+
buildFlags: string[],
594+
expectedBuildFlags: string[]
595+
) {
596+
const pathAndVersionStub = sandbox.stub().resolves({ path: 'file:///path/to/cli', parsedVersion })
597+
sandbox.stub(utils, 'getSamCliPathAndVersion').callsFake(pathAndVersionStub)
598+
return resolveBuildFlags(buildFlags, parsedVersion).then((resolvedBuildFlags) => {
599+
assert.deepEqual(resolvedBuildFlags, expectedBuildFlags)
600+
})
601+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "appBuilder: pass '--no-use-container' when '--use-container' is not selected in quickpick"
4+
}

0 commit comments

Comments
 (0)