Add question flag to "question" the build if it is incremental (#8012)

This PR adds "question" switch to msbuild.exe that will error out if a target or a task fails incremental check. Targets will fail if both Inputs and Outputs are present and not skip. Tasks changes are individually modified to support the interface IIncrementalTask, which sets the question boolean. Each task will need to be updated this interface take part.

I have started with the following tasks and fixed some of the issues within MSBuild enlistment. And there are more, see the notes below. Tasks updated: ToolTask, Copy, MakeDir, Touch, WriteLinesTofile, RemoveDir, DownloadFile, Move, ZipDirectory, Unzip, GenerateResource, GenerateBindingRedirects.

Using question investigate incremental issues is orders of magnitude easier. Reading the logs is simpler and repros are more consistent. In this PR, it includes a few fixes to the common targets which address some issues.

Fixes #7348

Author: yuehuang010

eng/BootStrapMSBuild.targets

@@ -122,63 +122,82 @@
 
     <!-- Copy in props and targets from the machine-installed MSBuildExtensionsPath -->
     <Copy SourceFiles="@(InstalledVersionedExtensions)"
-          DestinationFiles="@(InstalledVersionedExtensions->'$(BootstrapDestination)$(TargetMSBuildToolsVersion)\%(RecursiveDir)%(Filename)%(Extension)')" />
+          DestinationFiles="@(InstalledVersionedExtensions->'$(BootstrapDestination)$(TargetMSBuildToolsVersion)\%(RecursiveDir)%(Filename)%(Extension)')"
+          SkipUnchangedFiles="true" />
     <Copy SourceFiles="@(SdkResolverFiles)"
-          DestinationFiles="@(SdkResolverFiles->'$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\SdkResolvers\Microsoft.DotNet.MSBuildSdkResolver\%(RecursiveDir)%(Filename)%(Extension)')" />
+          DestinationFiles="@(SdkResolverFiles->'$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\SdkResolvers\Microsoft.DotNet.MSBuildSdkResolver\%(RecursiveDir)%(Filename)%(Extension)')"
+          SkipUnchangedFiles="true" />
 
     <Copy SourceFiles="@(InstalledMicrosoftExtensions)"
-          DestinationFiles="@(InstalledMicrosoftExtensions->'$(BootstrapDestination)Microsoft\%(RecursiveDir)%(Filename)%(Extension)')" />
+          DestinationFiles="@(InstalledMicrosoftExtensions->'$(BootstrapDestination)Microsoft\%(RecursiveDir)%(Filename)%(Extension)')"
+          SkipUnchangedFiles="true" />
 
     <Copy SourceFiles="@(InstalledSdks)"
           DestinationFiles="@(InstalledSdks -> '$(BootstrapDestination)Sdks\%(RecursiveDir)%(Filename)%(Extension)')"
-          Condition="'$(MonoBuild)' != 'true'" />
+          Condition="'$(MonoBuild)' != 'true'"
+          SkipUnchangedFiles="true" />
     <Copy SourceFiles="@(InstalledSdks)"
           DestinationFiles="@(InstalledSdks -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\Sdks\%(RecursiveDir)%(Filename)%(Extension)')"
-          Condition="'$(MonoBuild)' == 'true'" />
+          Condition="'$(MonoBuild)' == 'true'"
+          SkipUnchangedFiles="true" />
 
     <Copy SourceFiles="@(InstalledStaticAnalysisTools)"
-          DestinationFiles="@(InstalledStaticAnalysisTools -> '$(BootstrapDestination)..\Team Tools\Static Analysis Tools\%(RecursiveDir)%(Filename)%(Extension)')" />
+          DestinationFiles="@(InstalledStaticAnalysisTools -> '$(BootstrapDestination)..\Team Tools\Static Analysis Tools\%(RecursiveDir)%(Filename)%(Extension)')"
+          SkipUnchangedFiles="true" />
 
     <Copy SourceFiles="@(InstalledNuGetFiles)"
-          DestinationFiles="@(InstalledNuGetFiles->'$(BootstrapDestination)Microsoft\NuGet\%(Filename)%(Extension)')" />
+          DestinationFiles="@(InstalledNuGetFiles->'$(BootstrapDestination)Microsoft\NuGet\%(Filename)%(Extension)')"
+          SkipUnchangedFiles="true" />
 
     <Copy Condition="'$(MonoBuild)' != 'true'"
           SourceFiles="@(_NuGetRuntimeDependencies)"
-          DestinationFolder="$(BootstrapDestination)..\Common7\IDE\CommonExtensions\Microsoft\NuGet\" />
+          DestinationFolder="$(BootstrapDestination)..\Common7\IDE\CommonExtensions\Microsoft\NuGet\"
+          SkipUnchangedFiles="true" />
     <Copy Condition="'$(MonoBuild)' == 'true'"
           SourceFiles="@(_NuGetRuntimeDependencies)"
-          DestinationFolder="$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin" />
+          DestinationFolder="$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin" 
+          SkipUnchangedFiles="true" />
 
     <Copy SourceFiles="@(NuGetSdkResolverManifest)"
-          DestinationFolder="$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\SdkResolvers\Microsoft.Build.NuGetSdkResolver" />
+          DestinationFolder="$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\SdkResolvers\Microsoft.Build.NuGetSdkResolver" 
+          SkipUnchangedFiles="true" />
 
     <!-- Delete shim projects, because they point where we can't follow. -->
     <!-- It would be better to just not copy these. -->
     <Delete Files="@(ShimTargets->'$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\%(FileName)%(Extension)')" />
 
     <!-- Copy our binaries -->
     <Copy SourceFiles="@(FreshlyBuiltBinaries)"
-          DestinationFiles="@(FreshlyBuiltBinaries -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\%(RecursiveDir)%(Filename)%(Extension)')" />
+          DestinationFiles="@(FreshlyBuiltBinaries -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\%(RecursiveDir)%(Filename)%(Extension)')" 
+          SkipUnchangedFiles="true" />
+
     <Copy SourceFiles="@(RoslynBinaries)"
-          DestinationFiles="@(RoslynBinaries -> '$(BootstrapDestination)15.0\Bin\Roslyn\%(RecursiveDir)%(Filename)%(Extension)')" />
+          DestinationFiles="@(RoslynBinaries -> '$(BootstrapDestination)15.0\Bin\Roslyn\%(RecursiveDir)%(Filename)%(Extension)')" 
+          SkipUnchangedFiles="true" />
 
     <!-- Copy our binaries to the x64 location. -->
-     <Copy SourceFiles="@(FreshlyBuiltBinariesx64)"
-          DestinationFiles="@(FreshlyBuiltBinariesx64 -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\amd64\%(RecursiveDir)%(Filename)%(Extension)')" />
+    <Copy SourceFiles="@(FreshlyBuiltBinariesx64)"
+          DestinationFiles="@(FreshlyBuiltBinariesx64 -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\amd64\%(RecursiveDir)%(Filename)%(Extension)')" 
+          SkipUnchangedFiles="true" />
 
     <!-- Copy our binaries to the arm64 location. -->
-     <Copy SourceFiles="@(FreshlyBuiltBinariesArm64)"
-          DestinationFiles="@(FreshlyBuiltBinariesArm64 -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\arm64\%(RecursiveDir)%(Filename)%(Extension)')" />
+    <Copy SourceFiles="@(FreshlyBuiltBinariesArm64)"
+          DestinationFiles="@(FreshlyBuiltBinariesArm64 -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\arm64\%(RecursiveDir)%(Filename)%(Extension)')" 
+          SkipUnchangedFiles="true" />
 
     <!-- Copy our freshly-built props and targets, overwriting anything we copied from the machine -->
     <Copy SourceFiles="@(FreshlyBuiltRootProjects)"
-          DestinationFiles="@(FreshlyBuiltRootProjects -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\%(Filename)%(Extension)')" />
+          DestinationFiles="@(FreshlyBuiltRootProjects -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\%(Filename)%(Extension)')" 
+          SkipUnchangedFiles="true" />
     <Copy SourceFiles="@(FreshlyBuiltProjects)"
-          DestinationFiles="@(FreshlyBuiltProjects -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\%(RecursiveDir)%(Filename)%(Extension)')" />
+          DestinationFiles="@(FreshlyBuiltProjects -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\%(RecursiveDir)%(Filename)%(Extension)')" 
+          SkipUnchangedFiles="true" />
     <Copy SourceFiles="@(FreshlyBuiltProjects)"
-          DestinationFiles="@(FreshlyBuiltProjects -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\amd64\%(RecursiveDir)%(Filename)%(Extension)')" />
+          DestinationFiles="@(FreshlyBuiltProjects -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\amd64\%(RecursiveDir)%(Filename)%(Extension)')" 
+          SkipUnchangedFiles="true" />
     <Copy SourceFiles="@(FreshlyBuiltProjects)"
-          DestinationFiles="@(FreshlyBuiltProjects -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\arm64\%(RecursiveDir)%(Filename)%(Extension)')" />
+          DestinationFiles="@(FreshlyBuiltProjects -> '$(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin\arm64\%(RecursiveDir)%(Filename)%(Extension)')" 
+          SkipUnchangedFiles="true" />
 
   </Target>
 

src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs

@@ -236,6 +236,41 @@ Skipping target ""Build"" because all output files are up-to-date with respect t
             }
         }
 
+        [Fact]
+        public void TestErrorForSkippedTargetInputsAndOutputs()
+        {
+            string projectContents = @"
+<Project>
+  <Target Name=""Build"" Inputs=""a.txt;b.txt"" Outputs=""c.txt"">
+    <Message Text=""test"" Importance=""High"" />
+  </Target>
+</Project>";
+
+            using (var env = TestEnvironment.Create())
+            {
+                var buildParameters = new BuildParameters()
+                {
+                    Question = true,
+                };
+
+                using (var buildSession = new Helpers.BuildManagerSession(env, buildParameters))
+                {
+                    var files = env.CreateTestProjectWithFiles(projectContents, new[] { "a.txt", "b.txt", "c.txt" });
+                    var fileA = new FileInfo(files.CreatedFiles[0]);
+                    var fileB = new FileInfo(files.CreatedFiles[1]);
+                    var fileC = new FileInfo(files.CreatedFiles[2]);
+
+                    var now = DateTime.UtcNow;
+                    fileA.LastWriteTimeUtc = now - TimeSpan.FromSeconds(10);
+                    fileB.LastWriteTimeUtc = now + TimeSpan.FromSeconds(10);
+                    fileC.LastWriteTimeUtc = now;
+
+                    var result = buildSession.BuildProjectFile(files.ProjectFile);
+                    result.OverallResult.ShouldBe(BuildResultCode.Failure);
+                }
+            }
+        }
+
         /// <summary>
         /// Ensure that skipped targets only infer outputs once
         /// </summary>

src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs

@@ -572,7 +572,7 @@ private DependencyAnalysisResult PerformDependencyAnalysisTestHelper(
                 ItemBucket itemBucket = new ItemBucket(null, null, new Lookup(itemsByName, new PropertyDictionary<ProjectPropertyInstance>()), 0);
                 TargetUpToDateChecker analyzer = new TargetUpToDateChecker(p, p.Targets["Build"], _mockHost, BuildEventContext.Invalid);
 
-                return analyzer.PerformDependencyAnalysis(itemBucket, out changedTargetInputs, out upToDateTargetInputs);
+                return analyzer.PerformDependencyAnalysis(itemBucket, false, out changedTargetInputs, out upToDateTargetInputs);
             }
             finally
             {

src/Build.UnitTests/BackEnd/TaskBuilder_Tests.cs

@@ -164,7 +164,6 @@ public void CanceledTasksDoNotLogMSB4181()
                     Loggers = new ILogger[] { logger },
                     EnableNodeReuse = false
                 };
-                ;
 
                 BuildRequestData data = new BuildRequestData(project.CreateProjectInstance(), new string[] { "test" }, collection.HostServices);
                 manager.BeginBuild(_parameters);

src/Build/BackEnd/BuildManager/BuildParameters.cs

@@ -205,6 +205,8 @@ public class BuildParameters : ITranslatable
         /// </summary>
         private bool _logInitialPropertiesAndItems;
 
+        private bool _question;
+
         /// <summary>
         /// The settings used to load the project under build
         /// </summary>
@@ -303,6 +305,7 @@ internal BuildParameters(BuildParameters other, bool resetEnvironment = false)
             _outputResultsCacheFile = other._outputResultsCacheFile;
             DiscardBuildResults = other.DiscardBuildResults;
             LowPriority = other.LowPriority;
+            Question = other.Question;
             ProjectCacheDescriptor = other.ProjectCacheDescriptor;
         }
 
@@ -808,6 +811,15 @@ public string OutputResultsCacheFile
         /// </summary>
         public bool LowPriority { get; set; }
 
+        /// <summary>
+        /// Gets or sets a value that will error when the build process fails an incremental check.
+        /// </summary>
+        public bool Question
+        {
+            get => _question;
+            set => _question = value;
+        }
+
         /// <summary>
         /// Gets or sets the project cache description to use for all <see cref="BuildSubmission"/> or <see cref="GraphBuildSubmission"/>
         /// in addition to any potential project caches described in each project.
@@ -871,6 +883,7 @@ void ITranslatable.Translate(ITranslator translator)
             translator.Translate(ref _logInitialPropertiesAndItems);
             translator.TranslateEnum(ref _projectLoadSettings, (int)_projectLoadSettings);
             translator.Translate(ref _interactive);
+            translator.Translate(ref _question);
             translator.TranslateEnum(ref _projectIsolationMode, (int)_projectIsolationMode);
 
             // ProjectRootElementCache is not transmitted.

src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs

@@ -462,7 +462,7 @@ internal async Task ExecuteTarget(ITaskBuilder taskBuilder, BuildRequestEntry re
                         // UNDONE: (Refactor) Refactor TargetUpToDateChecker to take a logging context, not a logging service.
                         MSBuildEventSource.Log.TargetUpToDateStart();
                         TargetUpToDateChecker dependencyAnalyzer = new TargetUpToDateChecker(requestEntry.RequestConfiguration.Project, _target, targetLoggingContext.LoggingService, targetLoggingContext.BuildEventContext);
-                        DependencyAnalysisResult dependencyResult = dependencyAnalyzer.PerformDependencyAnalysis(bucket, out changedTargetInputs, out upToDateTargetInputs);
+                        DependencyAnalysisResult dependencyResult = dependencyAnalyzer.PerformDependencyAnalysis(bucket, _host.BuildParameters.Question, out changedTargetInputs, out upToDateTargetInputs);
                         MSBuildEventSource.Log.TargetUpToDateStop((int)dependencyResult);
 
                         switch (dependencyResult)
@@ -471,6 +471,13 @@ internal async Task ExecuteTarget(ITaskBuilder taskBuilder, BuildRequestEntry re
                             case DependencyAnalysisResult.FullBuild:
                             case DependencyAnalysisResult.IncrementalBuild:
                             case DependencyAnalysisResult.SkipUpToDate:
+                                if (dependencyResult != DependencyAnalysisResult.SkipUpToDate && _host.BuildParameters.Question && !string.IsNullOrEmpty(_target.Inputs) && !string.IsNullOrEmpty(_target.Outputs))
+                                {
+                                    targetSuccess = false;
+                                    aggregateResult = aggregateResult.AggregateResult(new WorkUnitResult(WorkUnitResultCode.Canceled, WorkUnitActionCode.Stop, null));
+                                    break;
+                                }
+
                                 // Create the lookups used to hold the current set of properties and items
                                 lookupForInference = bucket.Lookup;
                                 lookupForExecution = bucket.Lookup.Clone();

src/Build/BackEnd/Components/RequestBuilder/TargetUpToDateChecker.cs

@@ -118,6 +118,7 @@ private string TargetOutputSpecification
         /// incremental build is needed.
         /// </remarks>
         /// <param name="bucket"></param>
+        /// <param name="question"></param>
         /// <param name="changedTargetInputs"></param>
         /// <param name="upToDateTargetInputs"></param>
         /// <returns>
@@ -129,6 +130,7 @@ private string TargetOutputSpecification
         /// </returns>
         internal DependencyAnalysisResult PerformDependencyAnalysis(
             ItemBucket bucket,
+            bool question,
             out ItemDictionary<ProjectItemInstance> changedTargetInputs,
             out ItemDictionary<ProjectItemInstance> upToDateTargetInputs)
         {
@@ -252,7 +254,7 @@ internal DependencyAnalysisResult PerformDependencyAnalysis(
                 }
             }
 
-            LogReasonForBuildingTarget(result);
+            LogReasonForBuildingTarget(result, question);
 
             return result;
         }
@@ -261,15 +263,23 @@ internal DependencyAnalysisResult PerformDependencyAnalysis(
         /// Does appropriate logging to indicate why this target is being built fully or partially.
         /// </summary>
         /// <param name="result"></param>
-        private void LogReasonForBuildingTarget(DependencyAnalysisResult result)
+        /// <param name="question"></param>
+        private void LogReasonForBuildingTarget(DependencyAnalysisResult result, bool question)
         {
             // Only if we are not logging just critical events should we be logging the details
             if (!_loggingService.OnlyLogCriticalEvents)
             {
                 if (result == DependencyAnalysisResult.FullBuild && _dependencyAnalysisDetail.Count > 0)
                 {
-                    // For the full build decision the are three possible outcomes
-                    _loggingService.LogComment(_buildEventContext, MessageImportance.Low, "BuildTargetCompletely", _targetToAnalyze.Name);
+                    if (question)
+                    {
+                        _loggingService.LogError(_buildEventContext, new BuildEventFileInfo(String.Empty), "BuildTargetCompletely", _targetToAnalyze.Name);
+                    }
+                    else
+                    {
+                        // For the full build decision, there are three possible outcomes
+                        _loggingService.LogComment(_buildEventContext, MessageImportance.Low, "BuildTargetCompletely", _targetToAnalyze.Name);
+                    }
 
                     foreach (DependencyAnalysisLogDetail logDetail in _dependencyAnalysisDetail)
                     {
@@ -279,8 +289,15 @@ private void LogReasonForBuildingTarget(DependencyAnalysisResult result)
                 }
                 else if (result == DependencyAnalysisResult.IncrementalBuild)
                 {
-                    // For the partial build decision the are three possible outcomes
-                    _loggingService.LogComment(_buildEventContext, MessageImportance.Normal, "BuildTargetPartially", _targetToAnalyze.Name);
+                    if (question)
+                    {
+                        _loggingService.LogError(_buildEventContext, new BuildEventFileInfo(String.Empty), "BuildTargetPartially", _targetToAnalyze.Name);
+                    }
+                    else
+                    {
+                        // For the partial build decision the are three possible outcomes
+                        _loggingService.LogComment(_buildEventContext, MessageImportance.Normal, "BuildTargetPartially", _targetToAnalyze.Name);
+                    }
                     foreach (DependencyAnalysisLogDetail logDetail in _dependencyAnalysisDetail)
                     {
                         string reason = GetIncrementalBuildReason(logDetail);

src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs

@@ -369,6 +369,11 @@ bool ITaskExecutionHost.SetTaskParameters(IDictionary<string, (string, ElementLo
                 }
             }
 
+            if (this.TaskInstance is IIncrementalTask incrementalTask)
+            {
+                incrementalTask.FailIfNotIncremental = _buildComponentHost.BuildParameters.Question;
+            }
+
             if (taskInitialized)
             {
                 // See if any required properties were not set

src/Directory.Build.targets

@@ -129,7 +129,7 @@
     </ItemGroup>
   </Target>
 
-  <Target Name="CreateTypeLib" BeforeTargets="AfterBuild" Condition="'$(BuildingInsideVisualStudio)' != 'true' and '$(CreateTlb)' == 'true' and $([MSBuild]::IsOSPlatform('windows')) and '$(TargetFrameworkIdentifier)' == '.NETFramework' and '$(MSBuildRuntimeType)' != 'Core'">
+  <Target Name="CreateTypeLib" BeforeTargets="AfterBuild" Inputs="$(TargetPath)" Outputs="$(TargetDir)$(TargetName).tlb;$(TargetDir)x64\$(TargetName).tlb" Condition="'$(BuildingInsideVisualStudio)' != 'true' and '$(CreateTlb)' == 'true' and $([MSBuild]::IsOSPlatform('windows')) and '$(TargetFrameworkIdentifier)' == '.NETFramework' and '$(MSBuildRuntimeType)' != 'Core'">
     <PropertyGroup>
       <TlbExpPath>$([Microsoft.Build.Utilities.ToolLocationHelper]::GetPathToDotNetFrameworkSdkFile('tlbexp.exe'))</TlbExpPath>
       <!-- Provide a mechanism for turning on verbose TlbExp output for diagnosing issues -->

src/Framework/IIncrementalTask.cs

@@ -0,0 +1,19 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#nullable disable
+
+namespace Microsoft.Build.Framework
+{
+    /// <summary>
+    /// Interface for tasks which is supports incrementality.
+    /// </summary>
+    /// <remarks>The tasks implementing this interface should return false to stop the build when in <see cref="FailIfNotIncremental"/> is true and task is not fully incremental.  Try to provide helpful information to diagnose incremental behavior.</remarks>
+    public interface IIncrementalTask
+    {
+        /// <summary>
+        /// Set by MSBuild when Question flag is used.
+        /// </summary>
+        bool FailIfNotIncremental { set; }
+    }
+}