From ba3f31d9cbc745540fc7006e28de5cfeaec929c3 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Thu, 29 Jun 2023 21:44:27 +1000 Subject: [PATCH] SImpler fix for FUTD check of copy items when building for debug The previous fix is fine, however this is slightly more elegant and avoids raising fake events to make things work. In the F5 case, the SBM does fire `IVsUpdateSolutionEvents.UpdateSolution_Begin` before calling the FUTDC, so changing our implementation to use that event means we construct the `SolutionBuildContext` correctly, even when the SBM calls the FUTDC twice. --- .../UpToDateCheckBuildEventNotifier.cs | 5 ++- .../BuildUpToDateCheck.TimestampCache.cs | 8 +++- .../UpToDate/BuildUpToDateCheck.cs | 39 ++++++------------- .../UpToDate/SolutionBuildContext.cs | 1 - 4 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/UpToDate/UpToDateCheckBuildEventNotifier.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/UpToDate/UpToDateCheckBuildEventNotifier.cs index cb7a577f5f4..4ae52875ca9 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/UpToDate/UpToDateCheckBuildEventNotifier.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/UpToDate/UpToDateCheckBuildEventNotifier.cs @@ -151,12 +151,13 @@ private static bool IsBuild(uint options, out bool isRebuild) return null; } - int IVsUpdateSolutionEvents.UpdateSolution_StartUpdate(ref int pfCancelUpdate) + int IVsUpdateSolutionEvents.UpdateSolution_Begin(ref int pfCancelUpdate) { _solutionBuildEventListener.NotifySolutionBuildStarting(); return HResult.OK; } + int IVsUpdateSolutionEvents.UpdateSolution_Done(int fSucceeded, int fModified, int fCancelCommand) { _solutionBuildEventListener.NotifySolutionBuildCompleted(cancelled: false); @@ -174,7 +175,7 @@ int IVsUpdateSolutionEvents.UpdateSolution_Cancel() #region IVsUpdateSolutionEvents stubs int IVsUpdateSolutionEvents.OnActiveProjectCfgChange(IVsHierarchy pIVsHierarchy) => HResult.OK; - int IVsUpdateSolutionEvents.UpdateSolution_Begin(ref int pfCancelUpdate) => HResult.OK; + int IVsUpdateSolutionEvents.UpdateSolution_StartUpdate(ref int pfCancelUpdate) => HResult.OK; int IVsUpdateSolutionEvents2.OnActiveProjectCfgChange(IVsHierarchy pIVsHierarchy) => HResult.OK; int IVsUpdateSolutionEvents2.UpdateSolution_Begin(ref int pfCancelUpdate) => HResult.OK; diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/BuildUpToDateCheck.TimestampCache.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/BuildUpToDateCheck.TimestampCache.cs index 05296544a64..c9f9664b466 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/BuildUpToDateCheck.TimestampCache.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/BuildUpToDateCheck.TimestampCache.cs @@ -6,7 +6,7 @@ namespace Microsoft.VisualStudio.ProjectSystem.UpToDate { internal sealed partial class BuildUpToDateCheck { - internal readonly struct TimestampCache + internal readonly struct TimestampCache : ITimestampCache { private readonly Dictionary _timestampCache = new(StringComparers.Paths); private readonly IFileSystem _fileSystem; @@ -42,6 +42,12 @@ public TimestampCache(IFileSystem fileSystem) return time; } + + public void ClearTimestamps(IEnumerable paths) + { + // This should never be called. + throw new NotImplementedException(); + } } } } diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/BuildUpToDateCheck.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/BuildUpToDateCheck.cs index 087ad6525d9..f18c498b90a 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/BuildUpToDateCheck.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/BuildUpToDateCheck.cs @@ -722,12 +722,8 @@ private bool CheckBuiltFromInputFiles(Log log, in TimestampCache timestampCache, return true; } - private bool CheckCopyToOutputDirectoryItems(Log log, UpToDateCheckImplicitConfiguredInput state, IEnumerable<(string Path, ImmutableArray CopyItems)> copyItemsByProject, ConfiguredFileSystemOperationAggregator fileSystemAggregator, bool? isBuildAccelerationEnabled, CancellationToken token) + private bool CheckCopyToOutputDirectoryItems(Log log, UpToDateCheckImplicitConfiguredInput state, IEnumerable<(string Path, ImmutableArray CopyItems)> copyItemsByProject, ConfiguredFileSystemOperationAggregator fileSystemAggregator, ITimestampCache copyItemTimestampCache, bool? isBuildAccelerationEnabled, CancellationToken token) { - ITimestampCache? timestampCache = _solutionBuildContextProvider.CurrentSolutionBuildContext?.CopyItemTimestamps; - - Assumes.NotNull(timestampCache); - string outputFullPath = Path.Combine(state.MSBuildProjectDirectory, state.OutputRelativeOrFullPath); Log.Scope? scope1 = null; @@ -771,7 +767,7 @@ private bool CheckCopyToOutputDirectoryItems(Log log, UpToDateCheckImplicitConfi log.Verbose(nameof(Resources.FUTD_CheckingCopyToOutputDirectoryItem_1), copyType.ToString()); - DateTime? sourceTime = timestampCache.GetTimestampUtc(sourcePath); + DateTime? sourceTime = copyItemTimestampCache.GetTimestampUtc(sourcePath); if (sourceTime is null) { @@ -784,7 +780,7 @@ private bool CheckCopyToOutputDirectoryItems(Log log, UpToDateCheckImplicitConfi log.Verbose(nameof(Resources.FUTD_SourceFileTimeAndPath_2), sourceTime, sourcePath); - DateTime? destinationTime = timestampCache.GetTimestampUtc(destinationPath); + DateTime? destinationTime = copyItemTimestampCache.GetTimestampUtc(destinationPath); if (destinationTime is null) { @@ -958,26 +954,15 @@ private async Task IsUpToDateInternalAsync( token.ThrowIfCancellationRequested(); // Short-lived cache of timestamp by path - var timestampCache = new TimestampCache(_fileSystem); + TimestampCache timestampCache = new(_fileSystem); - // Ensure we have a context object for the current solution build. - // - // Ordinarily, this is created when the SBM calls ISolutionBuildEventListener.NotifySolutionBuildStarting, - // and cleared again later when the SBM calls ISolutionBuildEventListener.NotifySolutionBuildCompleted. - // - // However there are two cases where it may be null here: - // - // 1. When performing a validation run that continues after the solution build completed, or - // 2. When the build occurs in response to debugging (e.g. F5) in which case the SBM calls the - // FUTDC *before* it invokes any solution build events. - // - // In either case, we construct an event here lazily so that we can correctly test for the - // existence of copy items in CheckCopyToOutputDirectoryItems. - if (_solutionBuildContextProvider.CurrentSolutionBuildContext is null) - { - _solutionBuildEventListener.NotifySolutionBuildStarting(); - Assumes.NotNull(_solutionBuildContextProvider.CurrentSolutionBuildContext); - } + // For copy items, we share a timestamp cache across the solution build. + // It's constructed when the SBM calls IVsUpdateSolutionEvents.UpdateSolution_Begin, + // and cleared when the SBM calls IVsUpdateSolutionEvents.UpdateSolution_Done. + // If the FUTDC is called outside those events, the CurrentSolutionBuildContext will + // be null, so we share the same short-lived cache. This can happen when performing + // validation runs, which can occur after the solution build has completed. + ITimestampCache copyItemTimestampCache = _solutionBuildContextProvider.CurrentSolutionBuildContext?.CopyItemTimestamps ?? timestampCache; globalProperties.TryGetValue(FastUpToDateCheckIgnoresKindsGlobalPropertyName, out string? ignoreKindsString); @@ -1083,7 +1068,7 @@ private async Task IsUpToDateInternalAsync( !CheckInputsAndOutputs(logger, lastSuccessfulBuildStartTimeUtc, timestampCache, implicitState, ignoreKinds, token) || !CheckBuiltFromInputFiles(logger, timestampCache, implicitState, token) || !CheckMarkers(logger, timestampCache, implicitState, isBuildAccelerationEnabled, fileSystemOperations) || - !CheckCopyToOutputDirectoryItems(logger, implicitState, copyInfo.ItemsByProject, configuredFileSystemOperations, isBuildAccelerationEnabled, token)) + !CheckCopyToOutputDirectoryItems(logger, implicitState, copyInfo.ItemsByProject, configuredFileSystemOperations, copyItemTimestampCache, isBuildAccelerationEnabled, token)) { return (false, checkedConfigurations); } diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/SolutionBuildContext.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/SolutionBuildContext.cs index f4af5ccb854..26b67360e8b 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/SolutionBuildContext.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/SolutionBuildContext.cs @@ -10,7 +10,6 @@ namespace Microsoft.VisualStudio.ProjectSystem.UpToDate; /// internal sealed class SolutionBuildContext { - /// /// A cache of timestamps for the absolute paths of both source and target of copy items /// across all projects.