Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @siye566 on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check' |
🦋 Changeset detectedLatest commit: 78010e7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
TASTE.md review: complies — 0 violations (checked JS/Python parity T-1/T-2, error classes T-42/T-57–T-59, enum/literal rules T-15, docs T-69–T-71; changes are confined to private matcher helpers compile/_compile with identical logic in both SDKs and no public surface change).
| and not _WILDCARD_CHARS.search(suffix) | ||
| and "]" not in suffix | ||
| ): | ||
| return re.compile("^.*" + re.escape(suffix) + "$") |
There was a problem hiding this comment.
🟡 Globstar suffixes misclassify newline filenames
With **.txt, _compile retains a\n.txt and excludes a.txt\n. Regex . cannot cross newlines, while $ matches before a trailing newline. The JS matcher has the same behavior.
Learn more
The matcher compiles a leading globstar and literal suffix into a regular expression. Python's . does not match a newline unless DOTALL is enabled, and $ also matches just before a final newline. The same defaults apply in the JS matcher. Filenames can contain newlines, so both cases disagree with literal suffix matching and affect template file selection.
Example: Given **.txt, a\n.txt must be ignored but stays in the copy list; a.txt\n must remain in the list but is ignored.
Recommended fix: Check path.endswith(suffix) / path.endsWith(suffix) directly for this branch, or use a newline-safe regex prefix and a true end-of-string assertion in both SDKs. Add matching tests for embedded and trailing newlines.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — fixed in 78010e7. Both SDKs now do a true literal suffix match that is newline-safe:
- Python: the suffix branch compiles with
re.DOTALLand anchors with\Zinstead of$(Python's$also matches just before a trailing newline, and.does not cross newlines by default), soa\n.txtis ignored anda.txt\nstays in the copy list. - JS: same branch now uses the
s(dotAll) flag;$withoutmalready anchors to the absolute end of input, so only the embedded-newline direction was broken there.
Added string-level PatternMatcher unit tests in both SDKs (packages/python-sdk/tests/shared/template/utils/test_dockerignore_matcher.py, packages/js-sdk/tests/template/dockerignore.test.ts) covering normal, embedded-newline and trailing-newline names, plus extended the changeset. Red-green verified: the newline cases fail without each fix and pass with it; ruff/oxlint/prettier/tsc clean.
|
@cla-bot check |
|
We require contributors to sign our Contributor License Agreement, and we don't have @siye566 on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check' |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
Follow-up to the leading-globstar literal-suffix fix (e2b-dev#1933), addressing a review comment on e2b-dev#1934: the suffix branch compiled to a plain regex where '.' does not cross newlines and (in Python) '$' also matches just before a trailing newline, so a filename containing a newline was matched wrongly in both directions. - Python: compile the suffix branch with re.DOTALL and anchor with '\Z', not '$'. - JS: add the 's' (dotAll) flag; '$' without 'm' already anchors to the absolute end. - Add string-level PatternMatcher unit tests in both SDKs covering normal, embedded-newline and trailing-newline names (deterministic, no filesystem). - Extend the changeset with the newline semantics.
This branch has not been deployed
1 participant
Summary
Follow-up to #1917. The Dockerignore port currently compiles
**.txtto an optional directory prefix followed by the literal.txt, soroot.txtandsrc/nested.txtremain in the template copy file list.Moby uses suffix matching when a leading
**is followed only by literal characters. Apply that case in both SDKs while leaving the existing regex path unchanged for additional wildcard syntax. Python sync and async templates share this matcher. No new dependencies.*,?, or bracket expressions.e2band@e2b/python-sdk.Usage example
With
**.txtin.dockerignore, copying a context containingroot.txt,src/nested.txt, andkeep.txt.baknow excludes the first two and retains the backup.!**.tsre-includes matching files under excluded directories.Validation
root.txtwas still included.pnpm --dir packages/js-sdk run test --project template tests/template/utils: 89 passed, 1 skipped.uv run pytest tests/shared/template/utils -k 'not test_should_handle_symlinks and not test_should_resolve_symlinks_when_enabled and not test_should_preserve_symlinks_when_disabled' -q: 96 passed, 1 skipped, 3 deselected.format,lint,typecheck, andbuildcompleted successfully.ruff check .,ruff format --check ., andty checkpassed.git diff --cached --checkpassed.Tested on Windows with Node 22.23.2, Python 3.12.14, and pnpm 10.34.5. The three deselected existing Python tests fail with WinError 1314 (symlink privilege unavailable); the directory-symlink test is already skipped on Windows. GNU Make is unavailable, so the Python Makefile's actual commands were run directly. Validation was SDK-scoped; the whole-workspace commands and live cloud suites were not run, and no E2B credentials were used.
AI-assisted with Codex; the commands above were executed locally.