Docs: rewrite prebuild-script migration guide around --combined-output#296
Merged
Conversation
The migration section pointed migrants at `generate --swift-manifest`, but our own Bazel rule and Tuist plugin use `generate --combined-output --module-info-output` instead — it's strictly simpler for hand-written prebuild scripts because it produces a single statically-known output path. Rewrite the section to lead with that flow (single-module + multi-module examples), link to `bazel/safedi.bzl` and the Tuist plugin as reference implementations, and keep `--swift-manifest` as the alternative for per-root output files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dfed
commented
May 11, 2026
dfed
commented
May 11, 2026
Co-authored-by: Dan Federman <dfed@me.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48700ece56
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #296 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 41 41
Lines 7043 7040 -3
=========================================
- Hits 7043 7040 -3
🚀 New features to boost your workflow:
|
A deps CSV produced by `echo "x.safedi" > deps.csv` carries a trailing newline, which previously survived `components(separatedBy: ",")` + `removingEmpty()` and ended up baked into the resolved file URL — causing the dependent module load to fail with a misleading file-not-found error. Centralize the parse in a `parsedPathListCSV()` helper that splits on commas/newlines and trims whitespace, applied to all three CSV-reading sites (Generate's deps + sources + cache-check, Scan's sources). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per CLAUDE.md: never .contains() for generator output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Documentation/Manual.mdpointed prebuild-script migrants atgenerate --swift-manifest, but our own Bazel rule (bazel/safedi.bzl) and Tuist plugin (TuistPlugins/SafeDITuist/.../SafeDI.swift) usegenerate --combined-output --module-info-outputinstead — that flow produces a single statically-known output path, which is strictly simpler for any hand-written prebuild script--combined-output: minimal single-module example, then a multi-module example that wires--module-info-output+--dependent-module-info-file-pathfor cross-module@Instantiableresolutionbazel/safedi.bzland the Tuist plugin as canonical reference implementations--swift-manifestdocumented as the alternative for per-root output files, and notes the two emission modes are mutually exclusiveSafeDITool's path-list CSV parsing: anecho "x.safedi" > deps.csv(as shown in the new docs) writes a trailing newline that previously survivedcomponents(separatedBy: ",")+removingEmpty()and got baked into the resolved file URL, failing the dependent-module load with a misleading file-not-found. A newString.parsedPathListCSV()helper now splits on commas/newlines and trims surrounding whitespace; applied to Generate's deps/sources/cache-check sites and Scan's sources site.Test plan
Documentation/Manual.mdand confirm the new section flows correctlybazel/safedi.bzl,TuistPlugins/SafeDITuist/ProjectDescriptionHelpers/SafeDI.swift) resolve on GitHubrun_combinedOutput_resolvesDependentModuleInfo_whenDepsCSVHasTrailingNewlinefails on the pre-fix code (file-not-found on<uuid>.safedi\n) and passes after the helper changeswift test --traits sourceBuild— 922 tests pass./CLI/lint.shclean🤖 Generated with Claude Code