From 2b86ab0cd3a17341bf7bef6f7d2fd5daaa8218b2 Mon Sep 17 00:00:00 2001 From: robobun Date: Wed, 14 Jan 2026 13:31:06 -0800 Subject: [PATCH] fix(shell): implement long listing format for `ls -l` builtin (#25991) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Implements the `-l` (long listing) flag functionality for the shell `ls` builtin - The flag was being parsed but never used - output was identical to short format - Now displays proper long listing format: file type, permissions, hard link count, UID, GID, size, modification time, and filename ## Test plan - [x] Added regression test in `test/regression/issue/25831.test.ts` - [x] Test passes with debug build: `bun bd test test/regression/issue/25831.test.ts` - [x] Test fails with system bun (confirming the bug exists): `USE_SYSTEM_BUN=1 bun test test/regression/issue/25831.test.ts` Example output with fix: ``` $ bun -e 'import { $ } from "bun"; console.log(await $`ls -l`.text())' drwxr-xr-x 2 1000 1000 4096 Jan 12 15:30 subdir -rw-r--r-- 1 1000 1000 11 Jan 12 15:30 file.txt ``` Fixes #25831 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Bot Co-authored-by: Claude Opus 4.5 --- src/shell/builtin/ls.zig | 175 ++++++++++++++++++++++++++-- src/sys.zig | 22 ++++ test/regression/issue/25831.test.ts | 169 +++++++++++++++++++++++++++ 3 files changed, 356 insertions(+), 10 deletions(-) create mode 100644 test/regression/issue/25831.test.ts diff --git a/src/shell/builtin/ls.zig b/src/shell/builtin/ls.zig index 943b40e6c5..b5b68e5678 100644 --- a/src/shell/builtin/ls.zig +++ b/src/shell/builtin/ls.zig @@ -225,6 +225,9 @@ pub const ShellLsTask = struct { is_absolute: bool = false, err: ?Syscall.Error = null, result_kind: enum { file, dir, idk } = .idk, + /// Cached current time (seconds since epoch) for formatting timestamps. + /// Cached once per task to avoid repeated syscalls. + #now_secs: u64 = 0, event_loop: jsc.EventLoopHandle, concurrent_task: jsc.EventLoopTask, @@ -293,6 +296,11 @@ pub const ShellLsTask = struct { } pub fn run(this: *@This()) void { + // Cache current time once per task for timestamp formatting + if (this.opts.long_listing) { + this.#now_secs = @intCast(std.time.timestamp()); + } + const fd = switch (ShellSyscall.openat(this.cwd, this.path, bun.O.RDONLY | bun.O.DIRECTORY, 0)) { .err => |e| { switch (e.getErrno()) { @@ -301,7 +309,7 @@ pub const ShellLsTask = struct { }, .NOTDIR => { this.result_kind = .file; - this.addEntry(this.path); + this.addEntry(this.path, this.cwd); }, else => { this.err = this.errorWithPath(e, this.path); @@ -329,7 +337,7 @@ pub const ShellLsTask = struct { // If `-a` is used, "." and ".." should show up as results. However, // our `DirIterator` abstraction skips them, so let's just add them // now. - this.addDotEntriesIfNeeded(); + this.addDotEntriesIfNeeded(fd); while (switch (entry) { .err => |e| { @@ -338,7 +346,7 @@ pub const ShellLsTask = struct { }, .result => |ent| ent, }) |current| : (entry = iterator.next()) { - this.addEntry(current.name.sliceAssumeZ()); + this.addEntry(current.name.sliceAssumeZ(), fd); if (current.kind == .directory and this.opts.recursive) { this.enqueue(current.name.sliceAssumeZ()); } @@ -367,20 +375,167 @@ pub const ShellLsTask = struct { } // TODO more complex output like multi-column - fn addEntry(this: *@This(), name: [:0]const u8) void { + fn addEntry(this: *@This(), name: [:0]const u8, dir_fd: bun.FileDescriptor) void { const skip = this.shouldSkipEntry(name); debug("Entry: (skip={}) {s} :: {s}", .{ skip, this.path, name }); if (skip) return; - bun.handleOom(this.output.ensureUnusedCapacity(name.len + 1)); - bun.handleOom(this.output.appendSlice(name)); - bun.handleOom(this.output.append('\n')); + + if (this.opts.long_listing) { + this.addEntryLong(name, dir_fd); + } else { + bun.handleOom(this.output.ensureUnusedCapacity(name.len + 1)); + bun.handleOom(this.output.appendSlice(name)); + bun.handleOom(this.output.append('\n')); + } } - fn addDotEntriesIfNeeded(this: *@This()) void { + fn addEntryLong(this: *@This(), name: [:0]const u8, dir_fd: bun.FileDescriptor) void { + // Use lstatat to not follow symlinks (so symlinks show as 'l' type) + const stat_result = Syscall.lstatat(dir_fd, name); + const stat = switch (stat_result) { + .err => { + // If stat fails, just output the name with placeholders + const writer = this.output.writer(); + bun.handleOom(writer.print("?????????? ? ? ? ? ? {s}\n", .{name})); + return; + }, + .result => |s| s, + }; + + const writer = this.output.writer(); + + // File type and permissions + const mode: u32 = @intCast(stat.mode); + const file_type = getFileTypeChar(mode); + const perms = formatPermissions(mode); + + // Number of hard links + const nlink: u64 = @intCast(stat.nlink); + + // Owner and group (numeric) + const uid: u64 = @intCast(stat.uid); + const gid: u64 = @intCast(stat.gid); + + // File size + const size: i64 = @intCast(stat.size); + + // Modification time + const mtime = stat.mtime(); + const time_str = formatTime(@intCast(mtime.sec), this.#now_secs); + + bun.handleOom(writer.print("{c}{s} {d: >3} {d: >5} {d: >5} {d: >8} {s} {s}\n", .{ + file_type, + &perms, + nlink, + uid, + gid, + size, + &time_str, + name, + })); + } + + fn getFileTypeChar(mode: u32) u8 { + const file_type = mode & bun.S.IFMT; + return switch (file_type) { + bun.S.IFDIR => 'd', + bun.S.IFLNK => 'l', + bun.S.IFBLK => 'b', + bun.S.IFCHR => 'c', + bun.S.IFIFO => 'p', + bun.S.IFSOCK => 's', + else => '-', // IFREG or unknown + }; + } + + fn formatPermissions(mode: u32) [9]u8 { + var perms: [9]u8 = undefined; + // Owner permissions + perms[0] = if (mode & bun.S.IRUSR != 0) 'r' else '-'; + perms[1] = if (mode & bun.S.IWUSR != 0) 'w' else '-'; + // Owner execute with setuid handling + const owner_exec = mode & bun.S.IXUSR != 0; + const setuid = mode & bun.S.ISUID != 0; + perms[2] = if (setuid) + (if (owner_exec) 's' else 'S') + else + (if (owner_exec) 'x' else '-'); + + // Group permissions + perms[3] = if (mode & bun.S.IRGRP != 0) 'r' else '-'; + perms[4] = if (mode & bun.S.IWGRP != 0) 'w' else '-'; + // Group execute with setgid handling + const group_exec = mode & bun.S.IXGRP != 0; + const setgid = mode & bun.S.ISGID != 0; + perms[5] = if (setgid) + (if (group_exec) 's' else 'S') + else + (if (group_exec) 'x' else '-'); + + // Other permissions + perms[6] = if (mode & bun.S.IROTH != 0) 'r' else '-'; + perms[7] = if (mode & bun.S.IWOTH != 0) 'w' else '-'; + // Other execute with sticky bit handling + const other_exec = mode & bun.S.IXOTH != 0; + const sticky = mode & bun.S.ISVTX != 0; + perms[8] = if (sticky) + (if (other_exec) 't' else 'T') + else + (if (other_exec) 'x' else '-'); + + return perms; + } + + fn formatTime(timestamp: i64, now_secs: u64) [12]u8 { + var buf: [12]u8 = undefined; + // Format as "Mon DD HH:MM" for recent files (within 6 months) + // or "Mon DD YYYY" for older files + const epoch_secs: u64 = if (timestamp < 0) 0 else @intCast(timestamp); + const epoch = std.time.epoch.EpochSeconds{ .secs = epoch_secs }; + const day_seconds = epoch.getDaySeconds(); + const year_day = epoch.getEpochDay().calculateYearDay(); + + const month_names = [_][]const u8{ "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec" }; + const month_day = year_day.calculateMonthDay(); + const month_name = month_names[month_day.month.numeric() - 1]; + + // Check if file is older than 6 months (approximately 180 days) + const six_months_secs: u64 = 180 * 24 * 60 * 60; + const is_recent = epoch_secs > now_secs -| six_months_secs and epoch_secs <= now_secs + six_months_secs; + + if (is_recent) { + const hours = day_seconds.getHoursIntoDay(); + const minutes = day_seconds.getMinutesIntoHour(); + + _ = std.fmt.bufPrint(&buf, "{s} {d:0>2} {d:0>2}:{d:0>2}", .{ + month_name, + month_day.day_index + 1, + hours, + minutes, + }) catch { + @memcpy(&buf, "??? ?? ??:??"); + }; + } else { + // Show year for old files + const year = year_day.year; + + _ = std.fmt.bufPrint(&buf, "{s} {d:0>2} {d:4}", .{ + month_name, + month_day.day_index + 1, + year, + }) catch { + @memcpy(&buf, "??? ?? ????"); + }; + } + + return buf; + } + + fn addDotEntriesIfNeeded(this: *@This(), dir_fd: bun.FileDescriptor) void { // `.addEntry()` already checks will check if we can add "." and ".." to // the result - this.addEntry("."); - this.addEntry(".."); + this.addEntry(".", dir_fd); + this.addEntry("..", dir_fd); } fn errorWithPath(this: *@This(), err: Syscall.Error, path: [:0]const u8) Syscall.Error { diff --git a/src/sys.zig b/src/sys.zig index 0c0d23f3de..0dd4ce8392 100644 --- a/src/sys.zig +++ b/src/sys.zig @@ -744,6 +744,28 @@ pub fn fstatat(fd: bun.FileDescriptor, path: [:0]const u8) Maybe(bun.Stat) { return Maybe(bun.Stat){ .result = stat_buf }; } +/// Like fstatat but does not follow symlinks (uses AT_SYMLINK_NOFOLLOW) +pub fn lstatat(fd: bun.FileDescriptor, path: [:0]const u8) Maybe(bun.Stat) { + if (Environment.isWindows) { + // On Windows, use O.NOFOLLOW to get lstat behavior (prevents following symlinks) + return switch (openatWindowsA(fd, path, O.NOFOLLOW, 0)) { + .result => |file| { + defer file.close(); + return fstat(file); + }, + .err => |err| Maybe(bun.Stat){ .err = err }, + }; + } + var stat_buf = mem.zeroes(bun.Stat); + const fd_valid = if (fd == bun.invalid_fd) std.posix.AT.FDCWD else fd.native(); + if (Maybe(bun.Stat).errnoSysFP(syscall.fstatat(fd_valid, path, &stat_buf, std.posix.AT.SYMLINK_NOFOLLOW), .fstatat, fd, path)) |err| { + log("lstatat({f}, {s}) = {s}", .{ fd, path, @tagName(err.getErrno()) }); + return err; + } + log("lstatat({f}, {s}) = 0", .{ fd, path }); + return Maybe(bun.Stat){ .result = stat_buf }; +} + pub fn mkdir(file_path: [:0]const u8, flags: mode_t) Maybe(void) { return switch (Environment.os) { .mac => Maybe(void).errnoSysP(syscall.mkdir(file_path, flags), .mkdir, file_path) orelse .success, diff --git a/test/regression/issue/25831.test.ts b/test/regression/issue/25831.test.ts new file mode 100644 index 0000000000..aa4849b7ab --- /dev/null +++ b/test/regression/issue/25831.test.ts @@ -0,0 +1,169 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +test("ls -l shows long listing format", async () => { + // Create temp directory with test files + using dir = tempDir("ls-long-listing", { + "file.txt": "hello world", + "script.sh": "#!/bin/bash\necho hello", + subdir: { + "nested.txt": "nested content", + }, + }); + + // Run ls -l in the temp directory + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import { $ } from "bun"; + $.cwd("${String(dir).replace(/\\/g, "\\\\")}"); + const result = await $\`ls -l\`.text(); + console.log(result); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + // Verify no errors on stderr + expect(stderr).toBe(""); + + // Should show permission string (starts with - or d, followed by rwx/sStT permissions) + // Format: -rw-r--r-- 1 uid gid size date name + expect(stdout).toMatch(/^[-dlbcps][-rwxsStT]{9}/m); // Permission string pattern + expect(stdout).toContain("file.txt"); + expect(stdout).toContain("script.sh"); + expect(stdout).toContain("subdir"); + + // Verify that it's actually showing long format (contains size and date info) + // Long format has at least permissions, link count, uid, gid, size, date, name + const lines = stdout + .trim() + .split("\n") + .filter(line => line.includes("file.txt")); + expect(lines.length).toBeGreaterThan(0); + + // Each line should have multiple space-separated fields + const fileLine = lines[0]; + const fields = fileLine.trim().split(/\s+/); + expect(fields.length).toBeGreaterThanOrEqual(7); // perms, nlink, uid, gid, size, date fields, name + + expect(exitCode).toBe(0); +}); + +test("ls without -l shows short format", async () => { + using dir = tempDir("ls-short-listing", { + "file1.txt": "content1", + "file2.txt": "content2", + }); + + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import { $ } from "bun"; + $.cwd("${String(dir).replace(/\\/g, "\\\\")}"); + const result = await $\`ls\`.text(); + console.log(result); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + // Verify no errors on stderr + expect(stderr).toBe(""); + + // Short format should just show filenames, not permission strings + expect(stdout).not.toMatch(/^[-dlbcps][-rwxsStT]{9}/m); + expect(stdout).toContain("file1.txt"); + expect(stdout).toContain("file2.txt"); + + expect(exitCode).toBe(0); +}); + +test("ls -al shows hidden files in long format", async () => { + using dir = tempDir("ls-all-long", { + ".hidden": "hidden content", + "visible.txt": "visible content", + }); + + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import { $ } from "bun"; + $.cwd("${String(dir).replace(/\\/g, "\\\\")}"); + const result = await $\`ls -al\`.text(); + console.log(result); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + // Verify no errors on stderr + expect(stderr).toBe(""); + + // Should show hidden files + expect(stdout).toContain(".hidden"); + expect(stdout).toContain("visible.txt"); + // Should also show . and .. entries + expect(stdout).toMatch(/^d[-rwxsStT]{9}.*\s\.$/m); // . directory + expect(stdout).toMatch(/^d[-rwxsStT]{9}.*\s\.\.$/m); // .. directory + + // Should be in long format + expect(stdout).toMatch(/^[-dlbcps][-rwxsStT]{9}/m); + + expect(exitCode).toBe(0); +}); + +test("ls -l shows directory type indicator", async () => { + using dir = tempDir("ls-dir-type", { + "regular-file.txt": "content", + subdir: { + "nested.txt": "nested", + }, + }); + + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import { $ } from "bun"; + $.cwd("${String(dir).replace(/\\/g, "\\\\")}"); + const result = await $\`ls -l\`.text(); + console.log(result); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + // Verify no errors on stderr + expect(stderr).toBe(""); + + // Directory should start with 'd' + expect(stdout).toMatch(/^d[-rwxsStT]{9}.*subdir$/m); + // Regular file should start with '-' + expect(stdout).toMatch(/^-[-rwxsStT]{9}.*regular-file\.txt$/m); + + expect(exitCode).toBe(0); +});