Skip to content

Commit 56d16a8

Browse files
authored
Fix ignore files in file upload and added tests (#1012)
<!-- CURSOR_SUMMARY --> > [!NOTE] > Ensure file uploads honor ignore patterns (incl. .dockerignore), refactor file discovery/tar streaming in JS/Python SDKs, and add comprehensive tests. > > - **File upload behavior**: > - JS/TS and Python uploads now pass `ignorePatterns` (merged from `fileIgnorePatterns` and `.dockerignore`) to tar creation, ensuring ignored files aren’t uploaded. > - **Refactor/Utilities**: > - Rename `getAllFilesForFilesHash` -> `getAllFilesInPath` with optional directory inclusion in both SDKs. > - Implement `tar_file_stream` (JS and Python) to build archives from `getAllFilesInPath`, with `noDirRecurse` and symlink control. > - Hashing functions updated to use `getAllFilesInPath`. > - Type tweak: allow `None` in Python `stack_traces` during build wait. > - **Integration**: > - JS `uploadFile` and Python async/sync `upload_file` now use tar streaming with ignore patterns; JS `Template` wires ignore patterns into uploads. > - **Tests**: > - Add extensive tests for `getAllFilesInPath` and `tar_file_stream` in JS and Python, covering ignore patterns, directories, sorting, and symlinks. > - Remove obsolete tests relying on old function names. > - **Meta**: > - Changeset entries for `@e2b/python-sdk` and `e2b` (patch). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 2f83444. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 111fe06 commit 56d16a8

File tree

16 files changed

+512
-380
lines changed

16 files changed

+512
-380
lines changed

.changeset/old-carrots-greet.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@e2b/python-sdk': patch
3+
'e2b': patch
4+
---
5+
6+
fixes ignore files in file upload

packages/js-sdk/src/template/buildApi.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,18 @@ export async function uploadFile(
9191
fileName: string
9292
fileContextPath: string
9393
url: string
94+
ignorePatterns: string[]
9495
resolveSymlinks: boolean
9596
},
9697
stackTrace: string | undefined
9798
) {
98-
const { fileName, url, fileContextPath, resolveSymlinks } = options
99+
const { fileName, url, fileContextPath, ignorePatterns, resolveSymlinks } =
100+
options
99101
try {
100102
const { contentLength, uploadStream } = await tarFileStreamUpload(
101103
fileName,
102104
fileContextPath,
105+
ignorePatterns,
103106
resolveSymlinks
104107
)
105108

packages/js-sdk/src/template/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -925,6 +925,10 @@ export class TemplateBase
925925
fileName: src,
926926
fileContextPath: this.fileContextPath.toString(),
927927
url,
928+
ignorePatterns: [
929+
...this.fileIgnorePatterns,
930+
...readDockerignore(this.fileContextPath.toString()),
931+
],
928932
resolveSymlinks: instruction.resolveSymlinks ?? RESOLVE_SYMLINKS,
929933
},
930934
stackTrace

packages/js-sdk/src/template/utils.ts

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,11 @@ export function readDockerignore(contextPath: string): string[] {
3232
* @param ignorePatterns Ignore patterns
3333
* @returns Array of files
3434
*/
35-
export async function getAllFilesForFilesHash(
35+
export async function getAllFilesInPath(
3636
src: string,
3737
contextPath: string,
38-
ignorePatterns: string[]
38+
ignorePatterns: string[],
39+
includeDirectories: boolean = true
3940
) {
4041
const { glob } = await dynamicGlob()
4142
const files = new Map<string, Path>()
@@ -50,7 +51,9 @@ export async function getAllFilesForFilesHash(
5051
for (const file of globFiles) {
5152
if (file.isDirectory()) {
5253
// For directories, add the directory itself and all files inside it
53-
files.set(file.fullpath(), file)
54+
if (includeDirectories) {
55+
files.set(file.fullpath(), file)
56+
}
5457
const dirFiles = await glob(
5558
path.join(path.relative(contextPath, file.fullpath()), '**/*'),
5659
{
@@ -97,7 +100,7 @@ export async function calculateFilesHash(
97100

98101
hash.update(content)
99102

100-
const files = await getAllFilesForFilesHash(src, contextPath, ignorePatterns)
103+
const files = await getAllFilesInPath(src, contextPath, ignorePatterns, true)
101104

102105
if (files.length === 0) {
103106
const error = new Error(`No files found in ${srcPath}`)
@@ -233,25 +236,37 @@ export function padOctal(mode: number): string {
233236
*
234237
* @param fileName Glob pattern for files to include
235238
* @param fileContextPath Base directory for resolving file paths
239+
* @param ignorePatterns Ignore patterns to exclude from the archive
236240
* @param resolveSymlinks Whether to follow symbolic links
237241
* @returns A readable stream of the gzipped tar archive
238242
*/
239243
export async function tarFileStream(
240244
fileName: string,
241245
fileContextPath: string,
246+
ignorePatterns: string[],
242247
resolveSymlinks: boolean
243248
) {
244-
const { globSync } = await dynamicGlob()
245249
const { create } = await dynamicTar()
246-
const files = globSync(fileName, { cwd: fileContextPath })
250+
251+
const allFiles = await getAllFilesInPath(
252+
fileName,
253+
fileContextPath,
254+
ignorePatterns,
255+
true
256+
)
257+
258+
const filePaths = allFiles.map((file) =>
259+
path.relative(fileContextPath, file.fullpath())
260+
)
247261

248262
return create(
249263
{
250264
gzip: true,
251265
cwd: fileContextPath,
252266
follow: resolveSymlinks,
267+
noDirRecurse: true,
253268
},
254-
files
269+
filePaths
255270
)
256271
}
257272

@@ -266,12 +281,14 @@ export async function tarFileStream(
266281
export async function tarFileStreamUpload(
267282
fileName: string,
268283
fileContextPath: string,
284+
ignorePatterns: string[],
269285
resolveSymlinks: boolean
270286
) {
271287
// First pass: calculate the compressed size
272288
const sizeCalculationStream = await tarFileStream(
273289
fileName,
274290
fileContextPath,
291+
ignorePatterns,
275292
resolveSymlinks
276293
)
277294
let contentLength = 0
@@ -284,6 +301,7 @@ export async function tarFileStreamUpload(
284301
uploadStream: await tarFileStream(
285302
fileName,
286303
fileContextPath,
304+
ignorePatterns,
287305
resolveSymlinks
288306
),
289307
}

packages/js-sdk/tests/template/utils/getAllFilesForFilesHash.test.ts renamed to packages/js-sdk/tests/template/utils/getAllFilesInPath.test.ts

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { expect, test, describe, beforeAll, afterAll, beforeEach } from 'vitest'
22
import { writeFile, mkdir, rm } from 'fs/promises'
33
import { join } from 'path'
4-
import { getAllFilesForFilesHash } from '../../../src/template/utils'
4+
import { getAllFilesInPath } from '../../../src/template/utils'
55

6-
describe('getAllFilesForFilesHash', () => {
6+
describe('getAllFilesInPath', () => {
77
const testDir = join(__dirname, 'folder')
88

99
beforeAll(async () => {
@@ -26,7 +26,7 @@ describe('getAllFilesForFilesHash', () => {
2626
await writeFile(join(testDir, 'file2.txt'), 'content2')
2727
await writeFile(join(testDir, 'file3.js'), 'content3')
2828

29-
const files = await getAllFilesForFilesHash('*.txt', testDir, [])
29+
const files = await getAllFilesInPath('*.txt', testDir, [])
3030

3131
expect(files).toHaveLength(2)
3232
expect(files.some((f) => f.fullpath().endsWith('file1.txt'))).toBe(true)
@@ -51,7 +51,7 @@ describe('getAllFilesForFilesHash', () => {
5151
)
5252
await writeFile(join(testDir, 'README.md'), 'readme content')
5353

54-
const files = await getAllFilesForFilesHash('src', testDir, [])
54+
const files = await getAllFilesInPath('src', testDir, [])
5555

5656
expect(files).toHaveLength(6) // 3 files + 3 directories (src, components, utils)
5757
expect(files.some((f) => f.fullpath().endsWith('index.ts'))).toBe(true)
@@ -67,7 +67,7 @@ describe('getAllFilesForFilesHash', () => {
6767
await writeFile(join(testDir, 'temp.txt'), 'temp content')
6868
await writeFile(join(testDir, 'backup.txt'), 'backup content')
6969

70-
const files = await getAllFilesForFilesHash('*.txt', testDir, [
70+
const files = await getAllFilesInPath('*.txt', testDir, [
7171
'temp*',
7272
'backup*',
7373
])
@@ -105,7 +105,7 @@ describe('getAllFilesForFilesHash', () => {
105105
'spec content'
106106
)
107107

108-
const files = await getAllFilesForFilesHash('src', testDir, [
108+
const files = await getAllFilesInPath('src', testDir, [
109109
'**/*.test.*',
110110
'**/*.spec.*',
111111
])
@@ -126,7 +126,7 @@ describe('getAllFilesForFilesHash', () => {
126126
await mkdir(join(testDir, 'empty'), { recursive: true })
127127
await writeFile(join(testDir, 'file.txt'), 'content')
128128

129-
const files = await getAllFilesForFilesHash('empty', testDir, [])
129+
const files = await getAllFilesInPath('empty', testDir, [])
130130

131131
expect(files).toHaveLength(1) // The empty directory itself
132132
})
@@ -138,7 +138,7 @@ describe('getAllFilesForFilesHash', () => {
138138
await writeFile(join(testDir, 'dir1', 'file2.txt'), 'content2')
139139
await writeFile(join(testDir, 'file3.txt'), 'content3')
140140

141-
const files = await getAllFilesForFilesHash('*', testDir, [])
141+
const files = await getAllFilesInPath('*', testDir, [])
142142

143143
expect(files).toHaveLength(4) // 3 files + 1 directory
144144
expect(files.some((f) => f.fullpath().endsWith('file1.txt'))).toBe(true)
@@ -166,7 +166,7 @@ describe('getAllFilesForFilesHash', () => {
166166
'css content'
167167
)
168168

169-
const files = await getAllFilesForFilesHash('src/**/*', testDir, [])
169+
const files = await getAllFilesInPath('src/**/*', testDir, [])
170170

171171
expect(files).toHaveLength(6) // 4 files + 2 directories (components, utils)
172172
expect(files.some((f) => f.fullpath().endsWith('index.ts'))).toBe(true)
@@ -181,7 +181,7 @@ describe('getAllFilesForFilesHash', () => {
181181
await writeFile(join(testDir, 'file3.tsx'), 'tsx content')
182182
await writeFile(join(testDir, 'file4.css'), 'css content')
183183

184-
const files = await getAllFilesForFilesHash('*.ts', testDir, [])
184+
const files = await getAllFilesInPath('*.ts', testDir, [])
185185

186186
expect(files).toHaveLength(1)
187187
expect(files.some((f) => f.fullpath().endsWith('file1.ts'))).toBe(true)
@@ -192,7 +192,7 @@ describe('getAllFilesForFilesHash', () => {
192192
await writeFile(join(testDir, 'apple.txt'), 'a content')
193193
await writeFile(join(testDir, 'banana.txt'), 'b content')
194194

195-
const files = await getAllFilesForFilesHash('*.txt', testDir, [])
195+
const files = await getAllFilesInPath('*.txt', testDir, [])
196196

197197
expect(files).toHaveLength(3)
198198
// Files are sorted by full path, not just filename
@@ -203,7 +203,7 @@ describe('getAllFilesForFilesHash', () => {
203203
test('should handle no matching files', async () => {
204204
await writeFile(join(testDir, 'file.txt'), 'content')
205205

206-
const files = await getAllFilesForFilesHash('*.js', testDir, [])
206+
const files = await getAllFilesInPath('*.js', testDir, [])
207207

208208
expect(files).toHaveLength(0)
209209
})
@@ -232,7 +232,7 @@ describe('getAllFilesForFilesHash', () => {
232232
await writeFile(join(testDir, 'dist', 'bundle.js'), 'bundle content')
233233
await writeFile(join(testDir, 'README.md'), 'readme content')
234234

235-
const files = await getAllFilesForFilesHash('src', testDir, [
235+
const files = await getAllFilesInPath('src', testDir, [
236236
'**/tests/**',
237237
'**/*.spec.*',
238238
])

0 commit comments

Comments
 (0)