Skip to content

Use SystemPath for DirectoryWatcher.#1522

Merged
jglogan merged 2 commits into
apple:mainfrom
full-chaos:feat/chaos-1450-directory-watcher
May 11, 2026
Merged

Use SystemPath for DirectoryWatcher.#1522
jglogan merged 2 commits into
apple:mainfrom
full-chaos:feat/chaos-1450-directory-watcher

Conversation

@chrisgeo
Copy link
Copy Markdown
Contributor

@chrisgeo chrisgeo commented May 8, 2026

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Motivation and Context

Continues the URL → FilePath migration discussed in #1481.

Changes in this PR:

  • directoryURL: URLdirectoryPath: FilePath (public API)
  • init(directoryURL:, log:)init(directoryPath:, log:)
  • startWatching handler signature: ([URL]) throws -> Void([FilePath]) throws -> Void
  • Sources/APIServer/LocalhostDNSHandler.swift updated to call the new init directly (the previous URL(filePath: configPath.string) wrapping shim is no longer needed)

Behavioral note: URL.lastPathComponent returns String; FilePath.lastComponent returns FilePath.Component?. The filename filter in LocalhostDNSHandler handles the optional via $0.lastComponent?.string.starts(with: ...) == true, preserving the prior "skip malformed entries" behavior.

Testing

  • Tested locally (swift test --filter DirectoryWatcherTest and --filter ContainerOSTests both pass)
  • Added/updated tests (test fixtures updated to construct DirectoryWatcher with FilePath)
  • Added/updated docs

Replaces URL-based filesystem path handling with SystemPackage.FilePath,
following the pattern from apple#1480 (HostDNSResolver) and apple#1518 (PacketFilter):

- directoryURL → directoryPath: FilePath (public API)
- startWatching handler signature changes from ([URL]) to ([FilePath])
- LocalhostDNSHandler caller updated; the URL(filePath: configPath.string)
  wrapping shim is no longer needed
- FilePath.lastComponent (returns Optional<Component>) handled in
  LocalhostDNSHandler's filename filter
@chrisgeo chrisgeo force-pushed the feat/chaos-1450-directory-watcher branch from 849d310 to 4c1477d Compare May 8, 2026 22:15
Copy link
Copy Markdown
Contributor

@jglogan jglogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building now! One comment, just a nit.

let ipv4 = (match[1].substring.flatMap { try? IPv4Address(String($0)) })
{
let name = String(file.lastPathComponent.dropFirst(HostDNSResolver.containerizationPrefix.count))
let lastName = file.lastComponent?.string ?? ""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: instead of nil to empty string here, perhaps this?

let dnsName = file.lastComponent?.string
    .map { $0.dropFirst(HostDNSResolver.containerizationPrefix.count) }
    .map { try? DNSName($0) }
guard let dnsName else {
    continue
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch, thank you! I'm still learning some idioms in Swift. I switched to using the guard from your example and tests pass locally.

 guard let lastName = file.lastComponent?.string else {
     continue
  }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is also idiomatic and easy to read, thanks!

@chrisgeo chrisgeo force-pushed the feat/chaos-1450-directory-watcher branch from 9e19473 to 4c1477d Compare May 9, 2026 23:16
@jglogan jglogan merged commit 509fa04 into apple:main May 11, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Request]: TECH DEBT - Convert DirectoryWatcher to use FilePath instead of URL.

3 participants