Bazel: native build of SafeDI + safedi_compile rule + restored example#282
Bazel: native build of SafeDI + safedi_compile rule + restored example#282
Conversation
dfed
force-pushed
the
claude/bazel-example-project-d6f783
branch
from
f32076c to
1b399b0
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #282 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 41 41
Lines 7043 7043
=========================================
Hits 7043 7043 🚀 New features to boost your workflow:
|
dfed
force-pushed
the
claude/bazel-example-project-d6f783
branch
from
eb3f8ed to
996cba5
Compare
|
@codex please review this PR — it's a Bazel port of #281 demonstrating the same |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
## Summary Adds an opt-in `--combined-output <path>` flag on the `generate` subcommand that concatenates every Swift file the tool would otherwise emit — each root's dependency tree, each mock, the shared mock-configuration extensions — into a single file at the given path. ## Motivation Build systems whose code-generation rules require every output to be declared statically at analysis time (Bazel, Buck2) currently have two bad choices with SafeDI: 1. Hand-maintain a list of per-root / per-mock filenames in the BUILD / `*.bzl` file, edited every time a new `@Instantiable(isRoot: true)` or `generateMock: true` type is added. 2. Do the concatenation themselves in a `ctx.actions.run_shell(...)` — brings in shell escaping, manifest-path handling, sandbox-absolute paths, and throwaway declared outputs. `--combined-output` makes single-output rule shapes trivial: one action, one declared output, `ctx.actions.run` instead of `run_shell`. ## What's here - `@Option` on `Generate` with a clear help string. - Accumulates each entry's body in discovery order (roots first, then mocks, then shared mock configuration) and writes the file header exactly once at the top of the combined output. Empty bodies are skipped so the combined file doesn't contain duplicate headers with nothing underneath. - Composes with existing flags: - **`--swift-manifest` / `--output-directory`** — manifest still drives discovery; `--combined-output` only replaces placement. - **`--module-info-output`** — a module can emit both a combined `.swift` and a `.safedi` artifact. - **Neither of the above set** — tool synthesizes a temp scratch directory so the existing inline-scan path fires. - Parse errors: `#error(...)` stub routes to `--combined-output` in combined mode rather than each manifest entry's path. - Two tests (root + mock happy path; empty-module header-only edge case) in `Tests/SafeDIToolTests/SafeDIToolCombinedOutputTests.swift`. The test helper gains a `buildCombinedOutput` parameter that downstream tests can opt into. - `Documentation/Manual.md` gains a subsection on the flag. ## Follow-ups (not in this PR) - #282 will rebase onto main once this lands; its `safedi_generate` Bazel rule becomes `ctx.actions.run` with `--combined-output <declared>`, no concatenation, no `run_shell`. - TODO: BCR submission + release automation for SafeDI. ## Test plan - [x] `xcrun swift test --traits sourceBuild` — all 910 existing tests still pass; 2 new tests pass. - [x] `swift run swiftformat --lint Sources/ Tests/` clean. - [ ] CI green on macOS + Linux. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dfed
force-pushed
the
claude/bazel-example-project-d6f783
branch
from
996cba5 to
81702b9
Compare
dfed
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
dfed
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f56f3df2da
ℹ️ 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".
| let package = Package( | ||
| name: "ExampleBazelIntegration", | ||
| dependencies: [ | ||
| .package(url: "https://github.com/dfed/SafeDI.git", from: "2.0.0-beta-5"), |
There was a problem hiding this comment.
The example is pinned to the released GitHub package (from: "2.0.0-beta-5"), so the CI Build Example step in .github/workflows/ci.yml validates an external release instead of the code in this repo. That means Bazel integration regressions introduced in unreleased local changes (including changes to bazel/safedi.bzl or SafeDITool) can slip through while this job still passes, defeating the purpose of running the example build in this repository.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — the commit that intended to delete Package.swift missed the stage (the file was AD in git status; git commit only committed the A, leaving the deletion unstaged). Fixed in d1bd9b5 — Package.swift / Package.resolved are gone, and the example now consumes SafeDI via bazel_dep + local_path_override pointing at the workspace root. Our CI's Build Example step now validates local SafeDI changes as intended.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9418578d6f
ℹ️ 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".
| ctx.actions.run( | ||
| executable = ctx.executable._safeditool, | ||
| inputs = ctx.files.srcs + [input_csv] + extra_inputs, |
There was a problem hiding this comment.
safedi_generate only declares ctx.files.srcs (and optional module-info files) as action inputs, but SafeDITool can also parse files discovered via #SafeDIConfiguration(additionalDirectoriesToInclude:). In Bazel sandbox/remote execution, those discovered files are undeclared and therefore unavailable, so generation can fail or silently omit types for modules that rely on additional directories; the rule should add a scan/input-discovery phase (or explicit extra input attrs) before invoking generate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 2842816 via documentation rather than rule-level support. Bazel's hermetic model requires every input declared up front, and #SafeDIConfiguration(additionalDirectoriesToInclude:) is fundamentally a runtime-directory-walk feature — rewiring it to play with Bazel's sandbox would either require users to hand over the same files twice (once via srcs, once via a new rule attr) or smuggle non-declared files past the sandbox, which would break remote execution. I documented the incompatibility prominently at the top of bazel/safedi.bzl, pointing users at srcs as the single hand-off for "here's every source SafeDITool should see". If this turns into a real pain point once users hit it, we can add a data-style attr in a follow-up — but for v1 the doc + Bazel's own loud errors (missing-include-directory surfaces as a SafeDITool failure, not silent divergence) should be enough.
| @@ -0,0 +1,25 @@ | |||
| load("@rules_swift//swift:swift.bzl", "swift_library") | |||
| load("//bazel:safedi.bzl", "safedi_generate") | |||
There was a problem hiding this comment.
The example target loads a local copy of safedi.bzl instead of @safedi//bazel:safedi.bzl, so the CI “Build Example” job is not validating the newly exported consumer rules introduced in this commit. This allows regressions in the public SafeDI Bazel integration to slip through while the example still passes, because it exercises duplicated example-local Starlark instead of the module consumers are expected to load.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same commit-staging miss as the Package.swift thread. Fixed in d1bd9b5 — both Examples/ExampleBazelIntegration/ExampleBazelIntegration/BUILD.bazel and Subproject/BUILD.bazel now load("@safedi//bazel:safedi.bzl", ...) and depend on @safedi//Sources/SafeDI (not the old @swiftpkg_safedi). The example now exercises the exported consumer rules.
Addresses codex P1 feedback on PR #282. `safedi_generate` declares only `srcs` (and optional `module_infos`) as action inputs. If a `#SafeDIConfiguration` in those sources uses `additionalDirectoriesToInclude`, SafeDITool would try to walk those paths at runtime — but Bazel's hermetic sandbox only exposes declared inputs, so either SafeDITool can't find the directory (loud error) or finds a subset of what SPM/Xcode would see (silent divergence). Rather than build Starlark heuristics to detect the flag in Swift sources, or silently smuggle extra files past Bazel's dependency model, document the incompatibility at the top of `safedi.bzl` and point users at `srcs` as the single hand-off for "here's every source SafeDITool should see." Matches Bazel's hermetic idiom and keeps the rule surface small. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
…SafeDITool)
Provides Bazel-native build targets for SafeDI's four source modules
as an alternative to SPM. Downstream Bazel users can now depend on
SafeDI via `@safedi//Sources/SafeDI:SafeDI` without routing through
rules_swift_package_manager to consume SPM.
## What's in it
- **Root bzlmod scaffolding** — `MODULE.bazel`, `MODULE.bazel.lock`,
`BUILD.bazel`, `.bazelversion` (9.1.0), `.bazelrc`, `.bazelignore`.
- **Per-module `BUILD.bazel`** for the four source targets:
- `//Sources/SafeDICore:SafeDICore` — swift_library, depends on
SwiftSyntax/SwiftDiagnostics/SwiftSyntaxBuilder.
- `//Sources/SafeDIMacros:SafeDIMacros` — swift_compiler_plugin.
Inlines SafeDICore's sources into the same module via a
`SafeDICoreSources` filegroup (SPM does this via the
`Sources/SafeDIMacros/SafeDICore` symlink; Bazel's glob doesn't
follow symlinked directories, so we hand the files over
explicitly).
- `//Sources/SafeDI:SafeDI` — swift_library, wires SafeDIMacros as
its compiler plugin.
- `//Sources/SafeDITool:SafeDITool` — swift_binary, depends on
SafeDICore + ArgumentParser + SwiftParser.
- **External deps via rules_swift_package_manager** — reads
`Package.swift` / `Package.resolved` at bzlmod resolution and
generates `@swiftpkg_swift_syntax` / `@swiftpkg_swift_argument_parser`
targets. Keeps SPM and Bazel in sync from a single manifest.
- **CI job** — new `bazel` job in `.github/workflows/ci.yml` runs
`bazelisk build //Sources/...` on `macos-26`.
## Not in it (deferred)
- Test targets (SafeDITests, SafeDICoreTests, SafeDIMacrosTests,
SafeDIToolTests) — build-only first pass; tests can be added later
once the production build is proven.
- Publishing to the Bazel Central Registry — separate issue.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ships the Starlark surface that downstream Bazel consumers need to
actually use SafeDI (not just link against it).
## New
- `bazel/safedi.bzl` — two rules:
- `safedi_module_info(srcs)` — runs `SafeDITool generate
--module-info-output` and publishes the `.safedi` artifact for
cross-module consumers.
- `safedi_generate(srcs, module_infos)` — runs `SafeDITool generate
--combined-output` and emits a single declared `<rule_name>.swift`.
The `--combined-output` mode (landed in #283) is what makes
Bazel-native codegen clean: one statically-known output
regardless of how many `@Instantiable(isRoot:)` /
`generateMock: true` types the module declares.
Both default `_safeditool` to `Label("//Sources/SafeDITool:SafeDITool")`,
so consumers get the Bazel-built tool for free — no prebuilt bundle
fetching.
- `bazel/BUILD.bazel` — exports `safedi.bzl` for `load("@safedi//bazel:safedi.bzl", ...)`.
## Restored example
Brings `Examples/ExampleBazelIntegration/` back, adapted for the new
world:
- **Dep resolution via `bazel_dep` + `local_path_override`.** Dropped
`rules_swift_package_manager` + a bespoke `Package.swift`. The
example's `MODULE.bazel` declares `bazel_dep(name = "safedi", ...)`
and `local_path_override`s to `../..`. `local_path_override` only
applies when this module is root (our CI, local dev) — when BCR
eventually runs presubmit against a published source archive, it
ignores the override and resolves `safedi` from the registry. Same
file works in both contexts.
- **SafeDITool from source.** Dropped `bazel/safeditool_extension.bzl`
and its prebuilt artifact-bundle fetch. `safedi_generate` /
`safedi_module_info` reach `@safedi//Sources/SafeDITool:SafeDITool`,
which Bazel builds once and caches.
- **Load site updated** — `@safedi//bazel:safedi.bzl` instead of a
local `bazel/safedi.bzl` copy.
README reflects the new build model.
## CI
Extended the `bazel` job in `.github/workflows/ci.yml` to also build
the example (`cd Examples/ExampleBazelIntegration && bazelisk build
//Subproject:Subproject //ExampleBazelIntegration:ExampleBazelIntegration`)
so rule regressions get caught on every PR.
## Verified locally
- `bazelisk build //Sources/...` (outer workspace) — green.
- `cd Examples/ExampleBazelIntegration && bazelisk build //Subproject:Subproject //ExampleBazelIntegration:ExampleBazelIntegration` — green, emits one 190-line `safedi_generated.swift` containing the root + mocks + mock configuration.
- `swift build --traits sourceBuild` — green (SPM path unaffected).
- `./CLI/lint.sh` — clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The link-check job failed because `../ExampleTuistIntegration` only exists on the still-open PR #281 branch, not on main. Reference the Tuist example by name without the hyperlink — avoids cross-PR coupling without losing the intent.
Earlier commits `f56f3df` / `9418578` restored the example from the pre-reset commit via `git checkout 996cba5 -- Examples/...`, which staged the restore as a fresh addition. Subsequent Write/Edit calls modified the working tree but those edits weren't re-staged before committing — the committed state was the *restored* pre-repurpose content, not the adapted content. Codex correctly flagged this. This commit lands the actually-intended state: - `Package.swift` / `Package.resolved` — deleted. We now consume SafeDI via `bazel_dep`, not via `rules_swift_package_manager`. - `bazel/{BUILD.bazel,safedi.bzl,safeditool_extension.bzl}` — deleted. Rules live at `@safedi//bazel:safedi.bzl`; the prebuilt-fetcher extension is obsolete now that SafeDITool builds from source. - `MODULE.bazel` — switched to `bazel_dep(name = "safedi", ...)` + `local_path_override(path = "../..")`. Same file works in our CI and in downstream BCR presubmit. - `BUILD.bazel` — cleared root-file exports (Package.swift/Resolved are gone). - `ExampleBazelIntegration/BUILD.bazel` + `Subproject/BUILD.bazel` — load rules from `@safedi//bazel:safedi.bzl` (not a local copy) and depend on `@safedi//Sources/SafeDI` (not `@swiftpkg_safedi//:SafeDI`). - `.gitignore` — stopped ignoring `MODULE.bazel.lock`; the lock file is now committed for reproducibility (matches the root workspace). Addresses the "Use exported @Safedi rules in Bazel example" and "Use local SafeDI source for in-repo Bazel example builds" codex comments — both were pointing at stale-in-repo state that the earlier commits had intended but failed to actually land. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses codex P1 feedback on PR #282. `safedi_generate` declares only `srcs` (and optional `module_infos`) as action inputs. If a `#SafeDIConfiguration` in those sources uses `additionalDirectoriesToInclude`, SafeDITool would try to walk those paths at runtime — but Bazel's hermetic sandbox only exposes declared inputs, so either SafeDITool can't find the directory (loud error) or finds a subset of what SPM/Xcode would see (silent divergence). Rather than build Starlark heuristics to detect the flag in Swift sources, or silently smuggle extra files past Bazel's dependency model, document the incompatibility at the top of `safedi.bzl` and point users at `srcs` as the single hand-off for "here's every source SafeDITool should see." Matches Bazel's hermetic idiom and keeps the rule surface small. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The earlier copy ("would regress the 'no hardcoded filenames'
property we cared about for Tuist") singled out Tuist as the source
of that concern, but every build system worth using cares about not
making authors hand-maintain output lists. Recast as a generic
build-author-burden argument.
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…safedi_compile Mirrors the shape of the Tuist plugin helper that landed in #281 (`SafeDI.preCompileScript(module:dependents:)`), which writes both the `.safedi` module-info artifact and the combined `.swift` source in a single SafeDITool invocation. The previous Bazel split forced producer-with-mocks targets to invoke SafeDITool twice on the same sources for no gain. ## Rule shape ```starlark load("@safedi//bazel:safedi.bzl", "safedi_compile") safedi_compile( name = "Foo_safedi", srcs = glob([...]), dependents = ["//OtherModule:OtherModule_safedi"], # optional ) ``` Outputs two artifacts: - `<rule>.swift` — combined dependency tree + mocks + mock configuration. Exposed via `DefaultInfo`, so a downstream `swift_library.srcs = glob([...]) + [":<rule>"]` picks it up exactly the way Tuist's `SafeDI.generatedSource` adds the generated source to a target. - `<rule>.safedi` — module-info artifact. Carried via the new `SafeDIInfo` provider; cross-module consumers list the producer rule label in their own `safedi_compile.dependents`. ## Wins - Halves SafeDITool invocations for any producer-with-mocks target. - Tighter API parallel with Tuist (one call site per module). - The `providers = [SafeDIInfo]` constraint on `dependents` blocks passing arbitrary labels — type-safer than the old `allow_files = [".safedi"]` constraint. ## Updates - `bazel/safedi.bzl` — single `safedi_compile` rule + `SafeDIInfo` provider. File-level docstring rewritten to call out the Tuist helper parallel. - `bazel/BUILD.bazel` — comment refers to `safedi_compile`. - `Examples/ExampleBazelIntegration/Subproject/BUILD.bazel` — uses `safedi_compile`; the Subproject's own `swift_library` now includes its `:Subproject_safedi` output (matters once mocks land for Subproject, harmless before). - `Examples/ExampleBazelIntegration/ExampleBazelIntegration/BUILD.bazel` — uses `safedi_compile` with `dependents` instead of `module_infos`. - `Examples/ExampleBazelIntegration/README.md` — comparison table + prose updated to reference `safedi_compile`. ## Verified - `bazelisk build //Sources/...` from repo root — green. - `cd Examples/ExampleBazelIntegration && bazelisk build //...` — green. - `swift build --traits sourceBuild` — green. - `./CLI/lint.sh` — clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dfed
force-pushed
the
claude/bazel-example-project-d6f783
branch
from
7547902 to
5136cd9
Compare
dfed
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Adds a complete Bazel-native story for SafeDI in one PR: (1) Bazel builds of the four source modules, (2) a user-facing
safedi_compilerule that mirrors the Tuist plugin's helper, (3) a restored consumer example exercising the full pipeline.Scope is consumer-build-only. Test targets and BCR publishing are deferred to follow-up PRs.
What's in it
1. Bzlmod scaffolding + native build (
e873029)MODULE.bazel—rules_swift3.6.0 +rules_swift_package_manager1.15.0; translatesswift-syntax/swift-argument-parserfromPackage.swiftinto Bazel targets.BUILD.bazel://Sources/SafeDICore:SafeDICore—swift_library.//Sources/SafeDIMacros:SafeDIMacros—swift_compiler_plugin. SPM'sSources/SafeDIMacros/SafeDICoresymlink isn't followed by Bazel'sglob, so aSafeDICoreSourcesfilegroup hands the files over explicitly.//Sources/SafeDI:SafeDI—swift_librarywithSafeDIMacroswired as compiler plugin.//Sources/SafeDITool:SafeDITool—swift_binary..bazelversion,.bazelrc,.bazelignore,MODULE.bazel.lock, rootBUILD.bazel.bazelCI job onmacos-26.2.
safedi_compilerule (5136cd9, latest)bazel/safedi.bzlships a single rule that runsSafeDITool generate --combined-output --module-info-outputonce per module and emits both artifacts:<rule>.swift(combined dependency tree + mocks + mock configuration) → exposed viaDefaultInfo, soswift_library.srcs = glob([...]) + [":<rule>"]picks it up exactly like Tuist'sSafeDI.generatedSource.<rule>.safedi(module-info artifact) → carried via the newSafeDIInfoprovider; cross-module consumers list the producer label in their ownsafedi_compile.dependents.The rule mirrors the Tuist plugin helper that landed in #281 (
SafeDI.preCompileScript(module:dependents:)). Previously this PR shipped two rules (safedi_module_info+safedi_generate); the collapse halves SafeDITool invocations for producer-with-mocks targets and gives a tighter API parallel with Tuist.3. Restored
Examples/ExampleBazelIntegration/Bazel port of
ExampleTuistIntegration's multi-target structure — same Swift sources (types, mocks, previews), different build system:swift_librarytargets (//Subproject:Subproject+//ExampleBazelIntegration:ExampleBazelIntegration).safedi_compileper module with cross-module.safedihandoff viadependents.MODULE.bazelusesbazel_dep(name = "safedi") + local_path_override(path = "../.."). The override applies only when this module is root (our CI, local dev); BCR presubmit (PR BCR: automate publishing SafeDI to the Bazel Central Registry #287) consuming the published source archive ignores it.bazeljob extended to also build the example.Not in it (deferred)
SafeDITests, etc.) — build-only first pass.Test plan
bazelisk build //Sources/...from repo root — green.cd Examples/ExampleBazelIntegration && bazelisk build //Subproject:Subproject //ExampleBazelIntegration:ExampleBazelIntegration— green; one SafeDITool invocation per module emitting both.safediand.swift.swift build --traits sourceBuild— green (SPM path unaffected)../CLI/lint.sh— clean.bazelCI job passes on macos-26 (both stages).🤖 Generated with Claude Code