Repository navigation
computerd: Report entry types and inodes from readdir #249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@cloudflare/computerd": patch | ||
| --- | ||
|
|
||
| Directory listings on the mount now report each entry's type and inode, on synced and local-only paths alike. Before, every entry came back as `DT_UNKNOWN`, which `bun install` treats as nothing to link: with its cache under a local-only path such as `/workspace/.cache`, it left packages out of `node_modules` and failed with `failed to enqueue lifecycle scripts ...: ENOENT`. Tools that read `d_type`, such as `find` and `git`, no longer need an `lstat` per entry to learn it. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -232,24 +232,44 @@ export function withLocalPassthrough( | |
| // A VFS directory may still contain local-only children: the | ||
| // entries live on disk but the parent does not. Merge both | ||
| // sides so `ls` shows what a command inside the container sees. | ||
| ops.readdir(path, (code, names) => { | ||
| ops.readdir(path, (code, names, stats) => { | ||
| if (code !== 0) { | ||
| cb(code, names); | ||
| cb(code, names, stats); | ||
| return; | ||
| } | ||
| const extra = localChildren(path); | ||
| if (extra.length === 0) { | ||
| cb(0, names); | ||
| cb(0, names, stats); | ||
| return; | ||
| } | ||
| const merged = new Set([...(names ?? []), ...extra]); | ||
| cb(0, [...merged]); | ||
| if (stats === undefined || stats.length !== names.length) { | ||
| const merged = new Set([...(names ?? []), ...extra]); | ||
| cb(0, [...merged]); | ||
| return; | ||
| } | ||
| // A local-only child shadows a synced entry of the same name, | ||
| // as getattr does, so its stat is the one reported. | ||
| const merged = new Map<string, FuseStat>(); | ||
| names.forEach((name, i) => { | ||
| const stat = stats[i]; | ||
| if (stat !== undefined) merged.set(name, stat); | ||
| }); | ||
| for (const [name, stat] of statLocalEntries(localPath(path), extra)) { | ||
| merged.set(name, stat); | ||
| } | ||
| cb(0, [...merged.keys()], [...merged.values()]); | ||
| }); | ||
| return; | ||
| } | ||
| localOps += 1; | ||
| try { | ||
| cb(0, fs.readdirSync(localPath(path))); | ||
| const directory = localPath(path); | ||
| const entries = statLocalEntries(directory, fs.readdirSync(directory) as string[]); | ||
| cb( | ||
| 0, | ||
| entries.map(([name]) => name), | ||
| entries.map(([, stat]) => stat), | ||
| ); | ||
| } catch (error) { | ||
| cb(toErrno(error), []); | ||
| } | ||
|
|
@@ -758,6 +778,21 @@ export function withLocalPassthrough( | |
| return names.filter((name) => isLocal(parent === "" ? name : `${parent}/${name}`)); | ||
| } | ||
|
|
||
| // lstat, as getattr does, so the type and inode match what a lookup of | ||
| // the same name returns. An entry removed between the listing and the | ||
| // stat is left out. | ||
| function statLocalEntries(directory: string, names: string[]): Array<[string, FuseStat]> { | ||
| const entries: Array<[string, FuseStat]> = []; | ||
| for (const name of names) { | ||
| try { | ||
| entries.push([name, statToFuse(fs.lstatSync(join(directory, name)))]); | ||
| } catch { | ||
| // Gone since the listing. | ||
| } | ||
| } | ||
|
Comment on lines
+787
to
+792
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Readable local directories appear empty For a readable local directory without search permission, Learn moreReading a directory requires read permission, but looking up its children also requires search permission. The local listing obtains names using readdirSync, then this helper discards any child that cannot be stat'd. For a mode Example: A local-only Recommended fix: Only omit a child when its stat fails because it disappeared (ENOENT/ENOTDIR); keep other names with a stat fallback that reports unknown type, or propagate a suitable error instead of silently hiding them. Ensure the merge path handles a missing stat without losing names. Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| return entries; | ||
| } | ||
|
|
||
| function hasLocalContents(path: string): boolean { | ||
| try { | ||
| return fs.readdirSync(localPath(path)).length > 0; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.