Skip to content

Commit 8fd89a3

Browse files
authored
fix(amazonq): allow including binary files when execution is enabled #6430
## Problem * Fixes for execution engine ## Solution * Don't drop binary files matching with extra gitignore rules * Don't drop `.gitignore` file from repository.
1 parent 4687c99 commit 8fd89a3

File tree

5 files changed

+67
-37
lines changed

5 files changed

+67
-37
lines changed

packages/amazonq/test/unit/amazonqFeatureDev/util/files.test.ts

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,45 @@ import { MetricName, Span } from 'aws-core-vscode/telemetry'
1616
import sinon from 'sinon'
1717
import { CodeWhispererSettings } from 'aws-core-vscode/codewhisperer'
1818

19-
const testDevfilePrepareRepo = async (expectedRepoSize: number, devfileEnabled: boolean) => {
19+
import AdmZip from 'adm-zip'
20+
21+
const testDevfilePrepareRepo = async (devfileEnabled: boolean) => {
22+
const files: Record<string, string> = {
23+
'file.md': 'test content',
24+
// only include when execution is enabled
25+
'devfile.yaml': 'test',
26+
// .git folder is always dropped (because of vscode global exclude rules)
27+
'.git/ref': '####',
28+
// .gitignore should always be included
29+
'.gitignore': 'node_models/*',
30+
// non code files only when dev execution is enabled
31+
'abc.jar': 'jar-content',
32+
'data/logo.ico': 'binary-content',
33+
}
2034
const folder = await TestFolder.create()
21-
await folder.write('devfile.yaml', 'test')
22-
await folder.write('file.md', 'test content')
35+
36+
for (const [fileName, content] of Object.entries(files)) {
37+
await folder.write(fileName, content)
38+
}
39+
40+
const expectedFiles = !devfileEnabled
41+
? ['./file.md', './.gitignore']
42+
: ['./devfile.yaml', './file.md', './.gitignore', './abc.jar', 'data/logo.ico']
43+
2344
const workspace = getWorkspaceFolder(folder.path)
2445
sinon
2546
.stub(CodeWhispererSettings.instance, 'getDevCommandWorkspaceConfigurations')
2647
.returns(devfileEnabled ? { [workspace.uri.fsPath]: true } : {})
2748

28-
await testPrepareRepoData(workspace, expectedRepoSize)
49+
await testPrepareRepoData(workspace, expectedFiles)
2950
}
3051

3152
const testPrepareRepoData = async (
3253
workspace: vscode.WorkspaceFolder,
33-
expectedRepoSize: number,
54+
expectedFiles: string[],
3455
expectedTelemetryMetrics?: Array<{ metricName: MetricName; value: any }>
3556
) => {
57+
expectedFiles.sort((a, b) => a.localeCompare(b))
3658
const telemetry = new TelemetryHelper()
3759
const result = await prepareRepoData([workspace.uri.fsPath], [workspace], telemetry, {
3860
record: () => {},
@@ -41,13 +63,18 @@ const testPrepareRepoData = async (
4163
assert.strictEqual(Buffer.isBuffer(result.zipFileBuffer), true)
4264
// checksum is not the same across different test executions because some unique random folder names are generated
4365
assert.strictEqual(result.zipFileChecksum.length, 44)
44-
assert.strictEqual(telemetry.repositorySize, expectedRepoSize)
4566

4667
if (expectedTelemetryMetrics) {
47-
expectedTelemetryMetrics.forEach((metric) => {
68+
for (const metric of expectedTelemetryMetrics) {
4869
assertTelemetry(metric.metricName, metric.value)
49-
})
70+
}
5071
}
72+
73+
// Unzip the buffer and compare the entry names
74+
const zip = new AdmZip(result.zipFileBuffer)
75+
const actualZipEntries = zip.getEntries().map((entry) => entry.entryName)
76+
actualZipEntries.sort((a, b) => a.localeCompare(b))
77+
assert.deepStrictEqual(actualZipEntries, expectedFiles)
5178
}
5279

5380
describe('file utils', () => {
@@ -62,25 +89,27 @@ describe('file utils', () => {
6289
await folder.write('file2.md', 'test content')
6390
const workspace = getWorkspaceFolder(folder.path)
6491

65-
await testPrepareRepoData(workspace, 24)
92+
await testPrepareRepoData(workspace, ['./file1.md', './file2.md'])
6693
})
6794

6895
it('prepareRepoData ignores denied file extensions', async function () {
6996
const folder = await TestFolder.create()
7097
await folder.write('file.mp4', 'test content')
7198
const workspace = getWorkspaceFolder(folder.path)
7299

73-
await testPrepareRepoData(workspace, 0, [
74-
{ metricName: 'amazonq_bundleExtensionIgnored', value: { filenameExt: 'mp4', count: 1 } },
75-
])
100+
await testPrepareRepoData(
101+
workspace,
102+
[],
103+
[{ metricName: 'amazonq_bundleExtensionIgnored', value: { filenameExt: 'mp4', count: 1 } }]
104+
)
76105
})
77106

78107
it('should ignore devfile.yaml when setting is disabled', async function () {
79-
await testDevfilePrepareRepo(12, false)
108+
await testDevfilePrepareRepo(false)
80109
})
81110

82111
it('should include devfile.yaml when setting is enabled', async function () {
83-
await testDevfilePrepareRepo(16, true)
112+
await testDevfilePrepareRepo(true)
84113
})
85114

86115
// Test the logic that allows the customer to modify root source folder

packages/core/src/amazonqFeatureDev/util/files.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@ export async function prepareRepoData(
4040
zip: AdmZip = new AdmZip()
4141
) {
4242
try {
43-
const files = await collectFiles(repoRootPaths, workspaceFolders, true, maxRepoSizeBytes)
4443
const devCommandWorkspaceConfigurations = CodeWhispererSettings.instance.getDevCommandWorkspaceConfigurations()
4544
const useAutoBuildFeature = devCommandWorkspaceConfigurations[repoRootPaths[0]] ?? false
45+
// We only respect gitignore file rules if useAutoBuildFeature is on, this is to avoid dropping necessary files for building the code (e.g. png files imported in js code)
46+
const files = await collectFiles(repoRootPaths, workspaceFolders, true, maxRepoSizeBytes, !useAutoBuildFeature)
4647

4748
let totalBytes = 0
4849
const ignoredExtensionMap = new Map<string, number>()
@@ -59,10 +60,10 @@ export async function prepareRepoData(
5960
throw error
6061
}
6162
const isCodeFile_ = isCodeFile(file.relativeFilePath)
62-
// exclude user's devfile if `useAutoBuildFeature` is set to false
63-
const excludeDevFile = useAutoBuildFeature ? false : file.relativeFilePath === 'devfile.yaml'
64-
65-
if (fileSize >= maxFileSizeBytes || !isCodeFile_ || excludeDevFile) {
63+
const isDevFile = file.relativeFilePath === 'devfile.yaml'
64+
// When useAutoBuildFeature is on, only respect the gitignore rules filtered earlier and apply the size limit, otherwise, exclude all non code files and gitignore files
65+
const isNonCodeFileAndIgnored = useAutoBuildFeature ? false : !isCodeFile_ || isDevFile
66+
if (fileSize >= maxFileSizeBytes || isNonCodeFileAndIgnored) {
6667
if (!isCodeFile_) {
6768
const re = /(?:\.([^.]+))?$/
6869
const extensionArray = re.exec(file.relativeFilePath)

packages/core/src/shared/filetypes.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,6 @@ export const codefileExtensions = new Set([
233233
'.idl',
234234
'.ini',
235235
'.io',
236-
'.jar',
237236
'.java',
238237
'.jl',
239238
'.js',
@@ -362,17 +361,10 @@ export const codefileExtensions = new Set([
362361
])
363362

364363
// Code file names without an extension
365-
export const codefileNames = new Set(['Dockerfile', 'Dockerfile.build', 'gradlew', 'mvnw'])
366-
367-
// Build file names
368-
export const buildfileNames = new Set(['gradle/wrapper/gradle-wrapper.jar'])
364+
export const codefileNames = new Set(['Dockerfile', 'Dockerfile.build', 'gradlew', 'mvnw', '.gitignore'])
369365

370366
/** Returns true if `filename` is a code file. */
371367
export function isCodeFile(filename: string): boolean {
372368
const ext = path.extname(filename).toLowerCase()
373-
return (
374-
codefileExtensions.has(ext) ||
375-
codefileNames.has(path.basename(filename)) ||
376-
buildfileNames.has(path.basename(filename))
377-
)
369+
return codefileExtensions.has(ext) || codefileNames.has(path.basename(filename))
378370
}

packages/core/src/shared/utilities/workspaceUtils.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ export function checkUnsavedChanges(): boolean {
268268
return vscode.workspace.textDocuments.some((doc) => doc.isDirty)
269269
}
270270

271-
export function getExcludePattern(additionalPatterns: string[] = []) {
271+
export function getExcludePattern(defaultExcludePatterns: boolean = true) {
272272
const globAlwaysExcludedDirs = getGlobDirExcludedPatterns().map((pattern) => `**/${pattern}/*`)
273273
const extraPatterns = [
274274
'**/package-lock.json',
@@ -290,19 +290,24 @@ export function getExcludePattern(additionalPatterns: string[] = []) {
290290
'**/License.md',
291291
'**/LICENSE.md',
292292
]
293-
const allPatterns = [...globAlwaysExcludedDirs, ...extraPatterns, ...additionalPatterns]
293+
const allPatterns = [...globAlwaysExcludedDirs, ...(defaultExcludePatterns ? extraPatterns : [])]
294294
return `{${allPatterns.join(',')}}`
295295
}
296296

297297
/**
298298
* @param rootPath root folder to look for .gitignore files
299+
* @param addExtraPatterns whether to add extra exclude patterns even if not in gitignore
299300
* @returns list of glob patterns extracted from .gitignore
300301
* These patterns are compatible with vscode exclude patterns
301302
*/
302-
async function filterOutGitignoredFiles(rootPath: string, files: vscode.Uri[]): Promise<vscode.Uri[]> {
303+
async function filterOutGitignoredFiles(
304+
rootPath: string,
305+
files: vscode.Uri[],
306+
defaultExcludePatterns: boolean = true
307+
): Promise<vscode.Uri[]> {
303308
const gitIgnoreFiles = await vscode.workspace.findFiles(
304309
new vscode.RelativePattern(rootPath, '**/.gitignore'),
305-
getExcludePattern()
310+
getExcludePattern(defaultExcludePatterns)
306311
)
307312
const gitIgnoreFilter = await GitIgnoreFilter.build(gitIgnoreFiles)
308313
return gitIgnoreFilter.filterFiles(files)
@@ -313,13 +318,15 @@ async function filterOutGitignoredFiles(rootPath: string, files: vscode.Uri[]):
313318
* @param sourcePaths the paths where collection starts
314319
* @param workspaceFolders the current workspace folders opened
315320
* @param respectGitIgnore whether to respect gitignore file
321+
* @param addExtraIgnorePatterns whether to add extra exclude patterns even if not in gitignore
316322
* @returns all matched files
317323
*/
318324
export async function collectFiles(
319325
sourcePaths: string[],
320326
workspaceFolders: CurrentWsFolders,
321327
respectGitIgnore: boolean = true,
322-
maxSize = 200 * 1024 * 1024 // 200 MB
328+
maxSize = 200 * 1024 * 1024, // 200 MB
329+
defaultExcludePatterns: boolean = true
323330
): Promise<
324331
{
325332
workspaceFolder: vscode.WorkspaceFolder
@@ -356,10 +363,12 @@ export async function collectFiles(
356363
for (const rootPath of sourcePaths) {
357364
const allFiles = await vscode.workspace.findFiles(
358365
new vscode.RelativePattern(rootPath, '**'),
359-
getExcludePattern()
366+
getExcludePattern(defaultExcludePatterns)
360367
)
361368

362-
const files = respectGitIgnore ? await filterOutGitignoredFiles(rootPath, allFiles) : allFiles
369+
const files = respectGitIgnore
370+
? await filterOutGitignoredFiles(rootPath, allFiles, defaultExcludePatterns)
371+
: allFiles
363372

364373
for (const file of files) {
365374
const relativePath = getWorkspaceRelativePath(file.fsPath, { workspaceFolders })

packages/core/src/test/shared/filetypes.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ describe('isCodeFile', () => {
160160
'mvnw',
161161
'build.gradle',
162162
'gradle/wrapper/gradle-wrapper.properties',
163-
'gradle/wrapper/gradle-wrapper.jar',
164163
]
165164
for (const codeFilePath of codeFiles) {
166165
assert.strictEqual(isCodeFile(codeFilePath), true)

0 commit comments

Comments
 (0)