Skip to content

Make GeneratePublishDependencyFile incremental #32305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 26, 2023
Merged

Make GeneratePublishDependencyFile incremental #32305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2c7a0cd
Add test for incremental publish with excluded assets
sbomer May 5, 2023
5b540f2
Make GeneratePublishDependencyFile incremental
sbomer May 5, 2023
9e519ad
Don't rely on incremental publish for compression test
sbomer May 10, 2023
01a16b2
Revert "Don't rely on incremental publish for compression test"
sbomer May 23, 2023
ef75097
Merge remote-tracking branch 'origin/main' into incrementalPublishFix
sbomer May 23, 2023
490d428
Fix incremental build when properties change
sbomer May 23, 2023
9cb55bc
Fix typo
sbomer May 23, 2023
fe61265
Fix target dependency, typo
sbomer May 24, 2023
7c36074
PR feedback
sbomer May 25, 2023
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
90 changes: 81 additions & 9 deletions src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,11 +919,37 @@ Copyright (c) .NET Foundation. All rights reserved.
</PropertyGroup>
</Target>

<Target Name="_GenerateSingleFileBundleInputCache">
<ItemGroup>
<_GenerateSingleFileBundlePropertyInputsCacheToHash Include="$(PublishedSingleFilePath)" />
<_GenerateSingleFileBundlePropertyInputsCacheToHash Include="$(TraceSingleFileBundler)" />
<_GenerateSingleFileBundlePropertyInputsCacheToHash Include="$(IncludeSymbolsInSingleFile)" />
<_GenerateSingleFileBundlePropertyInputsCacheToHash Include="$(IncludeAllContentForSelfExtract)" />
<_GenerateSingleFileBundlePropertyInputsCacheToHash Include="$(IncludeNativeLibrariesForSelfExtract)" />
<_GenerateSingleFileBundlePropertyInputsCacheToHash Include="$(EnableCompressionInSingleFile)" />
<_GenerateSingleFileBundlePropertyInputsCacheToHash Include="$(PublishedSingleFileName)" />
<_GenerateSingleFileBundlePropertyInputsCacheToHash Include="$(RuntimeIdentifier)" />
<_GenerateSingleFileBundlePropertyInputsCacheToHash Include="$(PublishDir)" />
<_GenerateSingleFileBundlePropertyInputsCacheToHash Include="$(_TargetFrameworkVersionWithoutV)" />
<_GenerateSingleFileBundlePropertyInputsCacheToHash Include="@(FilesToBundle)" />
</ItemGroup>

<Hash ItemsToHash="@(_GenerateSingleFileBundlePropertyInputsCacheToHash)">
<Output TaskParameter="HashResult" PropertyName="_GenerateSingleFileBundlePropertyInputsCacheHash" />
</Hash>

<WriteLinesToFile
Lines="$(_GenerateSingleFileBundlePropertyInputsCacheHash)"
File="$(_GenerateSingleFileBundlePropertyInputsCache)"
Overwrite="true"
WriteOnlyWhenDifferent="true" />
</Target>

<UsingTask TaskName="GenerateBundle" AssemblyFile="$(MicrosoftNETBuildTasksAssembly)" />
<Target Name="GenerateSingleFileBundle"
Condition="'$(PublishSingleFile)' == 'true'"
DependsOnTargets="_ComputeFilesToBundle;PrepareForBundle"
Inputs="@(FilesToBundle)"
DependsOnTargets="_ComputeFilesToBundle;PrepareForBundle;_GenerateSingleFileBundleInputCache"
Inputs="@(FilesToBundle);$(_GenerateSingleFileBundlePropertyInputsCache)"
Copy link
Member

@dsplaisted dsplaisted May 25, 2023

Choose a reason for hiding this comment

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

It may be better to include FilesToBundle in the hash rather than as a normal input so that if a file to bundle is removed, it is handled correctly.

Copy link
Member Author

@sbomer sbomer May 25, 2023

Choose a reason for hiding this comment

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

I added this to the hash, but I think it also needs to be a normal input - that way it'll rebuild if the list of filenames changes, or if the file timestamps changes.

Outputs="$(PublishedSingleFilePath)">

<PropertyGroup>
Expand Down Expand Up @@ -957,6 +983,50 @@ Copyright (c) .NET Foundation. All rights reserved.

</Target>

<Target Name="_ComputeIntermediateDepsFilePath">
<PropertyGroup>
<!-- IntermediateDepsFilePath is the location where the deps.json file is originally created -->
<IntermediateDepsFilePath Condition=" '$(PublishDepsFilePath)' != ''">$(PublishDepsFilePath)</IntermediateDepsFilePath >
<IntermediateDepsFilePath Condition=" '$(PublishDepsFilePath)' == ''">$(IntermediateOutputPath)$(ProjectDepsFileName)</IntermediateDepsFilePath >
</PropertyGroup>
</Target>

<Target Name="_GeneratePublishDependencyFileInputCache"
DependsOnTargets="_ComputeIntermediateDepsFilePath">
<ItemGroup>
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(PublishDepsFilePath)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(PublishSingleFile)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(MSBuildProjectFullPath)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(ProjectAssetsFile)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(IntermediateDepsFilePath)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(TargetFramework)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(AssemblyName)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(TargetExt)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(Version)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(IncludeMainProjectInDepsFile)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(RuntimeIdentifier)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(MicrosoftNETPlatformLibrary)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(SelfContained)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(IncludeFileVersionsInDependencyFile)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(BundledRuntimeIdentifierGraphFile)" />
<_GeneratePublishDependencyFilePropertyInputsCacheToHash Include="$(IncludeProjectsNotInAssetsFileInDepsFile)" />
</ItemGroup>

<Hash ItemsToHash="@(_GeneratePublishDependencyFilePropertyInputsCacheToHash)">
<Output TaskParameter="HashResult" PropertyName="_GeneratePublishDependencyFilePropertyInputsCacheHash" />
</Hash>

<WriteLinesToFile
Lines="$(_GeneratePublishDependencyFilePropertyInputsCacheHash)"
File="$(_GeneratePublishDependencyFilePropertyInputsCache)"
Overwrite="True"
WriteOnlyWhenDifferent="True" />

<ItemGroup>
<FileWrites Include="$(_GeneratePublishDependencyFilePropertyInputsCache)" />
</ItemGroup>
</Target>

<!--
============================================================
_GeneratePublishDependencyFile
Expand All @@ -965,25 +1035,27 @@ Copyright (c) .NET Foundation. All rights reserved.
-->
<Target Name="GeneratePublishDependencyFile"
DependsOnTargets="_ComputeUseBuildDependencyFile;
_ComputeIntermediateDepsFilePath;
_DefaultMicrosoftNETPlatformLibrary;
_HandlePackageFileConflicts;
_HandlePackageFileConflictsForPublish;
_ComputeReferenceAssemblies;
_ComputeUserRuntimeAssemblies;
ResolveRuntimePackAssets;
_ComputePackageReferencePublish"
Condition="'$(GenerateDependencyFile)' == 'true' and '$(_UseBuildDependencyFile)' != 'true' and '$(PublishAot)' != 'true'">
_ComputePackageReferencePublish;
_GeneratePublishDependencyFileInputCache"
Condition="'$(GenerateDependencyFile)' == 'true' and '$(_UseBuildDependencyFile)' != 'true' and '$(PublishAot)' != 'true'"
Inputs="$(ProjectAssetsFile);$(ProjectAssetsCacheFile);$(MSBuildAllProjects);$(_GeneratePublishDependencyFilePropertyInputsCache)"
Copy link
Member

@dsplaisted dsplaisted May 25, 2023

Choose a reason for hiding this comment

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

A lot of the item inputs aren't declared here or included in the hash. Is this because MSBuildAllProjects will pick up any changes to the project file, and any properties passed on the command line will probably change properties that are included in the hash?

Copy link
Member Author

@sbomer sbomer May 25, 2023

Choose a reason for hiding this comment

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

I didn't include the items in Inputs or the hash because they weren't included in the incremental inputs for GenerateBuildDependencyFile either, and I would suspect that they are only likely to change when the project file changes.

I did try to include all relevant properties in the input hash, except for $(MSBuildProjectFullPath), since I thought that would likely invalidate most of the build already - but I'll add it to be safe.

Outputs="$(IntermediateDepsFilePath)">

<PropertyGroup>
<!-- IntermediateDepsFilePath is the location where the deps.json file is originally created
PublishDepsFilePath is the location where the deps.json resides when published
PublishDepsFilePath is empty (by default) for PublishSingleFile, since the deps.json file is embedde within the single-file bundle -->
<IntermediateDepsFilePath Condition=" '$(PublishDepsFilePath)' != ''">$(PublishDepsFilePath)</IntermediateDepsFilePath >
<IntermediateDepsFilePath Condition=" '$(PublishDepsFilePath)' == ''">$(IntermediateOutputPath)$(ProjectDepsFileName)</IntermediateDepsFilePath >
<!-- PublishDepsFilePath is the location where the deps.json resides when published
PublishDepsFilePath is empty (by default) for PublishSingleFile, since the deps.json file is embedded within the single-file bundle -->
<PublishDepsFilePath Condition=" '$(PublishDepsFilePath)' == '' And '$(PublishSingleFile)' != 'true'">$(PublishDir)$(ProjectDepsFileName)</PublishDepsFilePath>
<_IsSingleFilePublish Condition="'$(PublishSingleFile)' == ''">false</_IsSingleFilePublish>
<_IsSingleFilePublish Condition="'$(PublishSingleFile)' != ''">$(PublishSingleFile)</_IsSingleFilePublish>
</PropertyGroup>

<ItemGroup>
<ResolvedCompileFileDefinitions Remove="@(_PublishConflictPackageFiles)" Condition="'%(_PublishConflictPackageFiles.ConflictItemType)' == 'Reference'" />
<RuntimeTargetsCopyLocalItems Remove="@(_PublishConflictPackageFiles)" Condition="'%(_PublishConflictPackageFiles.ConflictItemType)' != 'Reference'" />
Expand Down
4 changes: 4 additions & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ Copyright (c) .NET Foundation. All rights reserved.
<ComputeNETCoreBuildOutputFiles Condition=" '$(ComputeNETCoreBuildOutputFiles)' == '' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp'">true</ComputeNETCoreBuildOutputFiles>
<_GenerateRuntimeConfigurationPropertyInputsCache Condition="'$(_GenerateRuntimeConfigurationPropertyInputsCache)' == ''">$(IntermediateOutputPath)$(MSBuildProjectName).genruntimeconfig.cache</_GenerateRuntimeConfigurationPropertyInputsCache>
<_GenerateRuntimeConfigurationPropertyInputsCache>$([MSBuild]::NormalizePath($(MSBuildProjectDirectory), $(_GenerateRuntimeConfigurationPropertyInputsCache)))</_GenerateRuntimeConfigurationPropertyInputsCache>
<_GeneratePublishDependencyFilePropertyInputsCache Condition="'$(_GeneratePublishDependencyFilePropertyInputsCache)' == ''">$(IntermediateOutputPath)$(MSBuildProjectName).genpublishdeps.cache</_GeneratePublishDependencyFilePropertyInputsCache>
<_GeneratePublishDependencyFilePropertyInputsCache>$([MSBuild]::NormalizePath($(MSBuildProjectDirectory), $(_GeneratePublishDependencyFilePropertyInputsCache)))</_GeneratePublishDependencyFilePropertyInputsCache>
<_GenerateSingleFileBundlePropertyInputsCache Condition="'$(_GenerateSingleFileBundlePropertyInputsCache)' == ''">$(IntermediateOutputPath)$(MSBuildProjectName).genbundle.cache</_GenerateSingleFileBundlePropertyInputsCache>
<_GenerateSingleFileBundlePropertyInputsCache>$([MSBuild]::NormalizePath($(MSBuildProjectDirectory), $(_GenerateSingleFileBundlePropertyInputsCache)))</_GenerateSingleFileBundlePropertyInputsCache>
</PropertyGroup>

<ItemGroup>
Expand Down
32 changes: 32 additions & 0 deletions src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishIncrementally.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using FluentAssertions;
using System;
using System.IO;
using System.Linq;
using Microsoft.NET.TestFramework;
Expand Down Expand Up @@ -300,6 +301,37 @@ public void It_cleans_with_multiple_output_dirs()
CheckPublishOutput(publishDir2, expectedSingleExeFiles, expectedNonSingleExeFiles);
}

[Theory]
[InlineData(ToolsetInfo.CurrentTargetFramework)]
public void GeneratePublishDependencyFile_runs_incrementally(string targetFramework)
{
var rid = EnvironmentInfo.GetCompatibleRid(targetFramework);
var testProject = new TestProject()
{
Name = "PublishDependencyFileIncremental",
TargetFrameworks = targetFramework,
IsExe = true,
RuntimeIdentifier = rid
};

testProject.PackageReferences.Add(new TestPackageReference("NewtonSoft.Json", "13.0.1", publish: "false"));
var testAsset = _testAssetsManager.CreateTestProject(testProject, testProject.Name);

var publishCommand = new PublishCommand(testAsset);
var publishDir = publishCommand.GetOutputDirectory(targetFramework, runtimeIdentifier: rid).FullName;
var depsJsonPath = Path.Combine(publishDir, testProject.Name + ".deps.json");

publishCommand.Execute().Should().Pass();
DateTime depsJsonFirstModifiedTime = File.GetLastWriteTimeUtc(depsJsonPath);

WaitForUtcNowToAdvance();

publishCommand.Execute().Should().Pass();
DateTime depsJsonSecondModifiedTime = File.GetLastWriteTimeUtc(depsJsonPath);

depsJsonSecondModifiedTime.Should().Be(depsJsonFirstModifiedTime);
}

private void CheckPublishOutput(string publishDir, IEnumerable<string> expectedFiles, IEnumerable<string> unexpectedFiles)
{
if (expectedFiles != null)
Expand Down