Delete the Slice output manifest when the last Slice file is removed - #4930
Conversation
_SlicecWriteOutputManifest only runs when the project has SliceFile items, so removing the last Slice file left the previous manifest in place after its outputs were pruned. Every later build then pruned the same paths again, deleting any file created there since, such as a hand-written replacement for the generated code. _SlicecPruneStaleOutputs now deletes the manifest when there are no SliceFile items, after pruning the recorded outputs. Same fix as the Protobuf targets in #4923.
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped to manifest lifecycle handling and aligns with the existing manifest/prune design, reducing the risk of unintended deletions in later builds.
Pull request overview
This PR closes an incremental-build safety gap in the Slice MSBuild targets: when the last SliceFile is removed from a project, the prior slicec.outputs.txt manifest could be left behind and cause subsequent builds to keep pruning files at the old generated output paths (including newly hand-written replacements).
Changes:
- Extend
_SlicecPruneStaleOutputsto delete$(IntermediateOutputPath)slicec.outputs.txtwhen there are noSliceFileitems. - Prevent repeated stale-output pruning in SliceFile-empty builds by removing the now-stale manifest after the recorded outputs have been pruned.
File summaries
| File | Description |
|---|---|
| src/IceRpc.Slice.Tools/IceRpc.Slice.Tools.targets | Deletes slicec.outputs.txt in the no-SliceFile case to stop repeated pruning of former generated-code paths. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
bernardnormier
left a comment
bernardnormier
left a comment
There was a problem hiding this comment.
One comment nit, the same one as on #4923.
The What's Changed entry can go: the scenario needs a hand-written file at a former generated-code path after the
last Slice file is removed, which no project does in practice. Suggested replacement:
None — only affects a hand-written file placed at a former generated-code path after the last Slice file is
removed, which no project does in practice.
Follow-up, not for this PR: the Slice targets could take the #4923 cleanups (BeforeTargets trimmed to Slicec and
SlicecClean, MatchOnMetadata instead of the relative item, no MakeDir or Exists guards, WriteOnlyWhenDifferent
on the manifest).
Co-authored-by: Bernard Normier <bernard@zeroc.com>
Follow-up to #4923, where the same gap was found in the Protobuf targets (Copilot's suppressed comment on
_ProtoPruneStaleOutputs)._SlicecWriteOutputManifestonly runs when the project hasSliceFileitems. Removing the last.slicefile therefore leaves the previousslicec.outputs.txtin place after_SlicecPruneStaleOutputsdeletes its outputs, and every later build prunes the same paths again. A file created at one of those paths since, such as a hand-written replacement for the generated code, is deleted on each build._SlicecPruneStaleOutputsnow deletes the manifest when there are noSliceFileitems, after pruning the recorded outputs.Verification
Two-file probe importing the source-build props/targets (macOS, .NET SDK 10.0.201), slicec runs per build:
.slicefiles, buildgenerated/a.csby hand, builda.csdeleteda.cskept.slicefiles back, builda.slicetoalpha.slicea.cspruneda.cspruned.slicefiles, Cleansrc/IceRpc(which imports these targets) builds clean from this branch.What's Changed entry
None — only affects a hand-written file placed at a former generated-code path after the last Slice file is removed, which no project does in practice.