Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.props
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
<Choose>
<When Condition="$([MSBuild]::IsOSPlatform('Windows'))">
<PropertyGroup>
<_ProtocExecutableExtension>.exe</_ProtocExecutableExtension>
<_ProtocPluginScriptExtension>.bat</_ProtocPluginScriptExtension>
</PropertyGroup>
</When>
Expand Down
124 changes: 110 additions & 14 deletions src/IceRpc.Protobuf.Tools/IceRpc.Protobuf.Tools.targets
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
AssemblyFile="$(IceRpcProtobufToolsTaskAssembliesPath)IceRpc.Protobuf.Tools.dll"
Runtime="NET"
/>

<!--
Build-order-only reference: the consuming project just needs IceRpc.Protobuf.Tools (and, via its own
ProjectReference, IceRpc.Protobuf.Generator) to be built so the protoc plug-in scripts exist on disk before we
Expand All @@ -33,10 +34,12 @@
<ProjectReference Include="$(MSBuildThisFileDirectory)IceRpc.Protobuf.Tools.csproj"
ReferenceOutputAssembly="false" Private="false" Targets="Build" />
</ItemGroup>

<ItemGroup>
<PropertyPageSchema Include="$(MSBuildThisFileDirectory)ProtoFile.ItemDefinition.xaml" />
<AvailableItemName Include="ProtoFile" />
</ItemGroup>

<ItemGroup Condition="'$(SetLinkMetadataAutomatically)' != 'false'">
<ProtoFile Update="@(ProtoFile)">
<LinkBase Condition="'%(LinkBase)' != ''">$([MSBuild]::EnsureTrailingSlash(%(LinkBase)))</LinkBase>
Expand All @@ -60,9 +63,96 @@
Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)"
/>
</ItemGroup>
<Target Name="ProtoCompile" BeforeTargets="CoreCompile" Condition="@(ProtoFile) != ''">

<!--
This target must not run the up-to-date check: Rebuild runs Clean then Build in the same project instance, so a
check made before Clean would mark as current the outputs Clean is about to delete. ProtoCompile keeps its own
_ProtoFile item because a task <Output> appends to an existing item.

In a source build the task assembly doesn't exist until IceRpc.Protobuf.Tools is built, hence the Exists check.
-->
<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'))" />
<_ProtoOutput Include="$([MSBuild]::NormalizePath('%(_ProtoNamedFile.OutputDir)/%(_ProtoNamedFile.OutputFileName).options'))" />
<_ProtoOutput
Include="$([MSBuild]::NormalizePath('%(_ProtoNamedFile.OutputDir)/%(_ProtoNamedFile.OutputFileName).BuildTelemetry.txt'))"
Condition="'$(IceRpcBuildTelemetry)' == 'true' And '$(IceRpcBuildTelemetryDebug)' == 'true'" />
</ItemGroup>
</Target>

<!-- Runs even when ProtoFile is empty: the outputs of a previous build still need pruning. -->
<Target Name="_ProtoPruneStaleOutputs"
BeforeTargets="ProtoCompile;ProtoClean"
DependsOnTargets="_ComputeProtoOutputs"
Condition="Exists('$(IntermediateOutputPath)protoc.outputs.txt')">

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude: Reproduced: a Debug build, a removed or renamed Proto file, then a Release build compiles the stale output until Debug is built again. The per-configuration/TargetFramework manifest deliberately mirrors IceRpc.Slice.Tools (#4589), so parallel multi-target inner builds do not share one state file. Building several configurations against the same shared OutputDir is a niche setup this PR does not handle. The suppressed comment on line 116 (manifest retained after the last Proto file is removed) is fixed as of 84d0d0a: _ProtoPruneStaleOutputs now deletes the manifest when there are no ProtoFile items.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When _ComputeProtoOutputs is skipped by its Exists condition while ProtoFile is not empty (source build, --no-dependencies after the Tools bin was cleaned), _ProtoOutput is empty, so every recorded output is treated as stale and deleted, and only then does ProtoCompile fail to load the task. The old ProtoCompile failed the same way but left the generated code alone. The Condition on _StaleProtoOutput in the suggestion below guards it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude: Applied in e323787. Verified: with the task assembly path pointing at a missing directory and --no-dependencies, _ProtoPruneStaleOutputs runs, deletes nothing, and the build fails on the task load with the generated code intact. ProtoClean's manifest Delete got the same condition: in a source build dotnet clean cleans the Tools project first, and deleting the manifest there would leave a removed Proto file's outputs unpruned by the next build.

<ReadLinesFromFile File="$(IntermediateOutputPath)protoc.outputs.txt">
<Output TaskParameter="Lines" ItemName="_PreviousProtoOutput" />
</ReadLinesFromFile>
<ItemGroup>
<!-- _ProtoOutput is also empty when _ComputeProtoOutputs was skipped; that must not prune everything. -->
<_StaleProtoOutput Include="@(_PreviousProtoOutput)" Exclude="@(_ProtoOutput)"
Condition="'@(_ProtoOutput)' != '' Or '@(ProtoFile)' == ''" />
<!-- Remove before deleting, or csc fails with CS2001 on the missing files. -->
<Compile Remove="@(_StaleProtoOutput)" MatchOnMetadata="FullPath" MatchOnMetadataOptions="PathLike" />
</ItemGroup>
<Delete Files="@(_StaleProtoOutput)" />
<!-- With no ProtoFile left, _ProtoWriteOutputManifest doesn't run to replace the manifest. -->
<Delete Files="$(IntermediateOutputPath)protoc.outputs.txt" Condition="@(ProtoFile) == ''" />
</Target>

<Target Name="ProtoCompile"
BeforeTargets="CoreCompile"
DependsOnTargets="_ComputeProtoOutputs"
Condition="@(ProtoFile) != ''">

<ItemGroup>
<!--
The inputs shared by every Proto file that change the generated code without touching a Proto file. Each
file's AdditionalOptions get their own record below.
-->
<_ProtocInputsCacheLine Include="protoc=$(ProtocBundledPluginVersion)" />
<_ProtocInputsCacheLine Include="icerpc-csharp=$(ProtocIceRpcPluginVersion)" />
<_ProtocInputsCacheLine Include="@(ProtoSearchPath->'search-path=%(FullPath)')" />
</ItemGroup>

<!--
WriteOnlyWhenDifferent moves the file's timestamp only when a value changes. As an input of every Proto file,
it then makes all outputs out of date, including those a failed protoc run left untouched.
-->
<WriteLinesToFile
File="$(IntermediateOutputPath)protoc.inputs.cache"
Lines="@(_ProtocInputsCacheLine)"
Overwrite="true"
WriteOnlyWhenDifferent="true" />

<ItemGroup>
<!-- Inputs of every Proto file, in addition to those recorded in its dependency file. -->
<_ProtocInput Include="$(IntermediateOutputPath)protoc.inputs.cache" />
<_ProtocInput Include="$(IceRpcProtocPath)$(IceRpcProtocPrefix)/protoc$(_ProtocExecutableExtension)" />
<_ProtocInput Include="$(IceRpcProtocGenPath)IceRpc.Protobuf.Generator.dll" />
</ItemGroup>

<!--
Each Proto file's own inputs cache, next to its dependency file: its AdditionalOptions, one per line, with the
same WriteOnlyWhenDifferent rule. A change makes that file, and only that file, out of date. The Condition skips
the empty batch MSBuild runs when _ComputeProtoOutputs was skipped, so the task load below fails first.
-->
<WriteLinesToFile
File="%(_ProtoNamedFile.OutputDir)/%(_ProtoNamedFile.OutputFileName).options"
Lines="%(_ProtoNamedFile.AdditionalOptions)"
Overwrite="true"
WriteOnlyWhenDifferent="true"
Condition="'%(_ProtoNamedFile.Identity)' != ''" />

<MakeDir Directories="%(ProtoFile.OutputDir)" />
<UpToDateCheckTask OutputDir="%(ProtoFile.OutputDir)" Sources="@(ProtoFile)">
<UpToDateCheckTask OutputDir="%(ProtoFile.OutputDir)" Sources="@(ProtoFile)" AdditionalInputs="@(_ProtocInput)">
<Output ItemName="_ProtoFile" TaskParameter="ComputedSources" />
</UpToDateCheckTask>

Expand Down Expand Up @@ -121,18 +211,24 @@
/>
</ItemGroup>
</Target>
<!--
_ProtoCleanFile must stay distinct from _ProtoFile: a task <Output> appends to an existing item, and Rebuild runs
Clean then Build in the same project instance.
-->
<Target Name="ProtoClean" BeforeTargets="Clean" Condition="Exists('$(IceRpcProtobufToolsTaskAssembliesPath)IceRpc.Protobuf.Tools.dll')">
<OutputFileNamesTask Sources="@(ProtoFile)">
<Output ItemName="_ProtoCleanFile" TaskParameter="ComputedSources" />
</OutputFileNamesTask>
<Delete Files="@(_ProtoCleanFile->'%(OutputDir)/%(OutputFileName).cs')" />
<Delete Files="@(_ProtoCleanFile->'%(OutputDir)/%(OutputFileName).IceRpc.cs')" />
<Delete Files="@(_ProtoCleanFile->'%(OutputDir)/%(OutputFileName).d')" />
<Delete Files="@(_ProtoCleanFile->'%(OutputDir)/%(OutputFileName).BuildTelemetry.txt')" />

<!-- Record this build's outputs so the next build can prune anything it no longer produces. -->
<Target Name="_ProtoWriteOutputManifest"
AfterTargets="ProtoCompile"
DependsOnTargets="_ComputeProtoOutputs"
Condition="@(ProtoFile) != ''">
<WriteLinesToFile
File="$(IntermediateOutputPath)protoc.outputs.txt"
Lines="@(_ProtoOutput)"
Overwrite="true"
WriteOnlyWhenDifferent="true" />
</Target>

<Target Name="ProtoClean" BeforeTargets="Clean" DependsOnTargets="_ComputeProtoOutputs">
<Delete Files="@(_ProtoOutput)" />
<!-- Keep the manifest when _ComputeProtoOutputs was skipped: the next build still needs it to prune. -->
<Delete Files="$(IntermediateOutputPath)protoc.outputs.txt;$(IntermediateOutputPath)protoc.inputs.cache"
Condition="'@(_ProtoOutput)' != '' Or '@(ProtoFile)' == ''" />
</Target>
Comment on lines +227 to 232

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete skips missing files (the first Delete already relies on that), so the Exists guards are redundant and one task does it. Same for the Exists half of the condition on the Delete in ProtoCompile, moot if the fingerprint becomes a cache file.

Suggested change
<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>
<Target Name="ProtoClean" BeforeTargets="Clean" DependsOnTargets="_ComputeProtoOutputs">
<Delete Files="@(_ProtoOutput)" />
<Delete Files="$(IntermediateOutputPath)protoc.outputs.txt;$(IntermediateOutputPath)protoc.fingerprint.txt" />
</Target>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude: Done in e323787; the Delete now covers protoc.inputs.cache instead of the fingerprint.


<!-- Package ProtoFile items -->
Expand Down
12 changes: 9 additions & 3 deletions src/IceRpc.Protobuf.Tools/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

The build runs `protoc` 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, or when its `AdditionalOptions` differ from those used to generate
it. Changing `ProtoSearchPath` or upgrading this package regenerates the code of all Proto files.

When you remove a Proto file from the project, rename it, or change its `OutputDir`, the next build deletes the code
previously generated for it.

## Generated code and NuGet packages

Expand Down
119 changes: 73 additions & 46 deletions src/IceRpc.Protobuf.Tools/UpToDateCheckTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ 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 the output directory for the generated code.</summary>
[Required]
public string OutputDir { get; set; } = "";
Expand All @@ -31,42 +36,32 @@ 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 all of its outputs exist, every input recorded in its dependency
/// file, its options record 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>();
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"),
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
Path.Combine(OutputDir, $"{fileName}.IceRpc.cs"),
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
];

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;
}
// The options record is the source's own inputs cache: the build rewrites it only when the source's
// AdditionalOptions change, so it is newer than the outputs exactly then.
string[] inputs = [.. additionalInputs, Path.Combine(OutputDir, $"{fileName}.options")];

bool upToDate = IsUpToDate(source.ItemSpec, outputs, dependOutput, inputs);

var computedSource = new TaskItem(source.ItemSpec);
source.CopyMetadataTo(computedSource);
Expand All @@ -78,36 +73,68 @@ 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)
// Every output must be newer than every input.
long oldestOutputTime = outputs.Min(output => File.GetLastWriteTime(output).Ticks);

foreach (string input in ProcessDependencies(dependOutput).Concat(additionalInputs))
{
if (!File.Exists(input))
{
return depends;
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;
}
}
Loading