diff --git a/work/projects/vscode-migration/findings.md b/work/projects/vscode-migration/findings.md index 9843d722..99d59969 100755 --- a/work/projects/vscode-migration/findings.md +++ b/work/projects/vscode-migration/findings.md @@ -7,9 +7,33 @@ Back to [[projects/vscode-migration/vscode-migration]]. Verified on Xcode 26.6 / - DMG and App Store targets compile identical sources (1398 files); `FEATURE_FLAGS` is empty for the app — only `DEBUG` remains (SPM handles it). - No ObjC/C in `macOS/DuckDuckGo/`, empty bridging header. - Unit Tests compile only 2 app files directly; the rest is `@testable import` → can switch to the library. -- 46 app files are shared with helper targets (VPN 36, DBP agent 18, NetP extensions 14 each): `UserText.swift`, `UserDefaultsWrapper`, `ConfigurationManager`, `Subscription*`, `TipKit*`, `VPN*`, `DuckDuckGoUserAgent`, … +- 38 app files (by path; an earlier name-based count said 46) were shared with helper targets. After the membership trim 27 remained; after stages A+B 18. Progress per stage: [[projects/vscode-migration/vscode-migration#Target split stages]]. - Remaining Xcode-only resources: storyboards `PasswordManager`, `BookmarksBar`, `NavigationBar`, `Feedback` + `BookmarksBarCollectionViewItem.xib`; 8 `.xcdatamodeld` in the app, 3 in BSK. +## Target split (files shared with helper targets) + +- Find out which target really needs a file by removing it from the target and building, not by grepping: 15 of 38 memberships turned out unused (some only transitively needed, e.g. TipKit, Safari/WebKit version readers). +- DMG and App Store variants must have identical sources: `InputFilesChecker` fails the build otherwise, so remove a file from both. +- `InputFilesChecker.temporarilyAllowedMisplacedFiles` lists the files still outside their owning target, one block per stage; stale entries fail the build. +- Local package products are linked as `XCSwiftPackageProductDependency` without a `package` key. Unit Tests don't link `NetworkProtectionMac` products; modules are visible through the host app. +- Transitive module imports work in Xcode (e.g. `Subscription` inside `DataBrokerProtection-macOS` without a direct product). `DataBrokerProtection-macOS` relies on this, so moved files follow the same convention. +- `UserText` exists twice (app and VPN agent, `DuckDuckGoVPN/UserText.swift`); shared files extend whichever one the target compiles. Moving them into a package means moving the strings (and translations) to the package's `Localizable.xcstrings`. +- `UserDefaultsWrapper` is `@deprecated` on main ("Use KeyedStoring pattern") — don't move it, follow that migration. +- `SANDBOX_TEST_TOOL` / `ALPHA` were dead: packages never receive target `FEATURE_FLAGS`, and sandbox-test-tool compiled only its own file. The system extension always has `AppVersion.runType == .normal`. +- `NETP_SYSTEM_EXTENSION` remains only in extension-only code (keychain service names, defaults, pixel source). + +## Build verification (offline) + +- Build with `-project macOS/DuckDuckGo-macOS.xcodeproj` (as CI does), not the root workspace: on main (late Sep 2026) the root `DuckDuckGo.xcworkspace` `Package.resolved` lacked `swift-http-structured-headers` (added by iOS `SitePermissions`), so workspace builds with `-disableAutomaticPackageResolution` fail. +- Offline builds: copy `SourcePackages` from a DerivedData whose checkouts contain all pinned revisions, then `-disableAutomaticPackageResolution -skipPackageUpdates -onlyUsePackageVersionsFromResolvedFile -skipPackagePluginValidation -skipMacroValidation CODE_SIGNING_ALLOWED=NO`. +- Schemes covering all helper targets: `macOS Browser`, `macOS Browser App Store`, `DuckDuckGoVPNSysexAppStore`, `macOS VPN Proxy Extension`, `macOS VPN System Extension`, `macOS VPN Agent App Store`, `macOS PIR Agent`, plus `build-for-testing` for `macOS Unit Tests` / `macOS Integration Tests`. +- Run SwiftLint from `macOS/` so the project `.swiftlint.yml` applies; running it from subfolders uses defaults and reports noise. + +## App Store UI tests (sandbox) + +- `AppStoreSandboxUITests` replaced sandbox-test-tool ([#6980](https://github.com/duckduckgo/apple-browsers/pull/6980)): custom downloads folder outside the auto-granted folders survives relaunch (security-scoped bookmark in `DownloadsPreferences`), and downloaded files stay accessible after relaunch (per-file bookmarks in `DownloadListStore`). +- The App Store schemes default to `UI Tests App Store Sandbox.xctestplan` (this class only); `UI Tests App Store.xctestplan` is used when CI filters by class name. CI runs the class along every full DMG run; failures create "GH Workflow Failure - App Store UI Tests". + ## Asset catalogs - Xcode generates asset symbols with `actool --bundle-identifier … --generate-swift-asset-symbols X.swift`. In that mode actool writes no `Assets.car`, so compiling is a second invocation (same as Xcode's `GenerateAssetSymbols` + `CompileAssetCatalog`). diff --git a/work/projects/vscode-migration/vscode-migration.md b/work/projects/vscode-migration/vscode-migration.md index f8d09a3f..90b9f91d 100755 --- a/work/projects/vscode-migration/vscode-migration.md +++ b/work/projects/vscode-migration/vscode-migration.md @@ -21,7 +21,10 @@ SPM-compatible, so `swift build` + SourceKit-LSP work, and configure | Initial VSCode config | ✅ Jun 2025 (stale: `sweetpad` worktree) | | Unify WebKit private methods (Sparkle/AppStore) | ✅ Feb 2026 | | Drop `#if SPARKLE/APPSTORE` | ✅ Mar 2026 — DMG and App Store targets now compile the same 1398 files | -| Drop storyboards | 🔄 4 storyboards + 1 xib left (`drop-sb-*` worktrees) | +| Drop dead target flags (`SANDBOX_TEST_TOOL`, `ALPHA`, `NETWORK_EXTENSION` in shared files) | ✅ [#6983](https://github.com/duckduckgo/apple-browsers/pull/6983) | +| Replace sandbox-test-tool with App Store UI tests (`AppStoreSandboxUITests`, CI leg along full DMG runs) | ✅ [#6980](https://github.com/duckduckgo/apple-browsers/pull/6980) | +| Split app files shared with helper targets | 🔄 A+B in review ([#7029](https://github.com/duckduckgo/apple-browsers/pull/7029)), C ready — see [Target split stages](#target-split-stages) | +| Drop storyboards | 🔄 3 left on main: `Feedback`, `NavigationBar`, `PasswordManager` (`drop-sb-*` worktrees) | | Core Data models in code | ❌ **Cancelled** 2026-10-01 — Xcode 27 builds these in SPM | | Asset catalogs for `swift build` | ❌ **Cancelled** 2026-10-01 — Xcode 27 builds these in SPM | | swift-build fork (Provisional Inputs) | ⏸ not started; may be unnecessary | @@ -30,29 +33,49 @@ SPM-compatible, so `swift build` + SourceKit-LSP work, and configure ## Open items after the cancellation -- [ ] PR [#6976](https://github.com/duckduckgo/apple-browsers/pull/6976) (AssetCatalogPlugin, reviewer ayoy): close, or keep until Xcode 27 is the minimum? Branch `alex/spm-asset-symbols-plugin`. -- [ ] Core Data branch `alex/coredata-code-models` (draft `PR.md`, Tech Design subtask): close. +- [x] Closed PR [#6976](https://github.com/duckduckgo/apple-browsers/pull/6976) (AssetCatalogPlugin) and the Core Data PR; their worktrees are removed. - [ ] Check what Xcode 27 actually does for `swift build` (native) and SourceKit-LSP: asset symbols, `.car`, `.momd`, storyboards? Earlier finding on Xcode 26: only `--build-system swiftbuild` handles assets, native doesn't. See [[projects/vscode-migration/findings#Asset catalogs]]. - [ ] Re-check whether storyboards still need dropping with Xcode 27. ## Plan (next steps) -1. **Extract files shared with helper targets into a package.** 46 app files are also compiled into VPN / DBP agent / NetP extensions, with `#if NETWORK_EXTENSION` / `NETP_SYSTEM_EXTENSION` / `SANDBOX_TEST_TOOL`. They block moving the app code into a package. Old draft: worktree `netp-dependencies` (`alex/align-netp-dependencies`, Oct 2025, far behind main) — redo on fresh main rather than merge. +1. **Extract files shared with helper targets into packages** — in progress, see [Target split stages](#target-split-stages). Started from 38 app files also compiled into VPN / PIR agent / NetP extensions. Dead flags are gone (#6983); unused memberships trimmed and stages A+B moved (#7029). The old draft `netp-dependencies` is superseded. 2. **Decide on strings & resources in the package** (tech design): 1964 `NSLocalizedString` without `bundle:` (→ `Bundle.main`), ~60 resource files, 8 `Bundle.main.url(…)`. `bundleIdentifier`/`appGroup` via `Bundle.main` can stay. 3. **App code as a library package** + thin app target (`AppMain`, Info.plist, entitlements, embed phases for login items / extensions). 4. **VSCode tasks with cached helper targets**: gate script hashes each extension's inputs (git-based, not mtime) and skips `xcodebuild` when unchanged; assemble step copies prebuilt extensions and signs. Only pays off once the app itself builds with `swift build`. See [[projects/vscode-migration/findings#Extensions in VSCode tasks]]. 5. Convert ~57 string-name image lookups (`NSImage(named:)`, `Image("…")`) to asset symbols — they break once code moves to `Bundle.module`. +## Target split stages + +Goal: no file in `macOS/DuckDuckGo/` is compiled into a helper target (VPN agent, PIR agent, NetP extensions), so the app directory can become one SPM target. `InputFilesChecker` tracks the remaining files in `temporarilyAllowedMisplacedFiles` (one block per stage); a stale entry fails the build, so each stage PR must remove its block. + +| Stage | Files | Used by | Home | New deps / project changes | Size & blockers | PR | +|---|---|---|---|---|---|---| +| A | `ApplicationBuildType`, `BuildFlags`, `DuckDuckGoUserAgent` | app + all 6 helpers | `AppKitExtensions` | none (all targets already link it) | 46 imports | [#7029](https://github.com/duckduckgo/apple-browsers/pull/7029) | +| B | `DBPFeatureFlagger`, `DataBrokerAuthenticationManagerBuilder`, `DataBrokerProtectionSettings+Environment`, `WebViewUserAgentProvider`, `SafariVersionReader`, `WebKitVersionProvider` | app + PIR agent | `DataBrokerProtection-macOS` | none (`Subscription` comes transitively, like elsewhere in the package) | 5 imports | [#7029](https://github.com/duckduckgo/apple-browsers/pull/7029) | +| C | `NetworkProtectionPixelEvent` (569 loc) | app, VPN agent, 3 extensions | `NetworkProtectionMac` → `VPNPixels` (new library product) | `VPNPixels` += `VPN`, `Configuration`; BSK package dep; product linked to 7 targets | 9 imports. Ready, uncommitted (`move-vpn-pixel-event`) | own PR, on top of #7029 | +| D | `SubscriptionPixel`, `SubscriptionPixelHandler`, `SubscriptionEnvironment+Default`, `WideEventFeatureFlagAdapter`, `SubscriptionFunnelOrigin` (836 loc) | app, VPN agent, PIR agent, 3 extensions | new local package (no existing one is linked by all consumers) | `Subscription`, `PixelKit`, `Networking`, `PrivacyConfig`, `WideEvent`, `AppKitExtensions`; product linked to ~8 targets | used in ~134 files — biggest diff | own PR (new package, Subscription reviewers) | +| E1 | TipKit ×4, `VPNIPCResources`, `VPNOperationErrorRecorder`, `NetworkProtectionVPNCountryLabelsModel`, `VPNSettings+Environment` (~470 loc) | app + VPN agent | `NetworkProtectionMac` (IPC / UI, or a small `TipKitSupport` target) | `Subscription` for `VPNSettings+Environment` | moderate, no strings | combine with G | +| G | 10 helper-only sources living in `DuckDuckGo/` (`Bundle+VPN`, `NetworkProtectionOptionKeyExtension`, `NetworkProtectionControllerErrorStore`, `NetworkProtectionTunnelController`, `MacPacketTunnelProvider`, `MacTransparentProxyProvider`, `NetworkProtectionNotificationsPresenterFactory`, `VPNFailureRecoveryPixel`, `Bundle+NetworkProtectionExtensions`, `SubscriptionManager+StandardConfiguration`) | helpers only, never the app | helper-owned folders | `git mv` + pbxproj group paths only | small, noisy pbxproj | combine with E1 | +| E2 | `UserText+NetworkProtection+Shared`, `DefaultVPNLocationFormatter` | app + VPN agent | `NetworkProtectionUI` (has `Localizable.xcstrings`) | strings → `Bundle.module` with translations; app and VPN agent each have their own `UserText` | blocked on the strings/resources decision | combine with H | +| H | 4 app icons, `macos-config.json`, 7 VPN Lottie JSONs, 2 NetP `.xcstrings` | app + helpers | package resources or per-target copies | resource bundles / `Bundle.module` | same decision as E2 and the app-package tech design | with E2 | +| F | `UserDefaultsWrapper` (415 loc) + `UserDefaults+NetworkProtectionShared` | app, VPN agent, all extensions; ~51 app files | don't move — deprecated in favour of `KeyedStoring` | follows the KeyedStoring migration | blocked on that migration (owner/timeline TBD) | after the migration | +| X (optional) | `NETP_SYSTEM_EXTENSION` in `MacPacketTunnelProvider`, `MacTransparentProxyProvider`, `NetworkProtectionNotificationsPresenterFactory` | extensions only | runtime check `Bundle.main.bundleURL.pathExtension == "systemextension"` | removes the last `FEATURE_FLAGS` from xcconfigs | changes keychain service / defaults selection — needs VPN team review | optional, separate | + +Order: C → D → E1+G → strings/resources tech design → E2+H → F after KeyedStoring. X any time if the VPN team agrees. + ## Worktrees | Worktree | Branch | What | |---|---|---| -| `spm-asset-symbols-plugin` | `alex/spm-asset-symbols-plugin` | AssetCatalogPlugin, PR #6976 | -| `coredata-code-models` | `alex/coredata-code-models` | Code-defined Core Data models (cancelled) | -| `coredata-code-models-bookmarks` | `alex/coredata-code-models-bookmarks` | Bookmarks models (not started, cancelled) | -| `drop-sb-*` (5) | `alex/drop-*-storyboard`, `…-xib` | Remaining storyboards | -| `netp-dependencies` | `alex/align-netp-dependencies` | Old draft: move files by target | +| `trim-target-memberships` | `alex/trim-target-memberships` | Stages A+B + membership trim, PR #7029 | +| `move-vpn-pixel-event` | `alex/move-vpn-pixel-event` | Stage C, uncommitted, on top of #7029 | +| `drop-sb-feedback`, `drop-sb-navigationbar`, `drop-sb-passwordmanager` | `alex/drop-*-storyboard` | Remaining storyboards | +| `coredata-spm-hacks-cleanup` | `alex/coredata-spm-hacks-cleanup` | no commits ahead of main yet | +| `netp-dependencies` | `alex/align-netp-dependencies` | Old draft: move files by target (superseded by the stages above) | | `vscode`, `vscode2` | `alex/fix-imports-cleanup`, `alex/vscode2` | Old: SwiftGenPlugin draft (uncommitted), root `Package.swift` | | `sweetpad` | `alex/cursor-fix-build` | Old `.vscode` (SweetPad) config | -Demo project proving the asset approach: `~/Developer/SPMAssetSymbolsDemo` (`./verify.sh`). +Gone: `spm-asset-symbols-plugin`, `coredata-code-models*`, `drop-sb-bookmarksbar*` (BookmarksBar storyboard + xib are on main), `appstore-sandbox-ui-tests` (#6980), `drop-dead-target-flags` (#6983), `split-shared-target-files`. + +Demo project for the (cancelled) asset approach: `~/Developer/SPMAssetSymbolsDemo` (`./verify.sh`).