[2026-10-01] eagle: work/projects/vscode-migration/findings.md work/projects/vscode-migration/vscode-migration.md
This commit is contained in:
1 parent
7985ade060
commit
faab194cd0
2 files changed
+58
-11
No files matched your search
@@ -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`).
|
||||
|
||||
@@ -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`).
|
||||
Reference in new issue
Block a user