Skip to content

Use FilePath for content store client/server pair#28

Open
chrisgeo wants to merge 1 commit into
mainfrom
feat/chaos-1455-content-store-filepath
Open

Use FilePath for content store client/server pair#28
chrisgeo wants to merge 1 commit into
mainfrom
feat/chaos-1455-content-store-filepath

Conversation

@chrisgeo

@chrisgeo chrisgeo commented May 9, 2026

Copy link
Copy Markdown

Summary

  • Converts ContentStoreService (server) and RemoteContentStoreClient (client) from URL to SystemPackage.FilePath across the content-store XPC pair.
  • Public API delta on the server actor:
    • ContentStoreService.init(root: URL, …)init(root: FilePath, …)
    • get(digest:) async throws -> URL?FilePath?
    • newIngestSession() async throws -> (id: String, ingestDir: URL)(id: String, ingestDir: FilePath)
  • RPC contract co-updated in lockstep: ContentServiceHarness now serializes paths via FilePath.string (replacing URL.path(percentEncoded: false)); the wire format on XPCMessageKey.contentPath and .directory is unchanged (still a plain absolute path string), so client and server stay compatible.
  • Single construction site for ContentStoreService updated at Sources/Plugins/CoreImages/ImagesHelper.swift:116 — passes FilePath(root.path(percentEncoded: false)) since appRoot upstream is still URL.
  • Client RemoteContentStoreClient: private _get(digest:) is now FilePath-typed and decodes XPC strings via FilePath(path) instead of URL(filePath: path). The public protocol surface for ContentStore (i.e. ingest(_ body: @Sendable @escaping (URL) async throws -> Void) and newIngestSession() -> (id, URL)) is pinned by the upstream ContainerizationOCI.ContentStore protocol and stays URL — see boundary notes below.
  • No Codable wire-format change. ContentStoreService is an actor with non-serialized stored properties; XPC payloads remain plain path strings.

URL boundaries preserved (with inline comments justifying each leak):

  • ContentStoreService.initLocalContentStore(path: URL(filePath: self.root.string)) — the upstream ContainerizationOCI.LocalContentStore constructor only accepts URL.
  • RemoteContentStoreClient.get(digest:)LocalContent(path: URL(filePath: path.string)) — the upstream ContainerizationOCI.LocalContent constructor only accepts URL.
  • ContentStoreService.get(digest:) and .newIngestSession() convert URLs returned by LocalContentStore back into FilePath via FilePath(url.path(percentEncoded: false)) at the Containerization library boundary.
  • RemoteContentStoreClient.newIngestSession() and .ingest(_:) keep their URL signatures unchanged because they are required by the external ContentStore protocol; changing them would break protocol conformance. Same precedent as PR Use FilePath for SnapshotStore #25's directorySize / URLResourceKey leak.

Closes CHAOS-1455. Sibling of #26 (Bundle), part of CHAOS-1448 epic.

Type of Change

  • Refactor / tech debt (URL → FilePath migration)

Testing

  • `swift build` succeeds — clean compile, no warnings (post-rebase verified at SHA 74395be).
  • `swift test --filter ContainerImagesService` — no failures (0 tests matched; module has no dedicated unit tests today, same as PR Use FilePath for SnapshotStore #25).
  • lsp_diagnostics clean on all four touched files.
  • Manual: `container image pull` exercises `ContentStoreService.newIngestSession` and `RemoteContentStoreClient.ingest` end-to-end; XPC-string round-trip unchanged because wire format is the same plain path string.
  • Manual: `container image rm` / `container system prune` exercises `delete(digests:)` / `delete(keeping:)` paths (untouched by this PR).

@linear

linear Bot commented May 9, 2026

Copy link
Copy Markdown

CHAOS-1455

CHAOS-1448

@chrisgeo chrisgeo force-pushed the feat/chaos-1455-content-store-filepath branch 4 times, most recently from bff5801 to 116bcbf Compare May 14, 2026 17:55
@chrisgeo chrisgeo force-pushed the feat/chaos-1455-content-store-filepath branch from 116bcbf to a926e39 Compare May 23, 2026 19:11
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.

1 participant