computerd: Support globs in ignore rules - #192
Conversation
馃 Changeset detectedLatest commit: 246a845 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
8dc13e8 to
1f86e67
Compare
| const trimmed = stripSlashes(body); | ||
| const parts = trimmed === "" ? [] : trimmed.split("/"); | ||
| if (parts.some((part) => part === "")) fail("contains an empty path segment."); | ||
| if (parts.some((part) => part === "." || part === "..")) { | ||
| fail(`contains a "." or ".." segment.`); | ||
| } | ||
| if (parts.some((part) => part !== "**" && part.includes("**"))) { | ||
| fail(`uses "**" inside a segment. "**" must be a whole path segment.`); | ||
| } | ||
| if (parts.every((part) => part === "**")) { |
There was a problem hiding this comment.
馃煛 Mount-root patterns fail after construction
With ignore: ["/workspace"], checkIgnorePatterns accepts a pattern the daemon rejects after stripping its mount prefix. The container fails at startup instead of rejecting the configuration when the backend is constructed.
Learn more
The host validates ignore in the backend constructor to catch rejected patterns before launching the container. It checks raw segments, but compilePattern first removes the mount point. A raw pattern that names the mount itself therefore passes on the host but becomes an invalid whole-mount pattern in the daemon. The same issue affects /workspace/**.
Example: Constructing a backend with ignore: ["/workspace"] succeeds. Its container receives that pattern, then exits during startup because the normalized pattern is /.
Recommended fix: Apply the daemon's mount-prefix normalization before the host's whole-mount check. Account for the effective MOUNT_POINT from containerEnv, and test both /workspace and /workspace/**.
Was this helpful? React with 馃憤 or 馃憥 to provide feedback.
| const sample = pattern.segments | ||
| .filter((segment) => segment.kind !== "globstar") | ||
| .map((segment) => (segment.kind === "literal" ? segment.value : segment.sample)); | ||
| for (let depth = 1; depth < sample.length; depth += 1) { | ||
| if (ignores(sample.slice(0, depth).join("/"))) return true; | ||
| } |
There was a problem hiding this comment.
馃攳 Exclusion warning uses one sample path
isShadowed tests one synthesized path per exclusion, not every path the pattern matches. Its warning can therefore label an effective exclusion ineffective; check whether this diagnostic needs stronger matching or a qualified message.
Was this helpful? React with 馃憤 or 馃憥 to provide feedback.
commit: |
MOUNT_IGNORE took plain paths from the mount root, so it could not say "every node_modules, wherever it is". In a monorepo that meant listing every package by hand. It now takes a small subset of gitignore: `*` within a path segment, `**` for any number of segments, and a leading `!` to exclude. MOUNT_IGNORE=**/node_modules,!/vendor/node_modules,/packages/*/dist Every pattern must start with "/" (from the mount root) or "**/" (at any depth). A bare `node_modules` would mean root-only here and any depth in gitignore, so it is rejected with both spellings suggested rather than left to be misread. Braces, character classes, `?`, and backslashes are rejected rather than matched literally, and so is any pattern that would make the whole mount local-only. The last matching pattern wins, and a path is local-only if it or any directory above it is ignored. That is git's rule, and it follows from how the two sides are stored: once a directory is on local disk, the synced filesystem has no directory for an excluded child to live in. An exclusion under a local-only directory therefore does nothing, and computerd warns about it at startup and lists it on /__computerd/info as `ineffectiveExclusions`. The ignore block reports `patterns` in the order written, replacing `paths` and `redundant`, since order now changes the meaning and a pattern cannot be joined onto the mount point. Deciding a path still never touches disk. The patterns are compiled once, and the check walks the path's directories against them. Globs make some situations routine that were rare with fixed paths, and the local-only layer now handles them. A synced directory listing includes local-only children at any depth, read from the matching directory under the local root, but not the scaffolding directories that only exist to hold them. Renaming a synced directory moves its local-only contents with it, so packages/foo/node_modules is not left behind on disk; the two moves cannot be atomic together, so a failure on the local side is logged rather than returned. Removing a synced directory returns ENOTEMPTY while it still holds local-only contents, which the synced side cannot see, and cleans up its scaffolding once it is gone.
computerd now reads MOUNT_IGNORE as glob patterns and reports them as `patterns` on /__computerd/info. The client follows. `ignore` takes the same patterns, such as "**/node_modules" and "!/vendor/node_modules", and the constructor checks them against the rules computerd applies. A pattern computerd would refuse, like a bare "node_modules", now throws before any container starts instead of surfacing as a daemon that exits during startup. The rules are duplicated rather than imported, since this package does not depend on computerd, and the two test suites cover the same cases. A pattern containing a comma is rejected too, because it would split into two entries in MOUNT_IGNORE. connect() compares the declared and applied patterns in order, after normalizing the declaration the way computerd does. Order matters once exclusions are involved: the same two patterns can keep a path synced one way round and local-only the other. `BackendHandle.ignore.paths` becomes `patterns`, and readIgnoreReport no longer joins entries onto the mount point, which made sense for plain paths but not for "**/node_modules". A computerd that reports `paths` predates patterns, so it is treated as unsupported and a connect that declared `ignore` fails.
Adds an end-to-end test for patterns: with `**/node_modules,!/vendor/node_modules,!**/node_modules/.bin`, a nested node_modules lands on local disk, the vendored one and ordinary source stay in the VFS, and the .bin exclusion is reported as having no effect. It then renames the synced package that holds the nested node_modules and checks the contents moved with it, and removes the package with rm -rf and checks both sides are gone. Like the other real-FUSE tests, it runs only on Linux with /dev/fuse and skips elsewhere.
The computerd README described MOUNT_IGNORE as plain paths with no glob syntax. It now covers the patterns: what each form matches, the anchoring rule, last-match-wins, and why an exclusion under a local-only directory does nothing. It lists the patterns rejected at startup and why, and that ContainerBackend checks the same rules in its constructor. The /__computerd/info example and the handle example show `patterns` and `ineffectiveExclusions` instead of `paths` and `redundant`. A new section explains how computerd keeps a synced directory and the local-only paths under it in step on listing, rename, and removal, and the one case it can't: a rename that changes which patterns match. The changeset and the performance doc's wording follow.
`/**` was refused for making the whole mount local-only, but `/*` was accepted, and it does the same thing: it matches every top-level entry, and everything under a local-only directory is local-only. So a workspace configured with `/*`, easily written when meaning `/*.log`, would keep every file off the Durable Object and lose it all when the container was replaced. computerd and ContainerBackend now refuse any pattern made only of `*` and `**` segments, such as `/*`, `**/*`, and `/*/*`. Targeted wildcards like `/*.log` and `/packages/*/dist` are unaffected.
The synced side never sees local-only children, so it would let a rename replace a directory that still held, say, node_modules on local disk. The follow-up move of the source's own local-only contents onto that occupied directory then failed, and was only logged, because the synced rename had already succeeded. The source's node_modules was left at a path the mount no longer showed. In the merged view the destination is not empty, so the rename now returns ENOTEMPTY before touching either side, the same answer rmdir already gave for such a directory. A destination holding only empty scaffolding is still replaced, and the source's contents move into it.
0e30cb5 to
246a845
Compare
This allows rules to include patterns rather than fixed paths, which is essential for things line node_modules etc.
ignoreonContainerBackendignorelists paths the container keeps on its own disk instead of syncing to the Durable Object. Use it for rebuildable trees such asnode_modules,.venvand build output.Each entry is a glob pattern that must start with
/(from the mount root) or**/(at any depth):/dist/workspace/distand everything under it**/node_modulesnode_modulesat any depth/packages/*/dist*matches within one segment and never crosses/!/vendor/node_modules!**/node_modules/.bincan't put.binback oncenode_modulesis local-only.node_modules, braces,?, a comma, or one that matches the whole mount.MOUNT_IGNORE. Onconnect()it reads back what computerd applied and refuses the connection unless the patterns match, in the same order.handle.ignorereports{ patterns, root, mountPoint, supported }.workspace.fs.EXDEV.