Prune stale Protobuf outputs and tighten the protoc up-to-date check

src/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.props

@@ -25,6 +25,7 @@
   <Choose>
     <When Condition="$([MSBuild]::IsOSPlatform('Windows'))">
       <PropertyGroup>
+        <_ProtocExecutableExtension>.exe</_ProtocExecutableExtension>
         <_ProtocPluginScriptExtension>.bat</_ProtocPluginScriptExtension>
       </PropertyGroup>
     </When>

src/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.targets

@@ -60,9 +60,94 @@
       Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)"
     />
   </ItemGroup>
-  <Target Name="ProtoCompile" BeforeTargets="CoreCompile" Condition="@(ProtoFile) != ''">
+  <!--
+    Computes the files that protoc produces for the current ProtoFile items, as absolute paths, and the fingerprint
+    of the configuration that produces them. ProtoCompile and the prune, manifest and clean targets share it, and
+    MSBuild runs it once per build.
+
+    It deliberately doesn't run the up-to-date check, and ProtoCompile keeps its own _ProtoFile item: Rebuild runs
+    Clean and Build in the same project instance, so an up-to-date check made before Clean would mark as current the
+    very outputs Clean is about to delete. A task <Output> also appends to an existing item, so sharing _ProtoFile
+    with ProtoCompile would leave it with two entries per ProtoFile.
+
+    The task assembly only exists in source builds once IceRpc.Protobuf.Tools has been built, which isn't the case
+    for a Clean that precedes any Build; the Exists check skips the computation in that case.
+  -->
+  <Target Name="_ComputeProtoOutputs"
+          Condition="@(ProtoFile) != '' And Exists('$(IceRpcProtobufToolsTaskAssembliesPath)IceRpc.Protobuf.Tools.dll')">
+    <OutputFileNamesTask Sources="@(ProtoFile)">
+      <Output ItemName="_ProtoNamedFile" TaskParameter="ComputedSources" />
+    </OutputFileNamesTask>
+    <ItemGroup>
+      <_ProtoOutput Include="$([MSBuild]::NormalizePath('%(_ProtoNamedFile.OutputDir)/%(_ProtoNamedFile.OutputFileName).cs'))" />
+      <_ProtoOutput Include="$([MSBuild]::NormalizePath('%(_ProtoNamedFile.OutputDir)/%(_ProtoNamedFile.OutputFileName).IceRpc.cs'))" />
+      <_ProtoOutput Include="$([MSBuild]::NormalizePath('%(_ProtoNamedFile.OutputDir)/%(_ProtoNamedFile.OutputFileName).d'))" />
+      <!-- The xxx.BuildTelemetry.txt is generated when both IceRpcBuildTelemetry and IceRpcBuildTelemetryDebug are true -->
+      <_ProtoOutput
+        Include="$([MSBuild]::NormalizePath('%(_ProtoNamedFile.OutputDir)/%(_ProtoNamedFile.OutputFileName).BuildTelemetry.txt'))"
+        Condition="'$(IceRpcBuildTelemetry)' == 'true' And '$(IceRpcBuildTelemetryDebug)' == 'true'" />
+    </ItemGroup>
+    <PropertyGroup>
+      <!--
+        Everything that changes the generated code without touching a Proto file: the compiler and generator
+        versions, the import search path and the per-file protoc options. A change regenerates every Proto file.
+      -->
+      <_ProtocFingerprint>protoc=$(ProtocBundledPluginVersion);icerpc-csharp=$(ProtocIceRpcPluginVersion);search-path=@(ProtoSearchPath->'%(FullPath)', ',');options=@(ProtoFile->'%(Identity)=%(AdditionalOptions)', ',')</_ProtocFingerprint>
+    </PropertyGroup>
+  </Target>
+
+  <!--
+    Delete generated files recorded in the previous build's manifest that the current build no longer produces.
+    Without this, a removed or renamed .proto file (or a ProtoFile whose OutputDir changed) leaves orphan .cs files
+    behind that the SDK's default Compile glob keeps picking up.
+
+    BeforeTargets covers both the ProtoFile-present case (ProtoCompile/ProtoClean) and the ProtoFile-empty case where
+    those targets are skipped by their conditions but stale files from a previous build still need pruning
+    (CoreCompile/Clean).
+  -->
+  <Target Name="_ProtoPruneStaleOutputs"
+          BeforeTargets="ProtoCompile;CoreCompile;ProtoClean;Clean"
+          DependsOnTargets="_ComputeProtoOutputs"
+          Condition="Exists('$(IntermediateOutputPath)protoc.outputs.txt')">
+    <ReadLinesFromFile File="$(IntermediateOutputPath)protoc.outputs.txt">
+      <Output TaskParameter="Lines" ItemName="_PreviousProtoOutput" />
+    </ReadLinesFromFile>
+    <ItemGroup>
+      <_StaleProtoOutput Include="@(_PreviousProtoOutput)" Exclude="@(_ProtoOutput)" />
+      <!--
+        The SDK's default Compile glob captured these paths during item evaluation, with project-relative
+        Identities. We materialize the relative form into its own item so that <Compile Remove> can match it
+        - inlining the MakeRelative call inside an item transform pattern doesn't evaluate the property
+        function as expected.
+      -->
+      <_StaleProtoOutputRelative
+        Include="$([MSBuild]::MakeRelative($(MSBuildProjectDirectory), %(_StaleProtoOutput.Identity)))"
+        Condition="'%(_StaleProtoOutput.Identity)' != ''" />
+      <!--
+        Remove now, otherwise csc still reports the stale files as inputs and fails with CS2001 once we
+        delete them on disk. We try both the absolute and relative form to cover Compile entries added by
+        either explicit ProjectReference users or the SDK glob.
+      -->
+      <Compile Remove="@(_StaleProtoOutput)" />
+      <Compile Remove="@(_StaleProtoOutputRelative)" />
+    </ItemGroup>
+    <Delete Files="@(_StaleProtoOutput)" />
+  </Target>
+
+  <Target Name="ProtoCompile" BeforeTargets="CoreCompile" DependsOnTargets="_ComputeProtoOutputs" Condition="@(ProtoFile) != ''">
+    <ItemGroup>
+      <!-- Tools whose update must regenerate the code, in addition to the inputs recorded in the dependency files. -->
+      <_ProtocInput Include="$(IceRpcProtocPath)$(IceRpcProtocPrefix)/protoc$(_ProtocExecutableExtension)" />
+      <_ProtocInput Include="$(IceRpcProtocGenPath)IceRpc.Protobuf.Generator.dll" />
+    </ItemGroup>
+
     <MakeDir Directories="%(ProtoFile.OutputDir)" />
-    <UpToDateCheckTask OutputDir="%(ProtoFile.OutputDir)" Sources="@(ProtoFile)">
+    <UpToDateCheckTask
+      OutputDir="%(ProtoFile.OutputDir)"
+      Sources="@(ProtoFile)"
+      AdditionalInputs="@(_ProtocInput)"
+      Fingerprint="$(_ProtocFingerprint)"
+      FingerprintFile="$(IntermediateOutputPath)protoc.fingerprint.txt">
       <Output ItemName="_ProtoFile" TaskParameter="ComputedSources" />
     </UpToDateCheckTask>
 
@@ -121,14 +206,36 @@
       />
     </ItemGroup>
   </Target>
-  <Target Name="ProtoClean" BeforeTargets="Clean" Condition="Exists('$(IceRpcProtobufToolsTaskAssembliesPath)IceRpc.Protobuf.Tools.dll')">
-    <OutputFileNamesTask Sources="@(ProtoFile)">
-      <Output ItemName="_ProtoFile" TaskParameter="ComputedSources" />
-    </OutputFileNamesTask>
-    <Delete Files="@(_ProtoFile->'%(OutputDir)/%(OutputFilename).cs')" />
-    <Delete Files="@(_ProtoFile->'%(OutputDir)/%(OutputFilename).IceRpc.cs')" />
-    <Delete Files="@(_ProtoFile->'%(OutputDir)/%(OutputFilename).d')" />
-    <Delete Files="@(_ProtoFile->'%(OutputDir)/%(OutputFilename).BuildTelemetry.txt')" />
+
+  <!--
+    Record this build's outputs and configuration so the next build can prune anything no longer produced and
+    detect a configuration change. As an AfterTargets of ProtoCompile this only runs once protoc succeeded, so a
+    failed run never records a fingerprint its outputs don't match. The fingerprint is escaped because
+    WriteLinesToFile would otherwise split it on the semicolons.
+  -->
+  <Target Name="_ProtoWriteOutputManifest"
+          AfterTargets="ProtoCompile"
+          DependsOnTargets="_ComputeProtoOutputs"
+          Condition="@(ProtoFile) != ''">
+    <MakeDir Directories="$(IntermediateOutputPath)" />
+    <WriteLinesToFile
+      File="$(IntermediateOutputPath)protoc.outputs.txt"
+      Lines="@(_ProtoOutput)"
+      Overwrite="true"
+      WriteOnlyWhenDifferent="true" />
+    <WriteLinesToFile
+      File="$(IntermediateOutputPath)protoc.fingerprint.txt"
+      Lines="$([MSBuild]::Escape($(_ProtocFingerprint)))"
+      Overwrite="true"
+      WriteOnlyWhenDifferent="true" />
+  </Target>
+
+  <Target Name="ProtoClean" BeforeTargets="Clean" DependsOnTargets="_ComputeProtoOutputs">
+    <Delete Files="@(_ProtoOutput)" />
+    <Delete Files="$(IntermediateOutputPath)protoc.outputs.txt"
+            Condition="Exists('$(IntermediateOutputPath)protoc.outputs.txt')" />
+    <Delete Files="$(IntermediateOutputPath)protoc.fingerprint.txt"
+            Condition="Exists('$(IntermediateOutputPath)protoc.fingerprint.txt')" />
   </Target>
 
   <!-- Package ProtoFile items -->

src/IceRpc.Protobuf.Tools/README.md

@@ -75,9 +75,15 @@ once per Proto file.
 | Pack              | `false`   | Specifies whether or not to include the items (Proto files) in the NuGet package.                                                                |
 | PackagePath       | protobuf  | Sets the target path in the NuGet package. Used only when Pack is `true`.                                                                        |
 
-> [!NOTE]
-> Changing `AdditionalOptions` does not mark previously generated code as out of date. Run `dotnet clean` and then
-> build again to regenerate the code with the new options.
+## Incremental builds
+
+`protoc` runs only for the Proto files whose generated code is missing or out of date. A Proto file is out of date
+when the Proto file itself, one of the files it imports, `protoc` or the `protoc-gen-icerpc-csharp` generator is newer
+than one of its generated files. Changing `AdditionalOptions` or `ProtoSearchPath`, or upgrading this package,
+regenerates the code of all Proto files.
+
+The generated code of a Proto file that is removed from the project, renamed, or given a different `OutputDir` is
+deleted during the next build.
 
 ## Generated code and NuGet packages
 

src/IceRpc.Protobuf.Tools/UpToDateCheckTask.cs

@@ -12,6 +12,21 @@ namespace IceRpc.Protobuf.Tools;
 /// <summary>A MSBuild task to compute what Protobuf files have to be rebuild by <c>protoc</c>.</summary>
 public class UpToDateCheckTask : Microsoft.Build.Utilities.Task
 {
+    /// <summary>Gets or sets additional input files that every source depends on, typically the <c>protoc</c>
+    /// compiler and the code generator plug-in. A source is out of date when any of these files is missing or is
+    /// newer than one of the source's outputs.</summary>
+    public ITaskItem[] AdditionalInputs { get; set; } = [];
+
+    /// <summary>Gets or sets a string that identifies the configuration used to generate the outputs, such as the
+    /// compiler and plug-in versions and the options passed to them. When this value differs from the content of
+    /// <see cref="FingerprintFile"/>, every source is out of date.</summary>
+    public string Fingerprint { get; set; } = "";
+
+    /// <summary>Gets or sets the path of the file that holds the <see cref="Fingerprint"/> recorded by the previous
+    /// successful build. A missing file counts as a changed fingerprint. When empty, the fingerprint check is
+    /// skipped.</summary>
+    public string FingerprintFile { get; set; } = "";
+
     /// <summary>Gets or sets the output directory for the generated code.</summary>
     [Required]
     public string OutputDir { get; set; } = "";
@@ -31,42 +46,41 @@ public class UpToDateCheckTask : Microsoft.Build.Utilities.Task
     /// item is up to date or needs to be rebuilt. The <c>OutputFileName</c> metadata contains the base file name for
     /// the generated outputs. This is the input item's file name without the extension, and converted to PascalCase.
     /// </summary>
+    /// <remarks>A source is up to date only when the <see cref="Fingerprint"/> matches the recorded one, all of its
+    /// outputs exist, every input recorded in its dependency file and every <see cref="AdditionalInputs"/> entry
+    /// exists, and the newest input is older than the oldest output.</remarks>
     /// <returns>Returns <see langword="true"/> if the task was executed successfully, <see langword="false"/>
     /// otherwise.</returns>
     public override bool Execute()
     {
-        var computedSources = new List<ITaskItem>();
+        bool fingerprintChanged = false;
+        if (FingerprintFile.Length > 0)
+        {
+            fingerprintChanged =
+                !File.Exists(FingerprintFile) || File.ReadAllText(FingerprintFile).Trim() != Fingerprint.Trim();
+            if (fingerprintChanged)
+            {
+                Log.LogMessage(
+                    MessageImportance.Normal,
+                    "The protoc configuration changed since the previous build; all Proto files are out of date.");
+            }
+        }
+
+        string[] additionalInputs = [.. AdditionalInputs.Select(item => item.GetMetadata("FullPath"))];
 
+        var computedSources = new List<ITaskItem>();
         foreach (ITaskItem source in Sources)
         {
-            bool upToDate = true;
             string fileName = source.GetMetadata("FileName").ToProtocPascalCase();
             string dependOutput = Path.Combine(OutputDir, $"{fileName}.d");
-            string csharpOutput = Path.Combine(OutputDir, $"{fileName}.cs");
-            string icerpcOutput = Path.Combine(OutputDir, $"{fileName}.IceRpc.cs");
+            string[] outputs =
+            [
+                dependOutput,
+                Path.Combine(OutputDir, $"{fileName}.cs"),
+                Path.Combine(OutputDir, $"{fileName}.IceRpc.cs"),
+            ];
 
-            if (File.Exists(dependOutput) && File.Exists(csharpOutput) && File.Exists(icerpcOutput))
-            {
-                long lastWriteTime = Math.Max(
-                    File.GetLastWriteTime(dependOutput).Ticks,
-                    File.GetLastWriteTime(csharpOutput).Ticks);
-                lastWriteTime = Math.Max(lastWriteTime, File.GetLastWriteTime(icerpcOutput).Ticks);
-                List<string> dependencies = ProcessDependencies(dependOutput);
-                foreach (string dependency in dependencies)
-                {
-                    if (File.GetLastWriteTime(dependency).Ticks >= lastWriteTime)
-                    {
-                        // If a dependency is newer than any of the outputs the source is not up to date.
-                        upToDate = false;
-                        break;
-                    }
-                }
-            }
-            else
-            {
-                // If any of the outputs is missing the file is not up to date.
-                upToDate = false;
-            }
+            bool upToDate = !fingerprintChanged && IsUpToDate(source.ItemSpec, outputs, dependOutput, additionalInputs);
 
             var computedSource = new TaskItem(source.ItemSpec);
             source.CopyMetadataTo(computedSource);
@@ -78,36 +92,71 @@ public override bool Execute()
 
         ComputedSources = [.. computedSources];
         return true;
+    }
 
-        static List<string> ProcessDependencies(string dependOutput)
+    private bool IsUpToDate(string source, string[] outputs, string dependOutput, string[] additionalInputs)
+    {
+        string? missingOutput = outputs.FirstOrDefault(output => !File.Exists(output));
+        if (missingOutput is not null)
         {
-            var depends = new List<string>();
-            string dependContents = File.ReadAllText(dependOutput);
+            Log.LogMessage(MessageImportance.Low, $"'{source}' is out of date: output '{missingOutput}' is missing.");
+            return false;
+        }
 
-            // Strip everything before and including "Xxx.cs:" (the output target).
-            const string outputPrefix = ".cs:";
-            int i = dependContents.IndexOf(outputPrefix, StringComparison.CurrentCultureIgnoreCase);
-            if (i == -1 || i + outputPrefix.Length >= dependContents.Length)
+        // The outputs are only up to date when all of them are newer than every input, so compare against the
+        // oldest output rather than the newest.
+        long oldestOutputTime = outputs.Min(output => File.GetLastWriteTime(output).Ticks);
+
+        foreach (string input in ProcessDependencies(dependOutput).Concat(additionalInputs))
+        {
+            if (!File.Exists(input))
             {
-                return depends;
+                // File.GetLastWriteTime returns a placeholder date for missing files, which would otherwise make a
+                // deleted import look older than the outputs.
+                Log.LogMessage(MessageImportance.Low, $"'{source}' is out of date: input '{input}' is missing.");
+                return false;
             }
 
-            dependContents = dependContents[(i + outputPrefix.Length)..];
-
-            // The Make depfile format uses '\' at end of line as a line continuation, and escapes
-            // spaces inside paths as '\ '. Windows directory separators are emitted as literal '\'
-            // (not escaped). We split on newlines, strip the trailing continuation '\' and whitespace,
-            // then unescape '\ ' -> ' ' so paths containing spaces resolve correctly.
-            foreach (string line in dependContents.Split('\n'))
+            if (File.GetLastWriteTime(input).Ticks >= oldestOutputTime)
             {
-                string filePath = line.TrimEnd().TrimEnd('\\').Trim().Replace("\\ ", " ", StringComparison.Ordinal);
-                if (!string.IsNullOrEmpty(filePath))
-                {
-                    depends.Add(Path.GetFullPath(filePath));
-                }
+                Log.LogMessage(
+                    MessageImportance.Low,
+                    $"'{source}' is out of date: input '{input}' is newer than one of its outputs.");
+                return false;
             }
+        }
+
+        return true;
+    }
+
+    private static List<string> ProcessDependencies(string dependOutput)
+    {
+        var depends = new List<string>();
+        string dependContents = File.ReadAllText(dependOutput);
 
+        // Strip everything before and including "Xxx.cs:" (the output target).
+        const string outputPrefix = ".cs:";
+        int i = dependContents.IndexOf(outputPrefix, StringComparison.CurrentCultureIgnoreCase);
+        if (i == -1 || i + outputPrefix.Length >= dependContents.Length)
+        {
             return depends;
         }
+
+        dependContents = dependContents[(i + outputPrefix.Length)..];
+
+        // The Make depfile format uses '\' at end of line as a line continuation, and escapes
+        // spaces inside paths as '\ '. Windows directory separators are emitted as literal '\'
+        // (not escaped). We split on newlines, strip the trailing continuation '\' and whitespace,
+        // then unescape '\ ' -> ' ' so paths containing spaces resolve correctly.
+        foreach (string line in dependContents.Split('\n'))
+        {
+            string filePath = line.TrimEnd().TrimEnd('\\').Trim().Replace("\\ ", " ", StringComparison.Ordinal);
+            if (!string.IsNullOrEmpty(filePath))
+            {
+                depends.Add(Path.GetFullPath(filePath));
+            }
+        }
+
+        return depends;
     }
 }