Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ Follow the [plugin development standards](docs/plugins/development-guidelines.md
- **Keep background work economical.** Use cached snapshots, event-driven updates, bounded asynchronous work, and visibility-aware presentation. Preserve intentional monitoring while hidden; stop owned work on deactivation. See [performance requirements](docs/plugins/development-guidelines.md#performance-and-energy).
- **Preserve user control.** Handle denied permissions, cancellation, unsupported hardware, and system changes. Keep existing confirmations, recovery paths, and destructive-operation safeguards.

Shared filesystem metadata code lives in `Sources/MacToolsFileSystem`. Disk Clean and Storage Explorer link this static module into their bundles; their core targets use it as a build dependency. Keep cleanup policy in the owning plugin and run both plugins' filesystem tests after changing the shared parser.
Shared filesystem metadata code lives in `Sources/MacToolsFileSystem`. Disk Clean, Storage Explorer, and Xcode Clean link this static module into their bundles; their core targets use it as a build dependency. Use `FileSystemDirectoryReader.readBatches` to bound enumeration memory for wide folders. Keep cleanup policy in the owning plugin and run the affected plugins' filesystem tests after changing the shared reader or parser.

## Validation

Expand Down
75 changes: 75 additions & 0 deletions Plugins/DiskClean/Sources/DiskCleanDiscoveryEntrySource.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import Darwin

/// Bulk discovery with a streaming fallback before the first batch is delivered.
struct DiskCleanDiscoveryEntrySourceFactory: DiskCleanDirectoryEntrySourceFactory {
private let bulkSourceFactory: any DiskCleanDirectoryEntrySourceFactory
private let streamSourceFactory: any DiskCleanDirectoryEntrySourceFactory

init(
bulkSourceFactory: any DiskCleanDirectoryEntrySourceFactory = DiskCleanBulkEntrySourceFactory(),
streamSourceFactory: any DiskCleanDirectoryEntrySourceFactory = DiskCleanDirectoryStreamEntrySourceFactory()
) {
self.bulkSourceFactory = bulkSourceFactory
self.streamSourceFactory = streamSourceFactory
}

func makeSource(fileDescriptor: Int32) throws -> any DiskCleanDirectoryEntrySource {
DiskCleanDiscoveryEntrySource(
source: try bulkSourceFactory.makeSource(fileDescriptor: fileDescriptor),
streamSourceFactory: streamSourceFactory
)
}
}

private final class DiskCleanDiscoveryEntrySource: DiskCleanDirectoryEntrySource {
private var source: any DiskCleanDirectoryEntrySource
private let streamSourceFactory: any DiskCleanDirectoryEntrySourceFactory
private var canFallBack = true
private var isClosed = false

var directoryFileDescriptor: Int32 { source.directoryFileDescriptor }

init(
source: any DiskCleanDirectoryEntrySource,
streamSourceFactory: any DiskCleanDirectoryEntrySourceFactory
) {
self.source = source
self.streamSourceFactory = streamSourceFactory
}

deinit { close() }

func nextBatch() throws -> [DiskCleanWalkEntry]? {
guard !isClosed else { return nil }
do {
let batch = try source.nextBatch()
canFallBack = false
return batch
} catch let error as DiskCleanPOSIXError
where canFallBack && (error.code == ENOTSUP || error.code == EINVAL || error.code == ENOSYS) {
canFallBack = false
// Open a new description at offset zero, anchored to the original directory.
// Reopening its path could follow a replacement directory or symbolic link.
let descriptor = openat(
source.directoryFileDescriptor, ".", O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_NONBLOCK
)
guard descriptor >= 0 else { throw DiskCleanPOSIXError(code: errno) }
let fallback: any DiskCleanDirectoryEntrySource
do {
fallback = try streamSourceFactory.makeSource(fileDescriptor: descriptor)
} catch {
Darwin.close(descriptor)
throw error
}
source.close()
source = fallback
return try source.nextBatch()
}
}

func close() {
guard !isClosed else { return }
isClosed = true
source.close()
}
}
14 changes: 11 additions & 3 deletions Plugins/DiskClean/Sources/DiskCleanInstallerScanner.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ enum DiskCleanInstallerScanOutcome: Equatable, Sendable {
/// `~/Downloads` is **top-level only, no recursion**: subdirectories are usually user-organized
/// material, and recursing for `.dmg` would surface already-archived content.
///
/// Blocking, but only one top-level `readdir` plus one `fstatat` per entry; cost scales with
/// Blocking, but only top-level bulk metadata reads plus `fstatat` for matching installers; cost scales with
/// entry count and never descends, so WorkerPool abandon budgets are unnecessary—that machinery
/// is for recursive sizing that can hang forever.
struct DiskCleanInstallerScanner: Sendable {
Expand All @@ -121,7 +121,7 @@ struct DiskCleanInstallerScanner: Sendable {
),
staleAge: TimeInterval = defaultStaleAge,
opener: DiskCleanRootOpener = DiskCleanRootOpener(),
sourceFactory: any DiskCleanDirectoryEntrySourceFactory = DiskCleanDirectoryStreamEntrySourceFactory(),
sourceFactory: any DiskCleanDirectoryEntrySourceFactory = DiskCleanDiscoveryEntrySourceFactory(),
now: @escaping @Sendable () -> Date = { Date() }
) {
self.downloadsPath = downloadsPath
Expand Down Expand Up @@ -173,6 +173,12 @@ struct DiskCleanInstallerScanner: Sendable {
do {
while let batch = try source.nextBatch() {
for entry in batch {
if case let .unresolved(code) = entry {
// A vanished entry can be skipped; other failures may hide installers
// or signal a malformed bulk batch and must not look like success.
if code == ENOENT { continue }
throw DiskCleanPOSIXError(code: code)
}
guard case let .resolved(resolved) = entry else { continue }
// Regular files only: do not follow symlinks (would delete the link, not the installer),
// and do not recurse into directories.
Expand All @@ -196,7 +202,7 @@ struct DiskCleanInstallerScanner: Sendable {
}
}
} catch {
// Never treat mid-stream readdir failure as a successful partial scan.
// Never treat an enumeration failure as a successful partial scan.
let code = (error as? DiskCleanPOSIXError)?.code ?? EIO
return code == EPERM || code == EACCES
? .denied(path: directoryPath)
Expand Down Expand Up @@ -239,6 +245,8 @@ struct DiskCleanInstallerScanner: Sendable {
private static func status(name: String, directoryFileDescriptor: Int32) -> stat? {
var status = stat()
guard fstatat(directoryFileDescriptor, name, &status, AT_SYMLINK_NOFOLLOW) == 0 else { return nil }
// The entry may have changed since the bulk read. Only a current regular file is an installer.
guard DiskCleanRootIdentity.FileType(mode: status.st_mode) == .regularFile else { return nil }
return status
}

Expand Down
4 changes: 2 additions & 2 deletions Plugins/DiskClean/Sources/DiskCleanPurgeScanner.swift
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ struct DiskCleanPurgeScanResult: Equatable, Sendable {
/// Developer-artifact discovery walk (design §10.1). **Blocking**; called by `DiskCleanPurgeScanner`
/// on a background queue.
///
/// Uses fd-relative `readdir` (reuses SlowWalker's entry source) rather than `FileManager.enumerator`:
/// Uses fd-relative bulk metadata with a `readdir` fallback rather than `FileManager.enumerator`:
/// the latter has no no-follow or device constraints and will follow symlinks out of the scan root
/// and silently cross mount points. This is discovery only and deletes nothing, but produced paths
/// go straight to removal primitives, so they must be trustworthy physical paths.
Expand All @@ -208,7 +208,7 @@ struct DiskCleanPurgeDiscovery: Sendable {
init(
maximumDepth: Int = defaultMaximumDepth,
opener: DiskCleanRootOpener = DiskCleanRootOpener(),
sourceFactory: any DiskCleanDirectoryEntrySourceFactory = DiskCleanDirectoryStreamEntrySourceFactory()
sourceFactory: any DiskCleanDirectoryEntrySourceFactory = DiskCleanDiscoveryEntrySourceFactory()
) {
self.maximumDepth = max(maximumDepth, 1)
self.opener = opener
Expand Down
87 changes: 87 additions & 0 deletions Plugins/DiskClean/Tests/DiskCleanDiscoveryEntrySourceTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import Darwin
import Foundation
import XCTest
@testable import DiskCleanPlugin

final class DiskCleanDiscoveryEntrySourceTests: XCTestCase {
func testUnsupportedBulkReadFallsBackToTheOriginalDirectory() throws {
let directory = try DiskCleanTempDirectory(name: "DiskCleanDiscoveryEntrySourceTests")
defer { directory.remove() }
try directory.makeFile("Outside/foreign.dmg", bytes: 20)
for code in [ENOTSUP, EINVAL, ENOSYS] {
let root = "Root-\(code)"
try directory.makeFile("\(root)/original.dmg", bytes: 10)
try directory.makeSymlink("\(root)/alias.dmg", destination: "original.dmg")
guard case let .directory(descriptor, _) = DiskCleanRootOpener().open(path: directory.resolve(root).path) else {
return XCTFail("expected directory")
}
let source = try DiskCleanDiscoveryEntrySourceFactory(
bulkSourceFactory: ScriptedDiscoverySourceFactory(steps: [.failure(DiskCleanPOSIXError(code: code))])
).makeSource(fileDescriptor: descriptor)
defer { source.close() }
try FileManager.default.moveItem(at: directory.resolve(root), to: directory.resolve("Moved-\(code)"))
try directory.makeSymlink(root, destination: "Outside")

let entries = try XCTUnwrap(source.nextBatch())

let resolved = entries.compactMap { entry -> DiskCleanResolvedEntry? in
guard case let .resolved(value) = entry else { return nil }
return value
}
XCTAssertEqual(Set(resolved.map { String(cString: $0.nameBytes) }), ["original.dmg", "alias.dmg"])
XCTAssertEqual(resolved.sorted { $0.nameBytes.lexicographicallyPrecedes($1.nameBytes) }.map(\.fileType), [.symlink, .regularFile])
XCTAssertNil(try source.nextBatch())
}
}

func testFailureAfterDeliveredBatchDoesNotRestartEnumeration() throws {
let directory = try DiskCleanTempDirectory(name: "DiskCleanDiscoveryEntrySourceTests")
defer { directory.remove() }
let root = try directory.makeDirectory("Root")
guard case let .directory(descriptor, _) = DiskCleanRootOpener().open(path: root.path) else {
return XCTFail("expected directory")
}
let delivered: [DiskCleanWalkEntry] = [.unresolved(code: ENOENT)]
let source = try DiskCleanDiscoveryEntrySourceFactory(
bulkSourceFactory: ScriptedDiscoverySourceFactory(steps: [
.success(delivered), .failure(DiskCleanPOSIXError(code: ENOTSUP))
])
).makeSource(fileDescriptor: descriptor)
defer { source.close() }

XCTAssertEqual(try source.nextBatch(), delivered)
XCTAssertThrowsError(try source.nextBatch()) { error in
XCTAssertEqual(error as? DiskCleanPOSIXError, DiskCleanPOSIXError(code: ENOTSUP))
}
}
}

private struct ScriptedDiscoverySourceFactory: DiskCleanDirectoryEntrySourceFactory {
let steps: [Result<[DiskCleanWalkEntry]?, DiskCleanPOSIXError>]

func makeSource(fileDescriptor: Int32) throws -> any DiskCleanDirectoryEntrySource {
ScriptedDiscoverySource(fileDescriptor: fileDescriptor, steps: steps)
}
}

private final class ScriptedDiscoverySource: DiskCleanDirectoryEntrySource {
let directoryFileDescriptor: Int32
private var steps: [Result<[DiskCleanWalkEntry]?, DiskCleanPOSIXError>]
private var isClosed = false

init(fileDescriptor: Int32, steps: [Result<[DiskCleanWalkEntry]?, DiskCleanPOSIXError>]) {
self.directoryFileDescriptor = fileDescriptor
self.steps = steps
}

func nextBatch() throws -> [DiskCleanWalkEntry]? {
guard !steps.isEmpty else { return nil }
return try steps.removeFirst().get()
}

func close() {
guard !isClosed else { return }
isClosed = true
Darwin.close(directoryFileDescriptor)
}
}
57 changes: 47 additions & 10 deletions Plugins/DiskClean/Tests/DiskCleanInstallerScannerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ final class DiskCleanInstallerScannerTests: XCTestCase {
let scanner = DiskCleanInstallerScanner(
downloadsPath: downloads,
staleAge: DiskCleanInstallerScanner.defaultStaleAge,
sourceFactory: ThrowingInstallerSourceFactory(code: EIO),
sourceFactory: DiskCleanDiscoveryEntrySourceFactory(
bulkSourceFactory: ScriptedInstallerSourceFactory(code: EIO)
),
now: { observationDate }
)

Expand All @@ -140,7 +142,9 @@ final class DiskCleanInstallerScannerTests: XCTestCase {
let scanner = DiskCleanInstallerScanner(
downloadsPath: downloads,
staleAge: DiskCleanInstallerScanner.defaultStaleAge,
sourceFactory: ThrowingInstallerSourceFactory(code: EACCES),
sourceFactory: DiskCleanDiscoveryEntrySourceFactory(
bulkSourceFactory: ScriptedInstallerSourceFactory(code: EACCES)
),
now: { observationDate }
)

Expand All @@ -152,6 +156,32 @@ final class DiskCleanInstallerScannerTests: XCTestCase {
XCTAssertEqual(path, downloads)
}

func testUnresolvedMetadataDoesNotBecomeASuccessfulScan() {
let downloads = temporaryDirectory.resolve("Downloads").path
let scanner = DiskCleanInstallerScanner(
downloadsPath: downloads,
sourceFactory: ScriptedInstallerSourceFactory(batch: [.unresolved(code: EIO)])
)

XCTAssertEqual(scanner.scan(), .unavailable(path: downloads, reason: .walkError))
}

func testRejectsInstallerWhenCurrentMetadataIsASymlink() throws {
try makeDownload("real.dmg", bytes: 8, ageDays: 100)
try temporaryDirectory.makeSymlink("Downloads/alias.dmg", destination: "real.dmg")
// A bulk observation can precede replacement of the entry by a symbolic link.
let staleEntry = DiskCleanResolvedEntry(
nameBytes: Array("alias.dmg".utf8).map { CChar(bitPattern: $0) } + [0],
fileType: .regularFile, devid: 0, fileID: 0, linkCount: 1, dataLength: 8
)
let scanner = DiskCleanInstallerScanner(
downloadsPath: temporaryDirectory.resolve("Downloads").path,
sourceFactory: ScriptedInstallerSourceFactory(batch: [.resolved(staleEntry)])
)

XCTAssertEqual(scanner.scan(), .scanned(candidates: []))
}

// MARK: - Fixtures

private func makeScanner(
Expand Down Expand Up @@ -190,27 +220,34 @@ final class DiskCleanInstallerScannerTests: XCTestCase {
}
}

/// Source factory that owns the fd and fails the first `nextBatch`, simulating mid-stream readdir error.
private struct ThrowingInstallerSourceFactory: DiskCleanDirectoryEntrySourceFactory {
let code: Int32
private struct ScriptedInstallerSourceFactory: DiskCleanDirectoryEntrySourceFactory {
var code: Int32? = nil
var batch: [DiskCleanWalkEntry]? = nil

func makeSource(fileDescriptor: Int32) throws -> any DiskCleanDirectoryEntrySource {
ThrowingInstallerSource(fileDescriptor: fileDescriptor, code: code)
ScriptedInstallerSource(fileDescriptor: fileDescriptor, code: code, batch: batch)
}
}

private final class ThrowingInstallerSource: DiskCleanDirectoryEntrySource {
private final class ScriptedInstallerSource: DiskCleanDirectoryEntrySource {
let directoryFileDescriptor: Int32
private let code: Int32
private let code: Int32?
private var batch: [DiskCleanWalkEntry]?
private var isClosed = false

init(fileDescriptor: Int32, code: Int32) {
init(fileDescriptor: Int32, code: Int32?, batch: [DiskCleanWalkEntry]?) {
self.directoryFileDescriptor = fileDescriptor
self.code = code
self.batch = batch
}

func nextBatch() throws -> [DiskCleanWalkEntry]? {
throw DiskCleanPOSIXError(code: code)
if let batch {
self.batch = nil
return batch
}
if let code { throw DiskCleanPOSIXError(code: code) }
return nil
}

func close() {
Expand Down
15 changes: 15 additions & 0 deletions Plugins/DiskClean/Tests/DiskCleanPurgeScannerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,21 @@ final class DiskCleanPurgeScannerTests: XCTestCase {
XCTAssertTrue(report.items.isEmpty)
}

func testMetadataFailureKeepsDiscoveredItemsButMarksScanIncomplete() throws {
let root = try temporaryDirectory.makeDirectory("root")
try temporaryDirectory.makeFile("root/package.json", bytes: 10)
try temporaryDirectory.makeDirectory("root/node_modules")
let factory = ScriptedPurgeSourceFactory(scripts: [[[
.resolved(entry(name: "node_modules", type: .directory, devid: try deviceID(of: root.path), fileID: 77)),
.unresolved(code: EIO)
]]])

let report = DiskCleanPurgeDiscovery(sourceFactory: factory).discover(root: root.path)

XCTAssertEqual(report.items.map(\.path), [path("root/node_modules")])
XCTAssertEqual(report.status, .traversed(completeness: .partial(reasons: [.walkError])))
}

// MARK: - Repository attribution

/// Worktree/submodule `.git` is a file, not a directory, and still counts as a repository.
Expand Down
Loading
Loading