From c988025a8aa49afcfa9d8394122f82e560abb14c Mon Sep 17 00:00:00 2001 From: David Federman Date: Fri, 31 Jul 2026 17:11:50 -0700 Subject: [PATCH 01/13] Add the typed observation schema for probe and enumeration fingerprinting File probes and directory enumerations are filtered out at the application layer, so neither contributes to a fingerprint. A cache entry therefore records no dependency on whether a file existed or on what a directory contained, which is why a lookup can hit after that state has changed. Closing that gap needs a fingerprint entry able to express what was observed rather than only which files were read. These are the types that express it, added ahead of any producer or consumer so the schema can be read on its own. ObservationType is a byte enum with pinned values because it is part of the on-disk PathSet payload. QuickBuild is growing the same feature against the same schema, so the values and the precedence order are fixed on both sides. ObservedPathEntry carries the path, the type, and -- for directory enumerations -- the search pattern and the member lists. Comparison is case-insensitive on the path and on member names to match Windows, and ordinal on the pattern, since a build that asked for `*.CS` made a different request than one that asked for `*.cs` whatever the filesystem then did with it. The constructor normalizes the enumeration-only fields to null on other types so entries that mean the same thing compare equal regardless of what a caller threaded through. ObservationTypePrecedence resolves a path observed several ways in one build: a read tells us more than an enumeration, which tells us more than a probe, so the strongest observation wins. ObservedAccess is the sandbox-side counterpart, holding an absolute path before normalization. --- src/Common.Tests/PathSetTests.cs | 134 ++++++++++++++++ src/Common/FileAccess/ObservedAccess.cs | 20 +++ src/Common/Fingerprinting/ObservationType.cs | 34 ++++ .../ObservationTypePrecedence.cs | 34 ++++ .../Fingerprinting/ObservedPathEntry.cs | 151 ++++++++++++++++++ src/Common/SourceGenerationContext.cs | 2 + 6 files changed, 375 insertions(+) create mode 100644 src/Common.Tests/PathSetTests.cs create mode 100644 src/Common/FileAccess/ObservedAccess.cs create mode 100644 src/Common/Fingerprinting/ObservationType.cs create mode 100644 src/Common/Fingerprinting/ObservationTypePrecedence.cs create mode 100644 src/Common/Fingerprinting/ObservedPathEntry.cs diff --git a/src/Common.Tests/PathSetTests.cs b/src/Common.Tests/PathSetTests.cs new file mode 100644 index 0000000..7063d65 --- /dev/null +++ b/src/Common.Tests/PathSetTests.cs @@ -0,0 +1,134 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Generic; +using Microsoft.MSBuildCache.Fingerprinting; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.MSBuildCache.Tests; + +[TestClass] +public class PathSetTests +{ + [TestMethod] + public void ObservationTypeByteValuesAreStable() + { + // Schema stability: these byte values are part of the on-disk schema and contribute to strong fingerprints. + // The QuickBuild implementation locks the same numeric values; changing either side risks + // semantic divergence in cross-system diagnostics or shared cache scenarios. + Dictionary expected = new() + { + { ObservationType.FileContentRead, 1 }, + { ObservationType.DirectoryEnumeration, 2 }, + { ObservationType.ExistingProbe, 3 }, + { ObservationType.AbsentPathProbe, 4 }, + }; + + foreach (KeyValuePair kvp in expected) + { + Assert.AreEqual(kvp.Value, (byte)kvp.Key, $"Byte value of {kvp.Key} changed; this would invalidate every PathSet on disk."); + } + } + + [TestMethod] + public void ObservationTypePrecedenceOrdering() + { + // Lock the precedence order: FileContentRead > DirectoryEnumeration > ExistingProbe > AbsentPathProbe. + // This is checked against the QuickBuild implementation; both systems must agree. + ObservationType[] descendingPrecedence = + { + ObservationType.FileContentRead, + ObservationType.DirectoryEnumeration, + ObservationType.ExistingProbe, + ObservationType.AbsentPathProbe, + }; + + // Pairwise: every left-of-right pair must yield the left value as Max (higher precedence wins). + for (int i = 0; i < descendingPrecedence.Length; i++) + { + for (int j = i + 1; j < descendingPrecedence.Length; j++) + { + ObservationType higher = descendingPrecedence[i]; + ObservationType lower = descendingPrecedence[j]; + Assert.AreEqual(higher, ObservationTypePrecedence.Max(higher, lower), $"Max({higher}, {lower})"); + Assert.AreEqual(higher, ObservationTypePrecedence.Max(lower, higher), $"Max({lower}, {higher}) (commutative)"); + } + } + + // Identity: Max(x, x) == x for all x. + foreach (ObservationType t in descendingPrecedence) + { + Assert.AreEqual(t, ObservationTypePrecedence.Max(t, t)); + } + } + + [TestMethod] + public void EnumerationPatternNullForNonDirectoryEnumeration() + { + // Pattern is meaningful only for DirectoryEnumeration. The constructor normalizes anything else to null + // so that semantically-equivalent observations compare equal regardless of pattern threading. + Assert.IsNull(new ObservedPathEntry("p", ObservationType.FileContentRead, "*.cs").EnumerationPattern); + Assert.IsNull(new ObservedPathEntry("p", ObservationType.ExistingProbe, "*.cs").EnumerationPattern); + Assert.IsNull(new ObservedPathEntry("p", ObservationType.AbsentPathProbe, "*.cs").EnumerationPattern); + Assert.AreEqual("*.cs", new ObservedPathEntry("p", ObservationType.DirectoryEnumeration, "*.cs").EnumerationPattern); + } + + // ----------------------------------------------------------------------------------------- + // Schema: DirectoryEnumeration Members + WrittenMembers. + // ----------------------------------------------------------------------------------------- + + [TestMethod] + public void EqualsIdenticalDirectoryEnumerationWithMembers() + { + var a = new ObservedPathEntry("dir/", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs", "b.cs" }, + writtenMembers: new[] { "Foo.dll" }); + var b = new ObservedPathEntry("dir/", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs", "b.cs" }, + writtenMembers: new[] { "Foo.dll" }); + + Assert.IsTrue(a.Equals(b)); + Assert.AreEqual(a.GetHashCode(), b.GetHashCode()); + } + + [TestMethod] + public void NotEqualWhenMembersDiffer() + { + var a = new ObservedPathEntry("dir/", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs" }, + writtenMembers: null); + var b = new ObservedPathEntry("dir/", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs", "b.cs" }, + writtenMembers: null); + + Assert.IsFalse(a.Equals(b)); + } + + [TestMethod] + public void NotEqualWhenWrittenMembersDiffer() + { + var a = new ObservedPathEntry("dir/", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs" }, + writtenMembers: new[] { "Foo.dll" }); + var b = new ObservedPathEntry("dir/", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs" }, + writtenMembers: new[] { "Bar.dll" }); + + Assert.IsFalse(a.Equals(b)); + } + + [TestMethod] + public void MembersAndWrittenMembersOnlyApplyToDirectoryEnumeration() + { + // The constructor normalizes Members and WrittenMembers to null on non-DirEnum types, + // mirroring EnumerationPattern's normalization. Ensures semantically-equivalent observations + // compare equal regardless of whether a stray Members value was threaded through. + var fcr = new ObservedPathEntry("p", ObservationType.FileContentRead, enumerationPattern: "*.cs", + members: new[] { "should-be-ignored" }, + writtenMembers: new[] { "should-be-ignored" }); + + Assert.IsNull(fcr.Members); + Assert.IsNull(fcr.WrittenMembers); + Assert.IsNull(fcr.EnumerationPattern); + } +} diff --git a/src/Common/FileAccess/ObservedAccess.cs b/src/Common/FileAccess/ObservedAccess.cs new file mode 100644 index 0000000..7e98618 --- /dev/null +++ b/src/Common/FileAccess/ObservedAccess.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Generic; +using Microsoft.MSBuildCache.Fingerprinting; + +namespace Microsoft.MSBuildCache.FileAccess; + +/// +/// A single typed sandbox observation that contributes (subject to filtering) to the strong +/// fingerprint as an . Created by as +/// probe and enumeration events arrive from MSBuild; consumed by 's +/// GetPathSet. +/// +public sealed record ObservedAccess( + string Path, + ObservationType Type, + string? EnumerationPattern = null, + IReadOnlyList? Members = null, + IReadOnlyList? WrittenMembers = null); diff --git a/src/Common/Fingerprinting/ObservationType.cs b/src/Common/Fingerprinting/ObservationType.cs new file mode 100644 index 0000000..60c6381 --- /dev/null +++ b/src/Common/Fingerprinting/ObservationType.cs @@ -0,0 +1,34 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.MSBuildCache.Fingerprinting; + +/// +/// Classifies how a path was observed during a build, so that the strong fingerprint can be re-derived +/// from the right kind of filesystem evidence (content hash, directory membership, or just existence). +/// +/// +/// Byte values are part of the on-disk schema and must remain stable. The numeric ordering also defines +/// type-precedence — lower value wins when the same path is observed multiple times. +/// +#pragma warning disable CA1008 // Enums should have zero value. Schema-stable enum; 0 has no defined observation semantics and adding a synthetic None would waste a byte value in a deliberately-compact schema. +#pragma warning disable CA1028 // Enum storage should be Int32. Schema-stable enum serialized into PathSet and contributing to the strong fingerprint; the byte width is intentional and must match the QuickBuild implementation. +public enum ObservationType : byte +#pragma warning restore CA1028 +#pragma warning restore CA1008 +{ + /// The file's content was read; cache depends on the content hash. + FileContentRead = 1, + + /// A directory was enumerated; cache depends on the (filtered) member list. + DirectoryEnumeration = 2, + + /// A path was probed and found to exist; cache depends on its existence + /// (but not on file content or directory membership). + ExistingProbe = 3, + + /// A path was probed but did not exist; cache depends on its absence. + /// This is the keystone of correct clean→dirty→miss caching: if the path later appears, the strong + /// fingerprint differs and the cache correctly misses. + AbsentPathProbe = 4, +} diff --git a/src/Common/Fingerprinting/ObservationTypePrecedence.cs b/src/Common/Fingerprinting/ObservationTypePrecedence.cs new file mode 100644 index 0000000..e5b6cc3 --- /dev/null +++ b/src/Common/Fingerprinting/ObservationTypePrecedence.cs @@ -0,0 +1,34 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.MSBuildCache.Fingerprinting; + +/// +/// Helpers for working with precedence when the same path is observed +/// multiple times in a single build. +/// +/// +/// Precedence order: FileContentRead > DirectoryEnumeration > ExistingProbe > AbsentPathProbe +/// — the most specific observation wins. Use this helper rather than relying on the underlying byte +/// values so precedence remains decoupled from the schema. +/// +internal static class ObservationTypePrecedence +{ + /// Returns whichever of and has higher precedence. + public static ObservationType Max(ObservationType a, ObservationType b) + => Rank(a) <= Rank(b) ? a : b; + + /// + /// Returns the precedence rank of an observation type. Lower rank = higher precedence + /// (matches the byte value, but callers must not rely on that — go through this helper). + /// + public static int Rank(ObservationType type) + => type switch + { + ObservationType.FileContentRead => 1, + ObservationType.DirectoryEnumeration => 2, + ObservationType.ExistingProbe => 3, + ObservationType.AbsentPathProbe => 4, + _ => int.MaxValue, // Unknown type — treat as lowest precedence (will lose every fold). + }; +} diff --git a/src/Common/Fingerprinting/ObservedPathEntry.cs b/src/Common/Fingerprinting/ObservedPathEntry.cs new file mode 100644 index 0000000..c802df4 --- /dev/null +++ b/src/Common/Fingerprinting/ObservedPathEntry.cs @@ -0,0 +1,151 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.Text.Json.Serialization; + +namespace Microsoft.MSBuildCache.Fingerprinting; + +/// +/// A single typed observation in a . +/// +/// +/// Equality and hashing use case-insensitive comparison on (Windows-first), ordinal +/// comparison on , and case-insensitive ordered comparison on +/// and . +/// +public sealed class ObservedPathEntry : IEquatable +{ + [JsonConstructor] + public ObservedPathEntry( + string path, + ObservationType type, + string? enumerationPattern = null, + IReadOnlyList? members = null, + IReadOnlyList? writtenMembers = null) + { + Path = path ?? throw new ArgumentNullException(nameof(path)); + Type = type; + + // EnumerationPattern, Members, and WrittenMembers are meaningful only for DirectoryEnumeration + // observations; normalize to null on other types so that semantically-equivalent observations + // compare equal regardless of whether a caller happened to thread stray values through. + bool isDirEnum = type == ObservationType.DirectoryEnumeration; + EnumerationPattern = isDirEnum ? enumerationPattern : null; + Members = isDirEnum ? members : null; + WrittenMembers = isDirEnum ? writtenMembers : null; + } + + /// The observed path. Normalization (e.g., repo/package-relative form) is the producer's responsibility. + public string Path { get; } + + /// The kind of observation. Determines how the entry contributes to the strong fingerprint + /// and how it is re-observed at cache lookup. + public ObservationType Type { get; } + + /// + /// Search pattern used for the enumeration (e.g., *.cs), or null for unfiltered enumerations + /// and for non- entries. + /// + public string? EnumerationPattern { get; } + + /// + /// For entries: the populate-time member-name list + /// (leaf names, sorted OrdinalIgnoreCase) with self-outputs subtracted. This is the project's + /// external view of the directory's membership — the part that can legitimately invalidate the cache + /// when it changes. Hashed directly into the strong fingerprint, making the strong-FP deterministic + /// from the entry without needing to touch the filesystem at compute time. Null for non-DirEnum + /// entries and for entries where the directory was missing/inaccessible at populate time. + /// + public IReadOnlyList? Members { get; } + + /// + /// For entries: the populate-time self-output + /// member-name list (leaf names, sorted OrdinalIgnoreCase) — the names that this project + /// itself wrote into the directory during the populate build, identified via the everWritten + /// set. At lookup time we subtract this from the re-enumerated current contents before comparing + /// against , so cache hits succeed regardless of whether the previous build's + /// outputs are still on disk (the deterministic-build assumption: the build would re-write the same + /// names). Null for non-DirEnum entries. + /// + public IReadOnlyList? WrittenMembers { get; } + + public bool Equals(ObservedPathEntry? other) + { + if (ReferenceEquals(this, other)) + { + return true; + } + + if (other is null) + { + return false; + } + + return Type == other.Type + && string.Equals(Path, other.Path, StringComparison.OrdinalIgnoreCase) + && string.Equals(EnumerationPattern, other.EnumerationPattern, StringComparison.Ordinal) + && SequenceEqualsOIC(Members, other.Members) + && SequenceEqualsOIC(WrittenMembers, other.WrittenMembers); + } + + public override bool Equals(object? obj) => Equals(obj as ObservedPathEntry); + + public override int GetHashCode() + { + var hashCode = default(HashCode); + hashCode.Add(Path, StringComparer.OrdinalIgnoreCase); + hashCode.Add((byte)Type); + if (EnumerationPattern is not null) + { + hashCode.Add(EnumerationPattern, StringComparer.Ordinal); + } + + if (Members is not null) + { + foreach (string m in Members) + { + hashCode.Add(m, StringComparer.OrdinalIgnoreCase); + } + } + + if (WrittenMembers is not null) + { + foreach (string m in WrittenMembers) + { + hashCode.Add(m, StringComparer.OrdinalIgnoreCase); + } + } + + return hashCode.ToHashCode(); + } + + private static bool SequenceEqualsOIC(IReadOnlyList? a, IReadOnlyList? b) + { + if (ReferenceEquals(a, b)) + { + return true; + } + + if (a is null || b is null) + { + return false; + } + + if (a.Count != b.Count) + { + return false; + } + + for (int i = 0; i < a.Count; i++) + { + if (!string.Equals(a[i], b[i], StringComparison.OrdinalIgnoreCase)) + { + return false; + } + } + + return true; + } +} diff --git a/src/Common/SourceGenerationContext.cs b/src/Common/SourceGenerationContext.cs index cdf4089..0cde5e3 100644 --- a/src/Common/SourceGenerationContext.cs +++ b/src/Common/SourceGenerationContext.cs @@ -12,6 +12,8 @@ namespace Microsoft.MSBuildCache; [JsonSourceGenerationOptions(WriteIndented = true, Converters = [typeof(ContentHashJsonConverter), typeof(SortedDictionaryConverter)])] [JsonSerializable(typeof(NodeBuildResult))] [JsonSerializable(typeof(PathSet))] +[JsonSerializable(typeof(ObservedPathEntry))] +[JsonSerializable(typeof(ObservationType))] [JsonSerializable(typeof(LocalCacheStateFile))] [JsonSerializable(typeof(IDictionary))] internal partial class SourceGenerationContext : JsonSerializerContext From fb217c6930b866520957378667b9e8bc67ea3305 Mon Sep 17 00:00:00 2001 From: David Federman Date: Fri, 31 Jul 2026 17:17:49 -0700 Subject: [PATCH 02/13] Route observed file reads through the typed schema PathSet holds a list of normalized paths, which cannot express anything beyond "this file was read". Replacing it with the ObservedPathEntry list is the structural change the feature needs, and doing it before anything new is observed keeps it reviewable on its own. Nothing new is captured here. The sandbox still filters probes and enumerations out, so every entry produced is a FileContentRead. This schema transition intentionally invalidates pre-feature cache entries: PathSet changes from FilesRead to Entries, the feature setting joins the weak fingerprint, and file reads gain a typed identity in the strong fingerprint. Existing entries degrade to cache misses and age out through normal eviction. The rules that only bind once probes and enumerations do arrive come with it, because they are what the entry list is for and splitting them out would leave a data structure with no semantics: * FilterObservations decides what reaches the PathSet. Predicted inputs and IgnoredInputPatterns matches are dropped as before, and file reads are still gated on the input hasher recognizing the path. Probes and enumerations must additionally normalize under the repo or package root: a path under neither is not something the cache can reason about, and that is where most of the volume and all of the machine-specific noise sits. * FoldPathSetEntries canonicalizes several observations of one path by the precedence order, while keeping enumerations of the same directory under different search patterns as separate entries rather than collapsing them. * The strong fingerprint gains a payload per type. A directory enumeration hashes the member list carried on the entry rather than reading the filesystem, which is what makes the value reproducible at lookup from the cached entry alone. EnableProbeAndEnumerationFingerprinting arrives off. It contributes to the weak fingerprint so entries produced with and without it never mix. PathSet also normalizes a null entry list to empty. It is used as a dictionary key during lookup, so a payload lacking the property would be hashed -- and throw -- before any caller could check it. Degrading to a miss is what the null handling downstream already assumed. --- src/Common.Tests/FingerprintFactoryTests.cs | 836 ++++++++++++++++++ src/Common.Tests/PathNormalizerTests.cs | 76 ++ src/Common.Tests/PathSetTests.cs | 168 ++++ src/Common/FileAccess/FileAccessRepository.cs | 7 +- src/Common/FileAccess/FileAccesses.cs | 15 +- .../Fingerprinting/FingerprintFactory.cs | 357 +++++++- .../Fingerprinting/IFingerprintFactory.cs | 3 +- src/Common/Fingerprinting/PathSet.cs | 27 +- src/Common/MSBuildCachePluginBase.cs | 12 +- src/Common/PathNormalizer.cs | 8 + src/Common/PluginSettings.cs | 11 + 11 files changed, 1465 insertions(+), 55 deletions(-) create mode 100644 src/Common.Tests/FingerprintFactoryTests.cs create mode 100644 src/Common.Tests/PathNormalizerTests.cs diff --git a/src/Common.Tests/FingerprintFactoryTests.cs b/src/Common.Tests/FingerprintFactoryTests.cs new file mode 100644 index 0000000..3621751 --- /dev/null +++ b/src/Common.Tests/FingerprintFactoryTests.cs @@ -0,0 +1,836 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Threading.Tasks; +using BuildXL.Cache.ContentStore.Hashing; +using DotNet.Globbing; +using Microsoft.MSBuildCache.FileAccess; +using Microsoft.MSBuildCache.Fingerprinting; +using Microsoft.MSBuildCache.Hashing; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.MSBuildCache.Tests; + +[TestClass] +public class FingerprintFactoryTests +{ + /// + /// Verifies that toggling changes the + /// plugin-settings fingerprint entries that fold into the weak fingerprint. This is the cache-entry + /// segregation mechanism: flag-on and flag-off builds end up under different weak fingerprints and therefore + /// don't pollute each other's selectors. + /// + [TestMethod] + public void WeakFingerprintFlagSegregation() + { + using IContentHasher contentHasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + IInputHasher inputHasher = new NoopInputHasher(); + var pathNormalizer = new PathNormalizer(@"X:\Repo", @"X:\Nuget"); + + PluginSettings off = new TestPluginSettings { RepoRoot = @"X:\Repo", EnableProbeAndEnumerationFingerprinting = false }; + PluginSettings on = new TestPluginSettings { RepoRoot = @"X:\Repo", EnableProbeAndEnumerationFingerprinting = true }; + + var factoryOff = new FingerprintFactory(contentHasher, inputHasher, off, pathNormalizer); + var factoryOn = new FingerprintFactory(contentHasher, inputHasher, on, pathNormalizer); + + List entriesOff = GetPluginSettingsFingerprintEntries(factoryOff); + List entriesOn = GetPluginSettingsFingerprintEntries(factoryOn); + + // The two collections must differ — specifically, exactly one entry differs (the flag value entry). + FingerprintEntry offFlagEntry = entriesOff.Single(e => e.Description.Contains(nameof(PluginSettings.EnableProbeAndEnumerationFingerprinting), StringComparison.Ordinal)); + FingerprintEntry onFlagEntry = entriesOn.Single(e => e.Description.Contains(nameof(PluginSettings.EnableProbeAndEnumerationFingerprinting), StringComparison.Ordinal)); + + Assert.AreNotEqual(offFlagEntry.Description, onFlagEntry.Description, "Flag descriptions should differ."); + CollectionAssert.AreNotEqual(offFlagEntry.Hash, onFlagEntry.Hash, "Flag entry hashes should differ."); + + // And no other entry should differ between the two factories — the flag is the only setting toggled. + List commonOff = entriesOff + .Where(e => !e.Description.Contains(nameof(PluginSettings.EnableProbeAndEnumerationFingerprinting), StringComparison.Ordinal)) + .Select(e => e.Description).ToList(); + List commonOn = entriesOn + .Where(e => !e.Description.Contains(nameof(PluginSettings.EnableProbeAndEnumerationFingerprinting), StringComparison.Ordinal)) + .Select(e => e.Description).ToList(); + CollectionAssert.AreEqual(commonOff, commonOn); + } + + /// + /// Sentinels are derived from the configured so they're the right + /// byte length for the active . Two factory instances using the same hash type must + /// produce bit-identical sentinels — that's the cross-machine stability guarantee that lets the type-aware + /// strong fingerprint be reproducible. + /// + [TestMethod] + [DataRow(HashType.Murmur)] + [DataRow(HashType.Vso0)] + [DataRow(HashType.SHA256)] + public void SentinelsAreDeterministicForHashType(HashType hashType) + { + using IContentHasher hasher = HashInfoLookup.Find(hashType).CreateContentHasher(); + FingerprintFactory a = CreateFactory(hasher); + FingerprintFactory b = CreateFactory(hasher); + + CollectionAssert.AreEqual(a.AbsentFileSentinel, b.AbsentFileSentinel, $"AbsentFileSentinel not deterministic for {hashType}."); + CollectionAssert.AreEqual(a.ZeroHash, b.ZeroHash, $"ZeroHash not deterministic for {hashType}."); + } + + /// + /// The two sentinels must be distinct — they encode different semantic states (absent path vs. + /// existence-only observation) and must never collapse to the same fingerprint payload. + /// + [TestMethod] + [DataRow(HashType.Murmur)] + [DataRow(HashType.Vso0)] + [DataRow(HashType.SHA256)] + public void SentinelsAreDistinct(HashType hashType) + { + using IContentHasher hasher = HashInfoLookup.Find(hashType).CreateContentHasher(); + FingerprintFactory factory = CreateFactory(hasher); + + CollectionAssert.AreNotEqual(factory.AbsentFileSentinel, factory.ZeroHash, "AbsentFileSentinel and ZeroHash must not collide."); + } + + /// + /// Sentinels must match the active hasher's output byte length. This is essential for downstream + /// hash-combining calls that concatenate sentinel + real hashes into the final strong fingerprint. + /// + [TestMethod] + [DataRow(HashType.Murmur)] + [DataRow(HashType.Vso0)] + [DataRow(HashType.SHA256)] + public void SentinelsMatchHasherByteLength(HashType hashType) + { + using IContentHasher hasher = HashInfoLookup.Find(hashType).CreateContentHasher(); + int expectedLength = hasher.Info.ByteLength; + FingerprintFactory factory = CreateFactory(hasher); + + Assert.AreEqual(expectedLength, factory.AbsentFileSentinel.Length, $"AbsentFileSentinel size mismatch for {hashType}."); + Assert.AreEqual(expectedLength, factory.ZeroHash.Length, $"ZeroHash size mismatch for {hashType}."); + } + + /// + /// AbsentPathProbe and ExistingProbe of the same path must produce different strong fingerprints. + /// This is the clean→dirty→miss / dirty→clean→miss fix at the strong-FP layer — re-observation at + /// cache lookup will surface these type differences at lookup time. + /// + [TestMethod] + public async Task StrongFingerprintDistinguishesAbsentFromExistent() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + FingerprintFactory absentFactory = CreateFactory(hasher); + FingerprintFactory existingFactory = CreateFactory(hasher); + + PathSet absent = new(new List { new(@"{RepoRoot}\foo", ObservationType.AbsentPathProbe) }); + PathSet existing = new(new List { new(@"{RepoRoot}\foo", ObservationType.ExistingProbe) }); + + Fingerprint? fpAbsent = await absentFactory.GetStrongFingerprintAsync(absent); + Fingerprint? fpExisting = await existingFactory.GetStrongFingerprintAsync(existing); + + Assert.IsNotNull(fpAbsent); + Assert.IsNotNull(fpExisting); + CollectionAssert.AreNotEqual(fpAbsent.Hash, fpExisting.Hash, + "AbsentPathProbe and ExistingProbe of the same path must produce different strong fingerprints."); + } + + /// + /// FileContentRead and ExistingProbe of the same path must produce different strong fingerprints. + /// Reading a file's content is strictly more information than probing for its existence; the strong + /// fingerprint must reflect this distinction. + /// + [TestMethod] + public async Task StrongFingerprintDistinguishesProbeFromContentRead() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var inputHasher = new DictInputHasher(new Dictionary(StringComparer.OrdinalIgnoreCase) + { + [@"X:\Repo\foo"] = new byte[] { 1, 2, 3 }, + }); + FingerprintFactory factory = CreateFactory(hasher, inputHasher); + + // Need at least one non-FCR entry to force the type-aware encoding path. Both PathSets carry the + // same dummy AbsentPathProbe entry; the FCR-vs-Probe distinction is on the path under test. + PathSet probe = new(new List + { + new(@"{RepoRoot}\bar", ObservationType.AbsentPathProbe), + new(@"{RepoRoot}\foo", ObservationType.ExistingProbe), + }); + PathSet read = new(new List + { + new(@"{RepoRoot}\bar", ObservationType.AbsentPathProbe), + new(@"{RepoRoot}\foo", ObservationType.FileContentRead), + }); + + Fingerprint? fpProbe = await CreateFactory(hasher, inputHasher).GetStrongFingerprintAsync(probe); + Fingerprint? fpRead = await CreateFactory(hasher, inputHasher).GetStrongFingerprintAsync(read); + + Assert.IsNotNull(fpProbe); + Assert.IsNotNull(fpRead); + CollectionAssert.AreNotEqual(fpProbe.Hash, fpRead.Hash); + } + + /// + /// When an ExistingProbe path's content changes but the path still exists, the strong fingerprint + /// must NOT change. Existence-only observations don't depend on content. + /// + [TestMethod] + public async Task StrongFingerprintExistingProbeIgnoresContentChange() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + + // Two different input hashers with different content hashes for the same path. ExistingProbe entries + // ignore the hasher; the strong fingerprint should be identical regardless. + var hasherA = new DictInputHasher(new Dictionary(StringComparer.OrdinalIgnoreCase) { [@"X:\Repo\foo"] = new byte[] { 1 } }); + var hasherB = new DictInputHasher(new Dictionary(StringComparer.OrdinalIgnoreCase) { [@"X:\Repo\foo"] = new byte[] { 2 } }); + + // Force the type-aware encoding by including a non-FCR sibling entry. + PathSet pathSet = new(new List + { + new(@"{RepoRoot}\bar", ObservationType.AbsentPathProbe), + new(@"{RepoRoot}\foo", ObservationType.ExistingProbe), + }); + + Fingerprint? fpA = await CreateFactory(hasher, hasherA).GetStrongFingerprintAsync(pathSet); + Fingerprint? fpB = await CreateFactory(hasher, hasherB).GetStrongFingerprintAsync(pathSet); + + Assert.IsNotNull(fpA); + Assert.IsNotNull(fpB); + CollectionAssert.AreEqual(fpA.Hash, fpB.Hash, + "ExistingProbe must NOT incorporate file content into the strong fingerprint."); + } + + /// + /// Adding a member to an enumerated directory must change the strong fingerprint. Under the + /// schema-driven model the strong-FP comes from the entry's Members field directly, + /// so two PathSets that differ only in their Members lists produce different fingerprints. + /// + [TestMethod] + public async Task StrongFingerprintDirectoryMemberHashDetectsAddition() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(@"X:\Repo", @"X:\Nuget"); + + PathSet before = new(new List + { + new(@"{RepoRoot}\sentinel", ObservationType.AbsentPathProbe), + new(@"{RepoRoot}\dir", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs" }, + writtenMembers: Array.Empty()), + }); + PathSet after = new(new List + { + new(@"{RepoRoot}\sentinel", ObservationType.AbsentPathProbe), + new(@"{RepoRoot}\dir", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs", "b.cs" }, + writtenMembers: Array.Empty()), + }); + + Fingerprint? fpBefore = await CreateFactory(hasher, pathNormalizer: pathNormalizer).GetStrongFingerprintAsync(before); + Fingerprint? fpAfter = await CreateFactory(hasher, pathNormalizer: pathNormalizer).GetStrongFingerprintAsync(after); + + Assert.IsNotNull(fpBefore); + Assert.IsNotNull(fpAfter); + CollectionAssert.AreNotEqual(fpBefore.Hash, fpAfter.Hash, + "PathSets that differ only in DirectoryEnumeration.Members must produce different strong fingerprints."); + } + + /// + /// Removing a member from an enumerated directory must change the strong fingerprint. Schema-driven + /// counterpart of the addition test. + /// + [TestMethod] + public async Task StrongFingerprintDirectoryMemberHashDetectsRemoval() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(@"X:\Repo", @"X:\Nuget"); + + PathSet before = new(new List + { + new(@"{RepoRoot}\sentinel", ObservationType.AbsentPathProbe), + new(@"{RepoRoot}\dir", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs", "b.cs" }, + writtenMembers: Array.Empty()), + }); + PathSet after = new(new List + { + new(@"{RepoRoot}\sentinel", ObservationType.AbsentPathProbe), + new(@"{RepoRoot}\dir", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs" }, + writtenMembers: Array.Empty()), + }); + + Fingerprint? fpBefore = await CreateFactory(hasher, pathNormalizer: pathNormalizer).GetStrongFingerprintAsync(before); + Fingerprint? fpAfter = await CreateFactory(hasher, pathNormalizer: pathNormalizer).GetStrongFingerprintAsync(after); + + Assert.IsNotNull(fpBefore); + Assert.IsNotNull(fpAfter); + CollectionAssert.AreNotEqual(fpBefore.Hash, fpAfter.Hash); + } + + /// + /// Changing the content of a directory member (without changing the member list) must NOT change the + /// strong fingerprint when the entry is DirectoryEnumeration. The enumeration only depends on the + /// member-name list (carried in the entry's Members); file content is irrelevant. + /// + [TestMethod] + public async Task StrongFingerprintDirectoryMemberHashIgnoresMemberContentChange() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(@"X:\Repo", @"X:\Nuget"); + + // Two PathSets with the same Members list. Member file content is not represented in the schema + // for DirEnum entries, so two PathSets with the same Members must produce identical strong FPs. + PathSet before = new(new List + { + new(@"{RepoRoot}\sentinel", ObservationType.AbsentPathProbe), + new(@"{RepoRoot}\dir", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs" }, + writtenMembers: Array.Empty()), + }); + PathSet after = new(new List + { + new(@"{RepoRoot}\sentinel", ObservationType.AbsentPathProbe), + new(@"{RepoRoot}\dir", ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "a.cs" }, + writtenMembers: Array.Empty()), + }); + + Fingerprint? fpBefore = await CreateFactory(hasher, pathNormalizer: pathNormalizer).GetStrongFingerprintAsync(before); + Fingerprint? fpAfter = await CreateFactory(hasher, pathNormalizer: pathNormalizer).GetStrongFingerprintAsync(after); + + Assert.IsNotNull(fpBefore); + Assert.IsNotNull(fpAfter); + CollectionAssert.AreEqual(fpBefore.Hash, fpAfter.Hash, + "DirectoryEnumeration must NOT depend on member file contents — only on the member-name list."); + } + + /// + /// ComputeDirectoryMemberHash: missing directory → AbsentFileSentinel (verified indirectly by + /// fingerprinting a PathSet with a DirectoryEnumeration of a missing path). + /// + [TestMethod] + public async Task StrongFingerprintDirectoryEnumerationMissingDirIsAbsent() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(@"X:\NonExistentRepo", @"X:\Nuget"); + + PathSet pathSetMissing = new(new List + { + new(@"{RepoRoot}\sentinel", ObservationType.AbsentPathProbe), + new(@"{RepoRoot}\definitely-not-here", ObservationType.DirectoryEnumeration), + }); + PathSet pathSetAbsent = new(new List + { + new(@"{RepoRoot}\sentinel", ObservationType.AbsentPathProbe), + new(@"{RepoRoot}\definitely-not-here", ObservationType.AbsentPathProbe), + }); + + Fingerprint? fpMissingDir = await CreateFactory(hasher, pathNormalizer: pathNormalizer).GetStrongFingerprintAsync(pathSetMissing); + Fingerprint? fpAbsent = await CreateFactory(hasher, pathNormalizer: pathNormalizer).GetStrongFingerprintAsync(pathSetAbsent); + + Assert.IsNotNull(fpMissingDir); + Assert.IsNotNull(fpAbsent); + // The two should still differ because the Type tag differs, but the underlying member-hash payload + // for the missing-directory case is AbsentFileSentinel (same payload bytes as the AbsentPathProbe). + // Different Type entries → different overall fingerprint. + CollectionAssert.AreNotEqual(fpMissingDir.Hash, fpAbsent.Hash); + } + + /// + /// Determinism: hashing the same PathSet twice produces the same strong fingerprint, even with member + /// hashing (no nondeterministic FS enumeration ordering leaking through). + /// + [TestMethod] + public async Task StrongFingerprintIsDeterministic() + { + using TempDirectory tempRepo = TempDirectory.Create("MSBuildCacheTest-Determinism"); + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(tempRepo.Path, @"X:\Nuget"); + + // Populate enough members to potentially exercise FS-ordering nondeterminism. + foreach (string name in new[] { "z.cs", "a.cs", "M.cs", "b.cs", "Y.txt" }) + { + WriteTextSync(Path.Combine(tempRepo.Path, name), ""); + } + + string normalizedDir = pathNormalizer.Normalize(tempRepo.Path); + PathSet pathSet = new(new List + { + new(@"{RepoRoot}\sentinel", ObservationType.AbsentPathProbe), + new(normalizedDir, ObservationType.DirectoryEnumeration), + }); + + Fingerprint? fp1 = await CreateFactory(hasher, pathNormalizer: pathNormalizer).GetStrongFingerprintAsync(pathSet); + Fingerprint? fp2 = await CreateFactory(hasher, pathNormalizer: pathNormalizer).GetStrongFingerprintAsync(pathSet); + + Assert.IsNotNull(fp1); + Assert.IsNotNull(fp2); + CollectionAssert.AreEqual(fp1.Hash, fp2.Hash, + "Strong fingerprint must be deterministic across factory instances."); + } + + // ========================================================================================= + // Same-path same-type pattern-fold (multi-pattern handling). + // + // Tests pin the precedence rule plus the multi-pattern handling for DirectoryEnumeration entries — + // different patterns are kept as distinct entries, identical patterns dedupe. + // ========================================================================================= + + /// + /// Flag off: non-FCR observations are dropped; only FCR entries contribute. + /// + [TestMethod] + public void FoldPathSetEntriesFlagOffIgnoresObservations() + { + List entries = FingerprintFactory.FoldPathSetEntries( + observations: new[] + { + new ObservedPathEntry("a.cs", ObservationType.FileContentRead), + new ObservedPathEntry("b.cs", ObservationType.ExistingProbe), + }, + enableProbeAndEnumeration: false); + + Assert.AreEqual(1, entries.Count); + Assert.AreEqual("a.cs", entries[0].Path); + Assert.AreEqual(ObservationType.FileContentRead, entries[0].Type); + } + + /// + /// FCR plus a probe of the same path: precedence keeps the FCR (FileContentRead > ExistingProbe). + /// + [TestMethod] + public void FoldPathSetEntriesPrecedenceKeepsContentReadOverProbe() + { + List entries = FingerprintFactory.FoldPathSetEntries( + observations: new[] + { + new ObservedPathEntry("shared.dll", ObservationType.FileContentRead), + new ObservedPathEntry("shared.dll", ObservationType.ExistingProbe), + }, + enableProbeAndEnumeration: true); + + Assert.AreEqual(1, entries.Count); + Assert.AreEqual(ObservationType.FileContentRead, entries[0].Type); + } + + /// + /// Probe-first then read: the FCR observation strictly outranks the previously-folded probe, so the + /// final entry is FCR. + /// + [TestMethod] + public void FoldPathSetEntriesPrecedenceReplacesProbeWithRead() + { + List entries = FingerprintFactory.FoldPathSetEntries( + observations: new[] + { + new ObservedPathEntry("foo", ObservationType.AbsentPathProbe), + new ObservedPathEntry("foo", ObservationType.ExistingProbe), + new ObservedPathEntry("foo", ObservationType.FileContentRead), + }, + enableProbeAndEnumeration: true); + + Assert.AreEqual(1, entries.Count); + Assert.AreEqual(ObservationType.FileContentRead, entries[0].Type); + } + + /// + /// Same-path DirectoryEnumeration entries with different patterns must each appear as separate entries + /// in the PathSet — this is the multi-pattern handling. Today this bug is latent because + /// EnumerationPattern is always null from MSBuild's FileAccessData, but the schema is ready. + /// + [TestMethod] + public void FoldPathSetEntriesDirectoryEnumerationKeepsDistinctPatterns() + { + List entries = FingerprintFactory.FoldPathSetEntries( + observations: new[] + { + new ObservedPathEntry("dir", ObservationType.DirectoryEnumeration, "*.cs"), + new ObservedPathEntry("dir", ObservationType.DirectoryEnumeration, "*.dll"), + }, + enableProbeAndEnumeration: true); + + Assert.AreEqual(2, entries.Count, "Same-path DirectoryEnumeration with different patterns must produce TWO entries."); + CollectionAssert.AreEqual( + new[] { "*.cs", "*.dll" }, + entries.Select(e => e.EnumerationPattern).ToArray()); + Assert.IsTrue(entries.All(e => e.Type == ObservationType.DirectoryEnumeration)); + Assert.IsTrue(entries.All(e => e.Path == "dir")); + } + + /// + /// Same-path DirectoryEnumeration entries with the SAME pattern fold to one entry — duplicate + /// observations dedup correctly. + /// + [TestMethod] + public void FoldPathSetEntriesDirectoryEnumerationFoldsDuplicatePatterns() + { + List entries = FingerprintFactory.FoldPathSetEntries( + observations: new[] + { + new ObservedPathEntry("dir", ObservationType.DirectoryEnumeration, "*.cs"), + new ObservedPathEntry("dir", ObservationType.DirectoryEnumeration, "*.cs"), + new ObservedPathEntry("dir", ObservationType.DirectoryEnumeration, "*.cs"), + }, + enableProbeAndEnumeration: true); + + Assert.AreEqual(1, entries.Count); + Assert.AreEqual("*.cs", entries[0].EnumerationPattern); + } + + /// + /// FCR followed by a DirectoryEnumeration of the same path: FCR has higher precedence, so the + /// final entry is FCR — the enumeration is dropped. + /// + [TestMethod] + public void FoldPathSetEntriesContentReadOutranksEnumeration() + { + List entries = FingerprintFactory.FoldPathSetEntries( + observations: new[] + { + new ObservedPathEntry("ambiguous", ObservationType.FileContentRead), + new ObservedPathEntry("ambiguous", ObservationType.DirectoryEnumeration, "*.cs"), + }, + enableProbeAndEnumeration: true); + + Assert.AreEqual(1, entries.Count); + Assert.AreEqual(ObservationType.FileContentRead, entries[0].Type); + Assert.IsNull(entries[0].EnumerationPattern); + } + + /// + /// DirectoryEnumeration observations followed by an FCR observation of the same path: FCR replaces all + /// existing DirEnum entries (including multi-pattern entries) with a single FCR entry. + /// + [TestMethod] + public void FoldPathSetEntriesContentReadReplacesMultiPatternEnumeration() + { + List entries = FingerprintFactory.FoldPathSetEntries( + observations: new[] + { + new ObservedPathEntry("ambiguous", ObservationType.DirectoryEnumeration, "*.cs"), + new ObservedPathEntry("ambiguous", ObservationType.DirectoryEnumeration, "*.dll"), + new ObservedPathEntry("ambiguous", ObservationType.FileContentRead), + }, + enableProbeAndEnumeration: true); + + Assert.AreEqual(1, entries.Count); + Assert.AreEqual(ObservationType.FileContentRead, entries[0].Type); + } + + /// + /// Sort order pin: (Path OrdinalIgnoreCase, Type ascending, Pattern Ordinal). + /// + [TestMethod] + public void FoldPathSetEntriesProducesCanonicalSort() + { + List entries = FingerprintFactory.FoldPathSetEntries( + observations: new[] + { + new ObservedPathEntry("zeta", ObservationType.AbsentPathProbe), + new ObservedPathEntry("alpha", ObservationType.ExistingProbe), + new ObservedPathEntry("alpha", ObservationType.AbsentPathProbe), // dropped (lower precedence) + new ObservedPathEntry("mike", ObservationType.DirectoryEnumeration, "*.cs"), + new ObservedPathEntry("mike", ObservationType.DirectoryEnumeration, "*.dll"), + }, + enableProbeAndEnumeration: true); + + Assert.AreEqual(4, entries.Count); + Assert.AreEqual("alpha", entries[0].Path); + Assert.AreEqual(ObservationType.ExistingProbe, entries[0].Type); + Assert.AreEqual("mike", entries[1].Path); + Assert.AreEqual("*.cs", entries[1].EnumerationPattern); + Assert.AreEqual("mike", entries[2].Path); + Assert.AreEqual("*.dll", entries[2].EnumerationPattern); + Assert.AreEqual("zeta", entries[3].Path); + } + + // ========================================================================================= + // FilterObservations: predicted-input + hasher-can-hash + scope filter for sandbox observations. + // ========================================================================================= + + private static readonly PathNormalizer TestNormalizer = new(@"X:\Repo", @"X:\Nuget"); + private static readonly HashSet EmptyPredictedInputs = new(StringComparer.OrdinalIgnoreCase); + private static readonly IReadOnlyCollection EmptyIgnoredPatterns = Array.Empty(); + + /// + /// FCR observation whose path the hasher can hash → included as a normalized FCR entry. + /// + [TestMethod] + public void FilterObservationsFcrIncludedWhenHasherCanHash() + { + var inputHasher = new DictInputHasher(new Dictionary(StringComparer.OrdinalIgnoreCase) + { + [@"X:\Repo\src\foo.cs"] = new byte[] { 1 }, + }); + + (List included, List excluded) = FingerprintFactory.FilterObservations( + new[] { new ObservedAccess(@"X:\Repo\src\foo.cs", ObservationType.FileContentRead) }, + EmptyPredictedInputs, + inputHasher, + TestNormalizer, + EmptyIgnoredPatterns); + + Assert.AreEqual(1, included.Count); + Assert.AreEqual(@"{RepoRoot}src\foo.cs", included[0].Path); + Assert.AreEqual(ObservationType.FileContentRead, included[0].Type); + Assert.AreEqual(0, excluded.Count); + } + + /// + /// FCR observation whose path the hasher cannot hash → routed to the excluded list (for debug log), + /// not the PathSet. + /// + [TestMethod] + public void FilterObservationsFcrExcludedWhenHasherCannotHash() + { + (List included, List excluded) = FingerprintFactory.FilterObservations( + new[] { new ObservedAccess(@"X:\Repo\src\foo.cs", ObservationType.FileContentRead) }, + EmptyPredictedInputs, + new NoopInputHasher(), + TestNormalizer, + EmptyIgnoredPatterns); + + Assert.AreEqual(0, included.Count); + Assert.AreEqual(1, excluded.Count); + Assert.AreEqual(@"{RepoRoot}src\foo.cs", excluded[0]); + } + + /// + /// In-scope probe → included as a normalized entry. + /// + [TestMethod] + public void FilterObservationsKeepsInScopeProbe() + { + (List included, List excluded) = FingerprintFactory.FilterObservations( + new[] { new ObservedAccess(@"X:\Repo\src\foo.cs", ObservationType.ExistingProbe) }, + EmptyPredictedInputs, + new NoopInputHasher(), + TestNormalizer, + EmptyIgnoredPatterns); + + Assert.AreEqual(1, included.Count); + Assert.AreEqual(@"{RepoRoot}src\foo.cs", included[0].Path); + Assert.AreEqual(ObservationType.ExistingProbe, included[0].Type); + Assert.AreEqual(0, excluded.Count); + } + + /// + /// Out-of-scope probe → dropped silently (not in PathSet, not in debug log). + /// + [TestMethod] + public void FilterObservationsDropsOutOfScopeProbe() + { + (List included, List excluded) = FingerprintFactory.FilterObservations( + new[] { new ObservedAccess(@"C:\ProgramData\Microsoft\NetFramework\BreadcrumbStore\Foo", ObservationType.AbsentPathProbe) }, + EmptyPredictedInputs, + new NoopInputHasher(), + TestNormalizer, + EmptyIgnoredPatterns); + + Assert.AreEqual(0, included.Count); + Assert.AreEqual(0, excluded.Count); + } + + /// + /// Predicted inputs are dropped regardless of observation type — they're already covered by the weak FP. + /// + [TestMethod] + public void FilterObservationsDropsPredictedInputs() + { + HashSet predicted = new(StringComparer.OrdinalIgnoreCase) + { + @"X:\Repo\src\predicted.cs", + }; + var inputHasher = new DictInputHasher(new Dictionary(StringComparer.OrdinalIgnoreCase) + { + [@"X:\Repo\src\predicted.cs"] = new byte[] { 1 }, + }); + + (List included, List excluded) = FingerprintFactory.FilterObservations( + new[] + { + new ObservedAccess(@"X:\Repo\src\predicted.cs", ObservationType.FileContentRead), + new ObservedAccess(@"X:\Repo\src\predicted.cs", ObservationType.ExistingProbe), + }, + predicted, + inputHasher, + TestNormalizer, + EmptyIgnoredPatterns); + + Assert.AreEqual(0, included.Count); + Assert.AreEqual(0, excluded.Count); + } + + /// + /// DirectoryEnumeration entries preserve EnumerationPattern, Members, and WrittenMembers through the filter. + /// + [TestMethod] + public void FilterObservationsPreservesDirectoryEnumerationFields() + { + (List included, _) = FingerprintFactory.FilterObservations( + new[] + { + new ObservedAccess( + @"X:\Repo\dir", + ObservationType.DirectoryEnumeration, + EnumerationPattern: "*.cs", + Members: new[] { "a.cs", "b.cs" }, + WrittenMembers: new[] { "Foo.dll" }), + }, + EmptyPredictedInputs, + new NoopInputHasher(), + TestNormalizer, + EmptyIgnoredPatterns); + + Assert.AreEqual(1, included.Count); + Assert.AreEqual(ObservationType.DirectoryEnumeration, included[0].Type); + Assert.AreEqual("*.cs", included[0].EnumerationPattern); + CollectionAssert.AreEqual(new[] { "a.cs", "b.cs" }, included[0].Members?.ToArray()); + CollectionAssert.AreEqual(new[] { "Foo.dll" }, included[0].WrittenMembers?.ToArray()); + } + + /// + /// Empty input → empty output. + /// + [TestMethod] + public void FilterObservationsEmptyInput() + { + (List included, List excluded) = FingerprintFactory.FilterObservations( + Array.Empty(), + EmptyPredictedInputs, + new NoopInputHasher(), + TestNormalizer, + EmptyIgnoredPatterns); + + Assert.AreEqual(0, included.Count); + Assert.AreEqual(0, excluded.Count); + } + + /// + /// Paths matching IgnoredInputPatterns are dropped regardless of observation type. The pattern is + /// matched against the absolute path (pre-normalization). + /// + [TestMethod] + public void FilterObservationsDropsIgnoredInputPatterns() + { + var inputHasher = new DictInputHasher(new Dictionary(StringComparer.OrdinalIgnoreCase) + { + [@"X:\Repo\obj\noisy.cache"] = new byte[] { 1 }, + [@"X:\Repo\src\foo.cs"] = new byte[] { 2 }, + }); + IReadOnlyCollection ignored = new[] { Glob.Parse(@"X:\Repo\obj\**\*.cache") }; + + (List included, List excluded) = FingerprintFactory.FilterObservations( + new[] + { + new ObservedAccess(@"X:\Repo\obj\noisy.cache", ObservationType.FileContentRead), + new ObservedAccess(@"X:\Repo\obj\noisy.cache", ObservationType.ExistingProbe), + new ObservedAccess(@"X:\Repo\src\foo.cs", ObservationType.FileContentRead), + }, + EmptyPredictedInputs, + inputHasher, + TestNormalizer, + ignored); + + Assert.AreEqual(1, included.Count); + Assert.AreEqual(@"{RepoRoot}src\foo.cs", included[0].Path); + Assert.AreEqual(0, excluded.Count, "Ignored paths must not appear in the excluded debug list either."); + } + + private static FingerprintFactory CreateFactory(IContentHasher hasher, IInputHasher? inputHasher = null, PathNormalizer? pathNormalizer = null) + { + inputHasher ??= new NoopInputHasher(); + pathNormalizer ??= new PathNormalizer(@"X:\Repo", @"X:\Nuget"); + var settings = new TestPluginSettings { RepoRoot = @"X:\Repo" }; + return new FingerprintFactory(hasher, inputHasher, settings, pathNormalizer); + } + + private static List GetPluginSettingsFingerprintEntries(FingerprintFactory factory) + { + FieldInfo field = typeof(FingerprintFactory).GetField("_pluginSettingsFingerprintEntries", BindingFlags.Instance | BindingFlags.NonPublic)!; + return (List)field.GetValue(factory)!; + } + + /// + /// Helper to write a text file synchronously. net472 doesn't have File.WriteAllTextAsync, and these + /// tests must build on both TFMs. The work is trivially fast and happens during one-time test setup, + /// so blocking the async caller is benign. + /// +#pragma warning disable CA1849 // Synchronous IO blocks async caller. net472 lacks an async equivalent; trivial test-setup IO. + private static void WriteTextSync(string path, string text) + { + File.WriteAllText(path, text); + } +#pragma warning restore CA1849 + + private sealed class TestPluginSettings : PluginSettings + { + } + + private sealed class NoopInputHasher : IInputHasher + { + public bool ContainsPath(string absolutePath) => false; + + public ValueTask GetHashAsync(string absolutePath) => new((byte[]?)null); + } + + /// + /// IInputHasher backed by a dictionary of (absolute path → content hash). Returns null for paths not + /// in the dictionary, matching the contract for "path not known to the hasher". + /// + private sealed class DictInputHasher : IInputHasher + { + private readonly IReadOnlyDictionary _hashes; + + public DictInputHasher(IReadOnlyDictionary hashes) + { + _hashes = hashes; + } + + public bool ContainsPath(string absolutePath) => _hashes.ContainsKey(absolutePath); + + public ValueTask GetHashAsync(string absolutePath) + { + return new ValueTask(_hashes.TryGetValue(absolutePath, out byte[]? hash) ? hash : null); + } + } + + /// + /// Disposable temporary directory for filesystem-backed tests. Created under the system temp root with a + /// unique GUID suffix; cleaned up on Dispose even if the test throws. + /// + private sealed class TempDirectory : IDisposable + { + public string Path { get; } + + private TempDirectory(string path) + { + Path = path; + } + + public static TempDirectory Create(string prefix) + { + string path = System.IO.Path.Combine(System.IO.Path.GetTempPath(), $"{prefix}-{Guid.NewGuid():N}"); + Directory.CreateDirectory(path); + return new TempDirectory(path); + } + + public void Dispose() + { + if (Directory.Exists(Path)) + { + try + { + Directory.Delete(Path, recursive: true); + } + catch + { + // Best-effort cleanup; don't mask the underlying test failure if any. + } + } + } + } +} diff --git a/src/Common.Tests/PathNormalizerTests.cs b/src/Common.Tests/PathNormalizerTests.cs new file mode 100644 index 0000000..25e0f35 --- /dev/null +++ b/src/Common.Tests/PathNormalizerTests.cs @@ -0,0 +1,76 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.MSBuildCache.Tests; + +[TestClass] +public class PathNormalizerTests +{ + /// + /// Repo-rooted paths are normalized after . + /// + [TestMethod] + public void IsNormalizedRepoRoot() + { + var pn = new PathNormalizer(@"X:\Repo", @"X:\Nuget"); + string normalized = pn.Normalize(@"X:\Repo\src\foo.cs"); + + Assert.IsTrue(PathNormalizer.IsNormalized(normalized), $"Expected '{normalized}' to be normalized (under repo root)."); + } + + /// + /// NuGet-rooted paths are normalized after . + /// + [TestMethod] + public void IsNormalizedNugetRoot() + { + var pn = new PathNormalizer(@"X:\Repo", @"X:\Nuget"); + string normalized = pn.Normalize(@"X:\Nuget\some.package\1.0.0\lib\foo.dll"); + + Assert.IsTrue(PathNormalizer.IsNormalized(normalized), $"Expected '{normalized}' to be normalized (under NuGet root)."); + } + + /// + /// System paths (drives outside repo+NuGet) are not normalized. This is the rule that filters + /// .NET Framework breadcrumb stores, NGEN caches, MSBuild SDK installs, etc. + /// + [TestMethod] + public void IsNormalizedSystemPathFails() + { + var pn = new PathNormalizer(@"X:\Repo", @"X:\Nuget"); + string normalized = pn.Normalize(@"C:\ProgramData\Microsoft\NetFramework\BreadcrumbStore\some.breadcrumb"); + + Assert.IsFalse(PathNormalizer.IsNormalized(normalized), $"Expected '{normalized}' to NOT be normalized (system path)."); + } + + /// + /// Same drive as the repo but outside the repo root is still not normalized. + /// + [TestMethod] + public void IsNormalizedSameDriveOutsideRepoFails() + { + var pn = new PathNormalizer(@"X:\Repo", @"X:\Nuget"); + string normalized = pn.Normalize(@"X:\OtherProject\foo.cs"); + + Assert.IsFalse(PathNormalizer.IsNormalized(normalized), $"Expected '{normalized}' to NOT be normalized (sibling of repo root)."); + } + + /// + /// Round-trip: Normalize then Unnormalize returns the original path (or an OS-equivalent form). + /// Pinned because the scope check depends on Normalize producing a placeholder prefix for in-scope paths. + /// + [TestMethod] + public void NormalizeUnnormalizeRoundTrip() + { + var pn = new PathNormalizer(@"X:\Repo", @"X:\Nuget"); + string original = @"X:\Repo\src\Program.cs"; + + string normalized = pn.Normalize(original); + string roundTripped = pn.Unnormalize(normalized); + + Assert.AreEqual(original, roundTripped); + Assert.IsTrue(normalized.StartsWith("{RepoRoot}", System.StringComparison.Ordinal)); + } +} diff --git a/src/Common.Tests/PathSetTests.cs b/src/Common.Tests/PathSetTests.cs index 7063d65..47bbbeb 100644 --- a/src/Common.Tests/PathSetTests.cs +++ b/src/Common.Tests/PathSetTests.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; +using System.Text.Json; using Microsoft.MSBuildCache.Fingerprinting; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -10,6 +11,156 @@ namespace Microsoft.MSBuildCache.Tests; [TestClass] public class PathSetTests { + /// + /// A payload without an Entries property — written by a version predating this schema, or + /// truncated — must degrade to a cache miss rather than throwing. is used as a + /// dictionary key during cache lookup, so it is hashed before any caller can inspect it; a null + /// Entries would throw from and take down the build, + /// bypassing the null handling in the fingerprint factory entirely. + /// + [TestMethod] + public void DeserializingPayloadWithoutEntriesDoesNotThrow() + { + const string NoEntriesPayload = @"{""FilesRead"":[""dir/a.cs"",""dir/b.cs""]}"; + + PathSet? deserialized = JsonSerializer.Deserialize(NoEntriesPayload, SourceGenerationContext.Default.PathSet); + + Assert.IsNotNull(deserialized); + Assert.IsNotNull(deserialized!.Entries, "Entries must never be null; it is hashed before it can be checked."); + Assert.AreEqual(0, deserialized.Entries.Count); + + // Both must be callable, since the type is used as a cache key. + _ = deserialized.GetHashCode(); + Assert.IsTrue(deserialized.Equals(new PathSet(new List()))); + } + + [TestMethod] + public void EqualsIdenticalEntriesWithNullPattern() + { + // Regression: pre-Phase-1 the path was stored as a raw string; the new schema permits a null EnumerationPattern. + // Equality and hashing must tolerate null on both sides without throwing. + var a = new PathSet(new List + { + new("dir/file.cs", ObservationType.FileContentRead), + }); + var b = new PathSet(new List + { + new("dir/file.cs", ObservationType.FileContentRead), + }); + + Assert.IsTrue(a.Equals(b)); + Assert.AreEqual(a.GetHashCode(), b.GetHashCode()); + } + + [TestMethod] + public void EqualsIdenticalEntries() + { + var a = new PathSet(new List + { + new("dir/file.cs", ObservationType.FileContentRead), + new("dir2/", ObservationType.DirectoryEnumeration, "*.cs"), + }); + var b = new PathSet(new List + { + new("DIR/FILE.CS", ObservationType.FileContentRead), // case-insensitive path + new("dir2/", ObservationType.DirectoryEnumeration, "*.cs"), + }); + + Assert.IsTrue(a.Equals(b)); + Assert.AreEqual(a.GetHashCode(), b.GetHashCode()); + } + + [TestMethod] + public void EqualsDifferingType() + { + var a = new PathSet(new List + { + new("file", ObservationType.FileContentRead), + }); + var b = new PathSet(new List + { + new("file", ObservationType.ExistingProbe), + }); + + Assert.IsFalse(a.Equals(b)); + } + + [TestMethod] + public void EqualsDifferingPattern() + { + var a = new PathSet(new List + { + new("dir", ObservationType.DirectoryEnumeration, "*.cs"), + }); + var b = new PathSet(new List + { + new("dir", ObservationType.DirectoryEnumeration, "*.fs"), + }); + + Assert.IsFalse(a.Equals(b)); + } + + [TestMethod] + public void EqualsDifferingPatternCase() + { + // Pattern equality is ordinal — a build that enumerated for "*.CS" is semantically distinct from one that + // enumerated for "*.cs" (the underlying enumeration API may or may not be case-insensitive at the filesystem + // layer, but the strong fingerprint must be deterministic in the pattern string itself). + var a = new PathSet(new List + { + new("dir", ObservationType.DirectoryEnumeration, "*.cs"), + }); + var b = new PathSet(new List + { + new("dir", ObservationType.DirectoryEnumeration, "*.CS"), + }); + + Assert.IsFalse(a.Equals(b)); + } + + [TestMethod] + public void EqualsOrderMatters() + { + // PathSet equality is positional. Callers are responsible for canonical sort before construction. + var a = new PathSet(new List + { + new("a", ObservationType.FileContentRead), + new("b", ObservationType.FileContentRead), + }); + var b = new PathSet(new List + { + new("b", ObservationType.FileContentRead), + new("a", ObservationType.FileContentRead), + }); + + Assert.IsFalse(a.Equals(b)); + } + + [TestMethod] + public void JsonRoundTripAllObservationTypes() + { + var original = new PathSet(new List + { + new("repo/src/foo.cs", ObservationType.FileContentRead), + new("repo/src/", ObservationType.DirectoryEnumeration, "*.cs"), + new("repo/bin/probed.dll", ObservationType.ExistingProbe), + new("repo/obj/missing.gen.cs", ObservationType.AbsentPathProbe), + }); + + string serialized = JsonSerializer.Serialize(original, SourceGenerationContext.Default.PathSet); + PathSet? deserialized = JsonSerializer.Deserialize(serialized, SourceGenerationContext.Default.PathSet); + + Assert.IsNotNull(deserialized); + Assert.AreEqual(original, deserialized); + // Sanity-check that every type round-tripped: equality covers it, but be explicit about the schema field. + Assert.AreEqual(4, deserialized!.Entries.Count); + Assert.AreEqual(ObservationType.FileContentRead, deserialized.Entries[0].Type); + Assert.AreEqual(ObservationType.DirectoryEnumeration, deserialized.Entries[1].Type); + Assert.AreEqual("*.cs", deserialized.Entries[1].EnumerationPattern); + Assert.AreEqual(ObservationType.ExistingProbe, deserialized.Entries[2].Type); + Assert.AreEqual(ObservationType.AbsentPathProbe, deserialized.Entries[3].Type); + } + [TestMethod] public void ObservationTypeByteValuesAreStable() { @@ -131,4 +282,21 @@ public void MembersAndWrittenMembersOnlyApplyToDirectoryEnumeration() Assert.IsNull(fcr.WrittenMembers); Assert.IsNull(fcr.EnumerationPattern); } + + [TestMethod] + public void JsonRoundTripPreservesMembersAndWrittenMembers() + { + var original = new PathSet(new List + { + new("dir/", ObservationType.DirectoryEnumeration, enumerationPattern: "*.cs", + members: new[] { "a.cs", "b.cs" }, + writtenMembers: new[] { "Foo.dll", "Foo.pdb" }), + }); + + string json = JsonSerializer.Serialize(original, SourceGenerationContext.Default.PathSet); + PathSet roundTripped = JsonSerializer.Deserialize(json, SourceGenerationContext.Default.PathSet)!; + + Assert.IsTrue(original.Equals(roundTripped), + $"PathSet should round-trip through JSON serialization. Original entries: {original.Entries.Count}, deserialized entries: {roundTripped.Entries.Count}."); + } } diff --git a/src/Common/FileAccess/FileAccessRepository.cs b/src/Common/FileAccess/FileAccessRepository.cs index 39ef67d..1b56921 100644 --- a/src/Common/FileAccess/FileAccessRepository.cs +++ b/src/Common/FileAccess/FileAccessRepository.cs @@ -9,6 +9,7 @@ using DotNet.Globbing; using Microsoft.Build.Experimental.FileAccess; using Microsoft.Build.Experimental.ProjectCache; +using Microsoft.MSBuildCache.Fingerprinting; namespace Microsoft.MSBuildCache.FileAccess; @@ -315,7 +316,7 @@ private static FileAccesses ProcessFileAccesses( List deletedDirectories) { var outputs = new HashSet(StringComparer.OrdinalIgnoreCase); - var inputs = new HashSet(StringComparer.OrdinalIgnoreCase); + List allObservations = new(); IEnumerable outputFileInfos = fileTable .Select(fileInfoKvp => fileInfoKvp.Value) @@ -346,10 +347,10 @@ private static FileAccesses ProcessFileAccesses( continue; } - inputs.Add(filePath); + allObservations.Add(new ObservedAccess(filePath, ObservationType.FileContentRead)); } - return new FileAccesses(inputs, outputs); + return new FileAccesses(allObservations, outputs); } private static bool IsOutput(FileAccessInfo fileInfo) diff --git a/src/Common/FileAccess/FileAccesses.cs b/src/Common/FileAccess/FileAccesses.cs index f55e401..c89c5d2 100644 --- a/src/Common/FileAccess/FileAccesses.cs +++ b/src/Common/FileAccess/FileAccesses.cs @@ -5,15 +5,6 @@ namespace Microsoft.MSBuildCache.FileAccess; -internal sealed class FileAccesses -{ - public FileAccesses(IReadOnlyCollection inputs, IReadOnlyCollection outputs) - { - Inputs = inputs; - Outputs = outputs; - } - - public IReadOnlyCollection Inputs { get; } - - public IReadOnlyCollection Outputs { get; } -} +internal sealed record FileAccesses( + IReadOnlyCollection Observations, + IReadOnlyCollection Outputs); diff --git a/src/Common/Fingerprinting/FingerprintFactory.cs b/src/Common/Fingerprinting/FingerprintFactory.cs index d37288b..884f6c0 100644 --- a/src/Common/Fingerprinting/FingerprintFactory.cs +++ b/src/Common/Fingerprinting/FingerprintFactory.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft. All rights reserved. +// Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; @@ -11,6 +11,7 @@ using BuildXL.Cache.ContentStore.Hashing; using BuildXL.Cache.ContentStore.Interfaces.Extensions; using DotNet.Globbing; +using Microsoft.MSBuildCache.FileAccess; using Microsoft.MSBuildCache.Hashing; using NuGet.Versioning; @@ -35,6 +36,11 @@ public sealed class FingerprintFactory : IFingerprintFactory private readonly PluginSettings _pluginSettings; private readonly PathNormalizer _pathNormalizer; + // Sentinels computed once per factory instance from the configured content hasher; constant across + // instances using the same HashType. + internal byte[] AbsentFileSentinel { get; } + internal byte[] ZeroHash { get; } + public FingerprintFactory( IContentHasher contentHasher, IInputHasher inputHasher, @@ -46,6 +52,11 @@ public FingerprintFactory( _pluginSettings = pluginSettings; _pathNormalizer = pathNormalizer; + byte[] ComputeWellKnownHash(string value) => contentHasher.GetContentHash(Encoding.UTF8.GetBytes(value)).ToHashByteArray(); + + AbsentFileSentinel = ComputeWellKnownHash("Microsoft.MSBuildCache.Fingerprinting.AbsentFile"); + ZeroHash = ComputeWellKnownHash("Microsoft.MSBuildCache.Fingerprinting.Zero"); + _pluginSettingsFingerprintEntries = new List() { CreateFingerprintEntry($"NodeBuildResultVersion: {NodeBuildResult.CurrentVersion}"), @@ -70,6 +81,9 @@ void AddSettingToFingerprint(IReadOnlyCollection? patterns, string setting AddSettingToFingerprint(pluginSettings.AllowFileAccessAfterProjectFinishFilePatterns, nameof(pluginSettings.AllowFileAccessAfterProjectFinishFilePatterns)); AddSettingToFingerprint(pluginSettings.AllowFileAccessAfterProjectFinishProcessPatterns, nameof(pluginSettings.AllowFileAccessAfterProjectFinishProcessPatterns)); AddSettingToFingerprint(pluginSettings.AllowProcessCloseAfterProjectFinishProcessPatterns, nameof(pluginSettings.AllowProcessCloseAfterProjectFinishProcessPatterns)); + + _pluginSettingsFingerprintEntries.Add( + CreateFingerprintEntry($"{nameof(pluginSettings.EnableProbeAndEnumerationFingerprinting)}: {pluginSettings.EnableProbeAndEnumerationFingerprinting}")); } public async Task GetWeakFingerprintAsync(NodeContext nodeContext) @@ -159,52 +173,191 @@ void AddSettingToFingerprint(IReadOnlyCollection? patterns, string setting return CreateFingerprint(entries); }); - public PathSet? GetPathSet(NodeContext nodeContext, IEnumerable observedInputs) + public PathSet? GetPathSet(NodeContext nodeContext, IReadOnlyCollection observations) { - List pathSetIncludedNormalizedInputs = new(); - List pathSetExcludedNormalizedInputs = new(); - HashSet predictedInputsSet = new(StringComparer.OrdinalIgnoreCase); foreach (string input in nodeContext.Inputs) { predictedInputsSet.Add(input); } - // As an optimization, only include non-predicted inputs. If a predicted input changes, the weak fingerprint - // will not match and so the associated PathSets will never be used. - foreach (string observedInput in observedInputs) + (List included, List excluded) = FilterObservations( + observations, + predictedInputsSet, + _inputHasher, + _pathNormalizer, + _pluginSettings.IgnoredInputPatterns); + + // Build the typed entry list. Extracted to a testable static helper. + List sortedEntries = FoldPathSetEntries( + included, + enableProbeAndEnumeration: _pluginSettings.EnableProbeAndEnumerationFingerprinting); + + // To help with debugging, dump the files which were included and excluded from the PathSet. + // When probe/enumeration fingerprinting is active, include the type column so the log is diagnosable. + File.WriteAllLines( + Path.Combine(nodeContext.LogDirectory, "pathSetIncluded.txt"), + sortedEntries.Select(e => e.Type == ObservationType.FileContentRead + ? e.Path + : (e.EnumerationPattern is null + ? $"[{e.Type}] {e.Path}" + : $"[{e.Type} {e.EnumerationPattern}] {e.Path}"))); + excluded.Sort(StringComparer.OrdinalIgnoreCase); + File.WriteAllLines(Path.Combine(nodeContext.LogDirectory, "pathSetExcluded.txt"), excluded); + + // If the PathSet is effectively empty, return null instead. + if (sortedEntries.Count == 0) + { + return null; + } + + return new PathSet(sortedEntries); + } + + /// + /// Filters and normalizes sandbox observations into records suitable for + /// inclusion in a . Drops: + /// + /// Predicted inputs (already covered by the weak fingerprint). + /// Observations whose absolute path matches one of . + /// Probe/enumeration observations outside any known root (see ). + /// + /// For FileContentRead observations, paths the input hasher cannot hash are returned in the + /// Excluded list (for debug logging) rather than the included list. + /// + internal static (List Included, List Excluded) FilterObservations( + IReadOnlyCollection observations, + HashSet predictedInputs, + IInputHasher inputHasher, + PathNormalizer pathNormalizer, + IReadOnlyCollection ignoredInputPatterns) + { + List included = new(); + List excluded = new(); + + foreach (ObservedAccess observation in observations) { - if (predictedInputsSet.Contains(observedInput)) + // Predicted inputs are already covered by the weak fingerprint; skip. + if (predictedInputs.Contains(observation.Path)) { continue; } - string normalizedInputPath = _pathNormalizer.Normalize(observedInput); - if (_inputHasher.ContainsPath(observedInput)) + // Drop observations whose path matches a configured ignore pattern. Applied before normalization + // so the pattern matches against the absolute path (the form users author). + if (MatchesAny(observation.Path, ignoredInputPatterns)) { - pathSetIncludedNormalizedInputs.Add(normalizedInputPath); + continue; } - else + + string normalizedPath = pathNormalizer.Normalize(observation.Path); + + if (observation.Type == ObservationType.FileContentRead) + { + // FCR contributes only if the hasher can hash this path; otherwise it just goes to a debug log. + if (inputHasher.ContainsPath(observation.Path)) + { + included.Add(new ObservedPathEntry(normalizedPath, ObservationType.FileContentRead)); + } + else + { + excluded.Add(normalizedPath); + } + } + else if (PathNormalizer.IsNormalized(normalizedPath)) { - pathSetExcludedNormalizedInputs.Add(normalizedInputPath); + // Probe/enum contributes only if it's under a known root. + included.Add(new ObservedPathEntry(normalizedPath, observation.Type, observation.EnumerationPattern, observation.Members, observation.WrittenMembers)); } } - // Sort the collections for consistent ordering - pathSetIncludedNormalizedInputs.Sort(StringComparer.OrdinalIgnoreCase); - pathSetExcludedNormalizedInputs.Sort(StringComparer.OrdinalIgnoreCase); + return (included, excluded); + } - // To help with debugging, dump the files which were included and excluded from the PathSet. - File.WriteAllLines(Path.Combine(nodeContext.LogDirectory, "pathSetIncluded.txt"), pathSetIncludedNormalizedInputs); - File.WriteAllLines(Path.Combine(nodeContext.LogDirectory, "pathSetExcluded.txt"), pathSetExcludedNormalizedInputs); + private static bool MatchesAny(string path, IReadOnlyCollection patterns) + { + if (patterns.Count == 0) + { + return false; + } - // If the PathSet is effectively empty, return null instead. - if (pathSetIncludedNormalizedInputs.Count == 0) + foreach (Glob pattern in patterns) { - return null; + if (pattern.IsMatch(path)) + { + return true; + } } - return new PathSet(pathSetIncludedNormalizedInputs); + return false; + } + + /// + /// Folds observations into a canonical, sorted list of . For multiple + /// observations of the same path: + /// + /// The highest-precedence type wins (per ). + /// DirectoryEnumeration entries with distinct EnumerationPatterns are kept as separate entries. + /// + /// When is false, only FileContentRead + /// observations contribute. Returns entries sorted by + /// (Path OrdinalIgnoreCase, Type ascending, Pattern Ordinal). + /// + internal static List FoldPathSetEntries( + IReadOnlyCollection observations, + bool enableProbeAndEnumeration) + { + Dictionary> normalizedPathToEntries = new(StringComparer.OrdinalIgnoreCase); + + foreach (ObservedPathEntry observation in observations) + { + if (!enableProbeAndEnumeration && observation.Type != ObservationType.FileContentRead) + { + continue; + } + + if (!normalizedPathToEntries.TryGetValue(observation.Path, out List? existingEntries)) + { + normalizedPathToEntries[observation.Path] = new List { observation }; + continue; + } + + // All entries for a given path share the same highest-precedence Type by construction. + ObservationType currentType = existingEntries[0].Type; + ObservationType winning = ObservationTypePrecedence.Max(currentType, observation.Type); + + if (winning != currentType) + { + existingEntries.Clear(); + existingEntries.Add(observation); + } + else if (currentType == ObservationType.DirectoryEnumeration + && observation.Type == ObservationType.DirectoryEnumeration) + { + bool patternAlreadyPresent = false; + foreach (ObservedPathEntry e in existingEntries) + { + if (string.Equals(e.EnumerationPattern, observation.EnumerationPattern, StringComparison.Ordinal)) + { + patternAlreadyPresent = true; + break; + } + } + + if (!patternAlreadyPresent) + { + existingEntries.Add(observation); + } + } + // else: new observation has same-or-lower precedence and is not a new DirEnum pattern — drop. + } + + return normalizedPathToEntries.Values + .SelectMany(list => list) + .OrderBy(e => e.Path, StringComparer.OrdinalIgnoreCase) + .ThenBy(e => (byte)e.Type) + .ThenBy(e => e.EnumerationPattern, StringComparer.Ordinal) + .ToList(); } public async Task GetStrongFingerprintAsync(PathSet? pathSet) @@ -214,13 +367,13 @@ void AddSettingToFingerprint(IReadOnlyCollection? patterns, string setting pathSet, async pathSet => { - if (pathSet?.FilesRead == null || pathSet.FilesRead.Count == 0) + if (pathSet?.Entries == null || pathSet.Entries.Count == 0) { return null; } List entries = new(); - await SortAndAddInputFileHashesAsync(entries, pathSet.FilesRead, pathsAreNormalized: true); + await SortAndAddPathSetEntriesAsync(entries, pathSet.Entries, pathsAreNormalized: true); if (entries.Count == 0) { @@ -230,6 +383,145 @@ void AddSettingToFingerprint(IReadOnlyCollection? patterns, string setting return CreateFingerprint(entries); }); + private async Task SortAndAddPathSetEntriesAsync(List entries, IReadOnlyList pathSetEntries, bool pathsAreNormalized) + { + // PathSet.Entries are sorted by (Path OrdinalIgnoreCase, Type ascending, Pattern Ordinal) per the + // PathSet contract — GetPathSet sorts at populate, deserialization preserves order at lookup. + + // Pre-compute file content hashes in parallel for FileContentRead entries — they're typically the + // most expensive payloads. + Dictionary fileContentHashes = new(StringComparer.OrdinalIgnoreCase); + List> pendingHashes = new(); + foreach (ObservedPathEntry entry in pathSetEntries) + { + if (entry.Type != ObservationType.FileContentRead) + { + continue; + } + + string absoluteFilePath = pathsAreNormalized ? _pathNormalizer.Unnormalize(entry.Path) : entry.Path; + if (_pluginSettings.IgnoredInputPatterns.Count > 0 + && _pluginSettings.IgnoredInputPatterns.Any(pattern => pattern.IsMatch(absoluteFilePath))) + { + continue; + } + + ValueTask hashTask = _inputHasher.GetHashAsync(absoluteFilePath); + if (hashTask.IsCompletedSuccessfully) + { + fileContentHashes[entry.Path] = hashTask.Result; + } + else + { + pendingHashes.Add(WrapAsync(entry.Path, hashTask.AsTask())); + } + + static async Task<(string Path, byte[]? Hash)> WrapAsync(string path, Task task) => (path, await task); + } + + if (pendingHashes.Count > 0) + { + foreach ((string path, byte[]? hash) in await Task.WhenAll(pendingHashes)) + { + fileContentHashes[path] = hash; + } + } + + // Emit entries — exactly one FingerprintEntry per ObservedPathEntry, with the + // description encoding the observation's identity (type + path + any pattern) and the payload + // encoding its value (content hash for FCR, member hash for DirEnum; probes have no value beyond + // their identity, so the description hash alone is the entry hash). + foreach (ObservedPathEntry entry in pathSetEntries) + { + string normalizedPath = pathsAreNormalized ? entry.Path : _pathNormalizer.Normalize(entry.Path); + + switch (entry.Type) + { + case ObservationType.FileContentRead: + { + byte[]? contentHash = fileContentHashes.TryGetValue(entry.Path, out byte[]? h) ? h : null; + if (contentHash is null) + { + // The configured IInputHasher returned no hash for this path (e.g., out-of-scope or + // excluded). The path is intentionally not part of the fingerprint — skip emitting + // any entry so neither its content nor its identity contributes to the strong FP. + break; + } + + entries.Add(CreateFingerprintEntry($"FileContentRead: {normalizedPath}", contentHash)); + break; + } + + case ObservationType.DirectoryEnumeration: + { + string description = string.IsNullOrEmpty(entry.EnumerationPattern) + ? $"DirectoryEnumeration: {normalizedPath}" + : $"DirectoryEnumeration: {normalizedPath} ({entry.EnumerationPattern})"; + byte[] memberHash = ComputeDirectoryMemberHash(entry); + + // Header entry: payload encodes the entry's state (absent / empty / member-hash for + // non-empty), so the strong FP differentiates all three. + entries.Add(CreateFingerprintEntry(description, memberHash)); + + // Per-member entries: redundant for correctness (the header already encodes membership + // via ComputeDirectoryMemberHash), but they make fingerprint dumps diff-friendly — a + // reviewer can see exactly which member was added or removed between builds. The payload + // reuses the precomputed member hash so the call shape stays uniform with the header. + if (entry.Members is not null) + { + foreach (string member in entry.Members) + { + entries.Add(CreateFingerprintEntry($"{description} - {member}", memberHash)); + } + } + break; + } + + case ObservationType.ExistingProbe: + { + entries.Add(CreateFingerprintEntry($"ExistingProbe: {normalizedPath}")); + break; + } + + case ObservationType.AbsentPathProbe: + { + entries.Add(CreateFingerprintEntry($"AbsentPathProbe: {normalizedPath}")); + break; + } + } + } + } + + /// + /// Computes the strong-fingerprint payload for a DirectoryEnumeration entry — a deterministic hash of + /// (no filesystem access). Returns + /// when Members is null (directory absent/inaccessible at populate) + /// and when Members is empty. + /// + private byte[] ComputeDirectoryMemberHash(ObservedPathEntry entry) + { + if (entry.Members is null) + { + return AbsentFileSentinel; + } + + if (entry.Members.Count == 0) + { + return ZeroHash; + } + + var sb = new StringBuilder(); + foreach (string name in entry.Members) + { + // Uppercase to mirror CreateFingerprintEntry's case-insensitive normalization; null separator + // disambiguates entries (so "ab" + "c" can't collide with "a" + "bc"). + sb.Append(name.ToUpperInvariant()).Append('\0'); + } + + byte[] bytes = Encoding.UTF8.GetBytes(sb.ToString()); + return _contentHasher.GetContentHash(bytes).ToHashByteArray(); + } + private async Task SortAndAddInputFileHashesAsync(List entries, IReadOnlyList files, bool pathsAreNormalized) { // Sort for consistent hash ordering @@ -308,4 +600,17 @@ private FingerprintEntry CreateFingerprintEntry(string info) }), info); } + + /// + /// Like but combines an additional payload (e.g., a file + /// content hash) into the entry's hash. The description-derived hash carries the entry's identity (path, + /// type); the payload carries its value (content / membership). The resulting entry's hash depends on + /// both — change either and the fingerprint differs. + /// + private FingerprintEntry CreateFingerprintEntry(string info, byte[] payload) + { + FingerprintEntry descriptionEntry = CreateFingerprintEntry(info); + byte[] combined = _contentHasher.CombineHashes(new[] { descriptionEntry.Hash, payload })!; + return new FingerprintEntry(combined, info); + } } diff --git a/src/Common/Fingerprinting/IFingerprintFactory.cs b/src/Common/Fingerprinting/IFingerprintFactory.cs index 64db72a..a8dbd69 100644 --- a/src/Common/Fingerprinting/IFingerprintFactory.cs +++ b/src/Common/Fingerprinting/IFingerprintFactory.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Threading.Tasks; +using Microsoft.MSBuildCache.FileAccess; namespace Microsoft.MSBuildCache.Fingerprinting; @@ -10,7 +11,7 @@ public interface IFingerprintFactory { Task GetWeakFingerprintAsync(NodeContext nodeContext); - PathSet? GetPathSet(NodeContext nodeContext, IEnumerable observedInputs); + PathSet? GetPathSet(NodeContext nodeContext, IReadOnlyCollection observations); Task GetStrongFingerprintAsync(PathSet? pathSet); } diff --git a/src/Common/Fingerprinting/PathSet.cs b/src/Common/Fingerprinting/PathSet.cs index 907d19b..dcb3dfa 100644 --- a/src/Common/Fingerprinting/PathSet.cs +++ b/src/Common/Fingerprinting/PathSet.cs @@ -8,15 +8,24 @@ namespace Microsoft.MSBuildCache.Fingerprinting; public sealed class PathSet : IEquatable { - public PathSet(IReadOnlyList filesRead) + public PathSet(IReadOnlyList entries) { - FilesRead = filesRead; + // Normalize null to empty. Deserializing a payload without an `Entries` property — a blob written + // by a version predating this schema, or a truncated one — yields null here, and this type is used + // as a dictionary key during cache lookup, so hashing happens before any caller can null-check it. + // An empty set produces a null strong fingerprint, which skips the selector and misses, so + // unrecognized data degrades to a cache miss rather than throwing mid-lookup. + Entries = entries ?? Array.Empty(); } /// - /// Gets the set of files read which were not predicted. These paths are normalized. + /// Gets the set of observations made during the build that were not predicted at planning time. /// - public IReadOnlyList FilesRead { get; } + /// + /// Entries are expected to be sorted by (Path OrdinalIgnoreCase, Type ascending, EnumerationPattern Ordinal) + /// so that semantically equivalent PathSets serialize and compare identically. + /// + public IReadOnlyList Entries { get; } public bool Equals(PathSet? other) { @@ -30,14 +39,14 @@ public bool Equals(PathSet? other) return false; } - if (FilesRead.Count != other.FilesRead.Count) + if (Entries.Count != other.Entries.Count) { return false; } - for (int i = 0; i < FilesRead.Count; i++) + for (int i = 0; i < Entries.Count; i++) { - if (!FilesRead[i].Equals(other.FilesRead[i], StringComparison.OrdinalIgnoreCase)) + if (!Entries[i].Equals(other.Entries[i])) { return false; } @@ -51,9 +60,9 @@ public bool Equals(PathSet? other) public override int GetHashCode() { var hashCode = default(HashCode); - foreach (string file in FilesRead) + foreach (ObservedPathEntry entry in Entries) { - hashCode.Add(file, StringComparer.OrdinalIgnoreCase); + hashCode.Add(entry); } return hashCode.ToHashCode(); diff --git a/src/Common/MSBuildCachePluginBase.cs b/src/Common/MSBuildCachePluginBase.cs index de18ce2..cc62712 100644 --- a/src/Common/MSBuildCachePluginBase.cs +++ b/src/Common/MSBuildCachePluginBase.cs @@ -614,11 +614,15 @@ private async Task HandleProjectFinishedInnerAsync(FileAccessContext fileAccessC List> packageFileHashingTasks = new(); static async Task<(byte[]?, string)> WrapHashingTask(Task hashTask, string packageRootRelativeFilePath) => (await hashTask, packageRootRelativeFilePath); - List filesRead = new(); using var observedInputsWriter = new StreamWriter(Path.Combine(nodeContext.LogDirectory, "observedInputs.txt")); - foreach (string absolutePath in fileAccesses.Inputs) + foreach (ObservedAccess observation in fileAccesses.Observations) { - filesRead.Add(absolutePath); + if (observation.Type != ObservationType.FileContentRead) + { + continue; + } + + string absolutePath = observation.Path; string? packageRootRelativeFilePath = absolutePath.MakePathRelativeTo(NugetPackageRoot); if (packageRootRelativeFilePath != null) @@ -717,7 +721,7 @@ bool MatchesIgnoredOutputPattern(string path) } } - PathSet? pathSet = FingerprintFactory.GetPathSet(nodeContext, filesRead); + PathSet? pathSet = FingerprintFactory.GetPathSet(nodeContext, fileAccesses.Observations); if (buildResult.OverallResult != BuildResultCode.Success) { diff --git a/src/Common/PathNormalizer.cs b/src/Common/PathNormalizer.cs index 5e87cb5..de1f448 100644 --- a/src/Common/PathNormalizer.cs +++ b/src/Common/PathNormalizer.cs @@ -39,4 +39,12 @@ public string Unnormalize(string normalized) => normalized .Replace(RepoRootPlaceholder, _repoRoot, StringComparison.Ordinal) .Replace(NugetPackageRootPlaceholder, _nugetPackageRoot, StringComparison.Ordinal); + + /// + /// Returns true if the given normalized path starts with one of the placeholders inserted by + /// — i.e., it was rooted under the repo or NuGet package root. + /// + public static bool IsNormalized(string normalizedPath) + => normalizedPath.StartsWith(RepoRootPlaceholder, StringComparison.Ordinal) + || normalizedPath.StartsWith(NugetPackageRootPlaceholder, StringComparison.Ordinal); } diff --git a/src/Common/PluginSettings.cs b/src/Common/PluginSettings.cs index 4845fbf..4d76621 100644 --- a/src/Common/PluginSettings.cs +++ b/src/Common/PluginSettings.cs @@ -111,6 +111,17 @@ public string LocalCacheRootPath public bool TouchOutputFiles { get; init; } + /// + /// Enables probe and directory-enumeration tracking in fingerprints. When false, only file content + /// reads contribute to the fingerprint, matching pre-feature behavior. + /// + /// + /// This contributes to the weak fingerprint, which keeps caches produced with and without the feature + /// from being shared — important because an entry produced without it records no probe or enumeration + /// dependencies at all, and reusing that entry on a host that does track them would be an incorrect hit. + /// + public bool EnableProbeAndEnumerationFingerprinting { get; init; } + public static T Create( IReadOnlyDictionary settings, PluginLoggerBase logger, From ca04555e25910dc0dcdceec5aaed5253e0e8f3bc Mon Sep 17 00:00:00 2001 From: David Federman Date: Fri, 31 Jul 2026 17:18:44 -0700 Subject: [PATCH 03/13] Detect whether the running MSBuild reports enumeration patterns MSBuild grew an EnumeratePattern field on FileAccessData, which directory-enumeration observations need: without the pattern a filtered enumeration such as `*.cs` is recorded as if it were unfiltered, so any unrelated file appearing in the directory invalidates the entry and the common case essentially never hits. Its absence therefore has to disable the feature outright rather than degrade it. MSBuildCache compiles against a reference assembly but runs against whatever Microsoft.Build.dll the host supplies, so a direct property access would not compile today and would throw on older hosts if the compile reference were raised. The property is bound once to a delegate instead, after which each read costs about a virtual call -- worth caring about, since this sits on the file-access reporting path. FileAccessData is a struct. Instance members on a value type take their receiver by reference, so the delegate does too; the alternative of calling PropertyInfo.GetValue per access would box the struct every time. The running MSBuild version is captured alongside it for diagnostics only. The version that first carries the field is not knowable while it is unreleased, so probing for the field tests the exact dependency and stays correct if it is ever serviced back to an older branch. Nothing consumes this yet. --- src/Common.Tests/ByRefGetterFactoryTests.cs | 135 ++++++++++++++++++ src/Common/FileAccess/ByRefGetterFactory.cs | 64 +++++++++ .../FileAccess/FileAccessDataCapabilities.cs | 59 ++++++++ 3 files changed, 258 insertions(+) create mode 100644 src/Common.Tests/ByRefGetterFactoryTests.cs create mode 100644 src/Common/FileAccess/ByRefGetterFactory.cs create mode 100644 src/Common/FileAccess/FileAccessDataCapabilities.cs diff --git a/src/Common.Tests/ByRefGetterFactoryTests.cs b/src/Common.Tests/ByRefGetterFactoryTests.cs new file mode 100644 index 0000000..9ab9069 --- /dev/null +++ b/src/Common.Tests/ByRefGetterFactoryTests.cs @@ -0,0 +1,135 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.MSBuildCache.FileAccess; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.MSBuildCache.Tests; + +/// +/// Covers binding against both shapes of MSBuild's FileAccessData: the older one that lacks +/// the enumeration fields and the newer one that carries them. The real struct comes from whichever +/// Microsoft.Build.dll the host supplies, so stand-in structs are used to exercise both sides +/// without needing two MSBuild installations. +/// +[TestClass] +public sealed class ByRefGetterFactoryTests +{ + private enum StandInAttributes : uint + { + None = 0, + Directory = 0x10, + } + + /// Mirrors the pre-18.10 FileAccessData: no enumeration fields. + private struct OlderFileAccessData + { + private string _path; + + public OlderFileAccessData(string path) => _path = path; + + public string Path + { + readonly get => _path; + private set => _path = value; + } + } + + /// Mirrors the newer FileAccessData, including the readonly-get/private-set shape. + private struct NewerFileAccessData + { + private string _path; + private string? _enumeratePattern; + private StandInAttributes _openedAttributes; + + public NewerFileAccessData(string path, string? enumeratePattern, StandInAttributes openedAttributes) + { + _path = path; + _enumeratePattern = enumeratePattern; + _openedAttributes = openedAttributes; + } + + public string Path + { + readonly get => _path; + private set => _path = value; + } + + public string? EnumeratePattern + { + readonly get => _enumeratePattern; + private set => _enumeratePattern = value; + } + + public StandInAttributes OpenedFileOrDirectoryAttributes + { + readonly get => _openedAttributes; + private set => _openedAttributes = value; + } + } + + [TestMethod] + public void ReturnsNullWhenPropertyIsAbsent() + { + Assert.IsNull(ByRefGetterFactory.TryCreate("EnumeratePattern")); + Assert.IsNull(ByRefGetterFactory.TryCreate("OpenedFileOrDirectoryAttributes")); + } + + [TestMethod] + public void ReturnsNullWhenPropertyTypeDiffers() + { + // Guards against silently binding to a property that was reshaped rather than added. + Assert.IsNull(ByRefGetterFactory.TryCreate("EnumeratePattern")); + } + + [TestMethod] + public void ReadsStringPropertyWithoutBoxing() + { + ByRefGetter? getter = + ByRefGetterFactory.TryCreate("EnumeratePattern"); + Assert.IsNotNull(getter); + + NewerFileAccessData data = new(@"X:\dir", "*.cs", StandInAttributes.Directory); + Assert.AreEqual("*.cs", getter(ref data)); + } + + [TestMethod] + public void ReadsEnumProperty() + { + ByRefGetter? getter = + ByRefGetterFactory.TryCreate("OpenedFileOrDirectoryAttributes"); + Assert.IsNotNull(getter); + + NewerFileAccessData data = new(@"X:\dir", "*.cs", StandInAttributes.Directory); + Assert.AreEqual(StandInAttributes.Directory, getter(ref data)); + } + + [TestMethod] + public void ReadsNullStringProperty() + { + ByRefGetter? getter = + ByRefGetterFactory.TryCreate("EnumeratePattern"); + Assert.IsNotNull(getter); + + NewerFileAccessData data = new(@"X:\dir", enumeratePattern: null, StandInAttributes.None); + Assert.IsNull(getter(ref data)); + } + + /// + /// The getter is bound once and reused across every reported file access, so it must observe the + /// instance it is handed rather than a snapshot captured at bind time. + /// + [TestMethod] + public void BoundGetterIsReusableAcrossInstances() + { + ByRefGetter? getter = + ByRefGetterFactory.TryCreate("EnumeratePattern"); + Assert.IsNotNull(getter); + + NewerFileAccessData first = new(@"X:\a", "*.cs", StandInAttributes.None); + NewerFileAccessData second = new(@"X:\b", "*.dll", StandInAttributes.None); + + Assert.AreEqual("*.cs", getter(ref first)); + Assert.AreEqual("*.dll", getter(ref second)); + } +} diff --git a/src/Common/FileAccess/ByRefGetterFactory.cs b/src/Common/FileAccess/ByRefGetterFactory.cs new file mode 100644 index 0000000..a91c662 --- /dev/null +++ b/src/Common/FileAccess/ByRefGetterFactory.cs @@ -0,0 +1,64 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Reflection; + +namespace Microsoft.MSBuildCache.FileAccess; + +/// +/// Reads a property from a struct without boxing it. Instance members on a value type take their +/// receiver by reference, so an open-instance delegate bound to a struct property getter must too. +/// +internal delegate TResult ByRefGetter(ref TStruct instance) + where TStruct : struct; + +/// +/// Binds open-instance getters for properties that may not exist on the running assembly. +/// +/// +/// MSBuildCache compiles against a reference assembly but runs against whatever Microsoft.Build.dll +/// the host MSBuild supplies, so properties added in newer MSBuild versions cannot be called directly — +/// they would not compile against the reference assembly, and would throw +/// on older hosts if they did. Binding a delegate once and reusing +/// it keeps the per-access cost to roughly a virtual call, which matters because file-access reporting +/// is a very hot path. +/// +internal static class ByRefGetterFactory +{ + /// + /// Binds a getter for , or returns null if the property does + /// not exist or does not have type — i.e. the running assembly + /// predates the property. + /// + public static ByRefGetter? TryCreate(string propertyName) + where TStruct : struct + { + PropertyInfo? property = typeof(TStruct).GetProperty(propertyName, BindingFlags.Public | BindingFlags.Instance); + if (property is null || property.PropertyType != typeof(TResult)) + { + return null; + } + + MethodInfo? getter = property.GetGetMethod(nonPublic: false); + if (getter is null) + { + return null; + } + + try + { +#if NET9_0_OR_GREATER + return getter.CreateDelegate>(); +#else + return (ByRefGetter)getter.CreateDelegate(typeof(ByRefGetter)); +#endif + } + catch (ArgumentException) + { + // The property exists but its getter does not have the expected shape. Treat it as absent + // rather than failing the build. + return null; + } + } +} diff --git a/src/Common/FileAccess/FileAccessDataCapabilities.cs b/src/Common/FileAccess/FileAccessDataCapabilities.cs new file mode 100644 index 0000000..fdf3215 --- /dev/null +++ b/src/Common/FileAccess/FileAccessDataCapabilities.cs @@ -0,0 +1,59 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Reflection; +using Microsoft.Build.Evaluation; +using Microsoft.Build.Experimental.FileAccess; + +namespace Microsoft.MSBuildCache.FileAccess; + +/// +/// Detects whether the running MSBuild reports the file-access fields that probe and enumeration +/// fingerprinting depends on. +/// +/// +/// +/// EnumeratePattern was added to after the MSBuild version +/// MSBuildCache compiles against, so it is read reflectively rather than by a direct property access +/// that would not compile — and, if the compile reference were raised, would throw on every older host. +/// +/// +/// Without it a filtered enumeration such as *.cs is recorded as if it were unfiltered, so any +/// unrelated file appearing in the directory invalidates the entry and the common case essentially +/// never hits. That is why its absence disables the feature outright rather than degrading it. +/// +/// +internal static class FileAccessDataCapabilities +{ + private static readonly ByRefGetter? EnumeratePatternGetter = + ByRefGetterFactory.TryCreate("EnumeratePattern"); + + /// + /// Whether the running MSBuild reports the enumeration pattern. Used to force + /// EnableProbeAndEnumerationFingerprinting off on hosts that cannot support it. + /// + public static bool IsSupported { get; } = EnumeratePatternGetter is not null; + + /// + /// The running MSBuild's file version, for diagnostics only, or null if it cannot be + /// determined. + /// + /// + /// Deliberately not used to decide . The version that first carries the + /// field is not knowable while it is unreleased, and a version comparison that guessed high would + /// enable the feature on a host that cannot actually report patterns. Probing for the field tests + /// the exact thing the feature depends on, and stays correct if the field is ever serviced back to + /// an older branch. + /// + public static string? MSBuildVersion { get; } = + typeof(ProjectCollection).Assembly + .GetCustomAttribute() + ?.InformationalVersion; + + /// + /// The search pattern for a directory enumeration, or null for an unfiltered enumeration, + /// a non-enumeration access, or a host that does not report it. + /// + public static string? GetEnumeratePattern(ref FileAccessData fileAccessData) + => EnumeratePatternGetter is null ? null : EnumeratePatternGetter(ref fileAccessData); +} From c2532ef7f5fa1060c8c1e9a7878f5e3fa04726e1 Mon Sep 17 00:00:00 2001 From: David Federman Date: Fri, 31 Jul 2026 17:20:35 -0700 Subject: [PATCH 04/13] Capture probes and directory enumerations from the sandbox The sandbox has always reported probes and enumerations; the repository logged them and threw them away. They are now classified and kept, which is what makes a cache entry able to depend on a file having been absent or a directory having held a particular set of names. Classification is asymmetric on purpose. A probe becomes AbsentPathProbe only for error codes that mean the path definitively did not exist -- not found, path not found, bad net path, invalid name -- and everything else, including success, becomes ExistingProbe. Treating a transient failure such as ERROR_SHARING_VIOLATION or ERROR_ACCESS_DENIED as absence would let a flaky machine fingerprint the same sources differently between builds. QuickBuild classifies the same way. Enumerate and EnumerationProbe are not the same thing despite the names. Enumerate is reported against the directory and carries the search pattern. EnumerationProbe is reported against each matched child -- FindFirstFileEx's first result and every FindNextFile after it -- so it is an existence probe on that child. Recording it as an enumeration would key a directory-enumeration entry on a file path, which lookup-time validation can never re-validate because it requires the path to still be a directory; every project enumerating this way would then miss permanently. The directory's own Enumerate observation still catches member-list changes. Classification runs before the generic `error != 0` skip, since a probe's whole value is often that it failed. Accesses carrying Read or Write are content accesses and keep flowing to the file table untouched. `*` is normalized to null so an unfiltered enumeration folds to one entry however the caller spelled it, and the observation list is only allocated when the feature is enabled, since probes dominate event volume. Self-outputs are not filtered yet and enumerations carry no member lists, so enumeration entries currently over-invalidate. The next commit addresses both. The feature remains off by default. --- src/Common.Tests/ObservationFilterTests.cs | 70 +++++++++ src/Common/FileAccess/FileAccessRepository.cs | 144 +++++++++++++++--- 2 files changed, 195 insertions(+), 19 deletions(-) create mode 100644 src/Common.Tests/ObservationFilterTests.cs diff --git a/src/Common.Tests/ObservationFilterTests.cs b/src/Common.Tests/ObservationFilterTests.cs new file mode 100644 index 0000000..711ea33 --- /dev/null +++ b/src/Common.Tests/ObservationFilterTests.cs @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.Build.Experimental.FileAccess; +using Microsoft.MSBuildCache.FileAccess; +using Microsoft.MSBuildCache.Fingerprinting; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.MSBuildCache.Tests; + +[TestClass] +public class ObservationFilterTests +{ + [TestMethod] + [DataRow(0u, ObservationType.ExistingProbe, DisplayName = "success")] + [DataRow(2u, ObservationType.AbsentPathProbe, DisplayName = "ERROR_FILE_NOT_FOUND")] + [DataRow(3u, ObservationType.AbsentPathProbe, DisplayName = "ERROR_PATH_NOT_FOUND")] + [DataRow(53u, ObservationType.AbsentPathProbe, DisplayName = "ERROR_BAD_NETPATH")] + [DataRow(123u, ObservationType.AbsentPathProbe, DisplayName = "ERROR_INVALID_NAME")] + [DataRow(5u, ObservationType.ExistingProbe, DisplayName = "ERROR_ACCESS_DENIED is transient, not absence")] + [DataRow(32u, ObservationType.ExistingProbe, DisplayName = "ERROR_SHARING_VIOLATION is transient, not absence")] + [DataRow(1224u, ObservationType.ExistingProbe, DisplayName = "unrecognized error is not absence")] + public void ProbeErrorClassification(uint error, ObservationType expected) + { + Assert.AreEqual( + expected, + FileAccessRepository.ClassifyObservation(RequestedAccess.Probe, error), + "Only definitive not-found codes may classify as absent. Mapping a transient failure such as " + + "ERROR_SHARING_VIOLATION or ERROR_ACCESS_DENIED to AbsentPathProbe would let machine flakiness " + + "change the PathSet and lose cache hits."); + } + + /// + /// is reported against the directory being enumerated, but + /// is reported against each matched child. Recording the + /// latter as a directory enumeration would key the entry on a file path, and since lookup-time + /// validation requires the path to still be a directory, such an entry could never re-validate — every + /// build using FindFirstFileEx/FindNextFile enumeration would permanently miss. + /// + [TestMethod] + public void EnumerationProbeIsAProbeNotADirectoryEnumeration() + { + Assert.AreEqual( + ObservationType.DirectoryEnumeration, + FileAccessRepository.ClassifyObservation(RequestedAccess.Enumerate, 0), + "Enumerate is reported on the directory itself and carries the search pattern."); + + Assert.AreEqual( + ObservationType.ExistingProbe, + FileAccessRepository.ClassifyObservation(RequestedAccess.EnumerationProbe, 0), + "EnumerationProbe is reported per matched child, so it is an existence probe on that child."); + } + + /// + /// is a flags enum, and content access must keep flowing to the normal + /// file-table handling rather than being short-circuited into an observation. + /// + [TestMethod] + [DataRow(RequestedAccess.Read, DisplayName = "Read")] + [DataRow(RequestedAccess.Write, DisplayName = "Write")] + [DataRow(RequestedAccess.ReadWrite, DisplayName = "ReadWrite")] + [DataRow(RequestedAccess.Read | RequestedAccess.Probe, DisplayName = "Read combined with Probe")] + [DataRow(RequestedAccess.None, DisplayName = "None")] + public void ContentAccessIsNotAnObservation(RequestedAccess requestedAccess) + { + Assert.IsNull( + FileAccessRepository.ClassifyObservation(requestedAccess, 0), + "Accesses carrying Read or Write are content accesses and must reach the file table."); + } +} diff --git a/src/Common/FileAccess/FileAccessRepository.cs b/src/Common/FileAccess/FileAccessRepository.cs index 1b56921..9033a05 100644 --- a/src/Common/FileAccess/FileAccessRepository.cs +++ b/src/Common/FileAccess/FileAccessRepository.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft. All rights reserved. +// Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; @@ -65,6 +65,70 @@ public FileAccesses FinishProject(NodeContext nodeContext) private FileAccessesState GetFileAccessesState(NodeContext nodeContext) => _fileAccessStates.GetOrAdd(nodeContext, nodeContext => new FileAccessesState(nodeContext, _logger, _pluginSettings, _processTable)); + // Win32 error codes that mean the probed path definitively did not exist. + private const uint ErrorFileNotFound = 2; + private const uint ErrorPathNotFound = 3; + private const uint ErrorBadNetPath = 53; + private const uint ErrorInvalidName = 123; + + /// + /// Whether a probe's error code means the path definitively did not exist. + /// + /// + /// Classification is deliberately asymmetric: only these codes produce + /// , and everything else — including success and + /// transient failures such as ERROR_SHARING_VIOLATION and ERROR_ACCESS_DENIED — + /// produces . Treating a transient failure as absence + /// would let machine flakiness change the PathSet, so the same sources would fingerprint + /// differently between builds and lose cache hits. This matches the QuickBuild implementation, + /// which the observation schema is kept in sync with. + /// + internal static bool IsKnownAbsentError(uint error) + => error is ErrorFileNotFound or ErrorPathNotFound or ErrorInvalidName or ErrorBadNetPath; + + /// + /// Classifies a reported access as a probe or directory-enumeration observation, or null when + /// it is neither and should flow to the normal content-access handling. + /// + /// + /// + /// is a enum. An access carrying + /// or is content access and is + /// never reclassified as an observation. + /// + /// + /// is reported against the directory itself and carries the + /// search pattern, so it becomes . + /// is reported against each matched child — + /// FindFirstFileEx's first result and every FindNextFile — so it is an existence probe on + /// that child. Recording it as an enumeration would key a + /// on a file path, which can never re-validate + /// because the lookup-time directory check always fails, permanently missing the cache. Member-list + /// changes are still caught by the directory's own observation. + /// + /// + internal static ObservationType? ClassifyObservation(RequestedAccess requestedAccess, uint error) + { + if ((requestedAccess & (RequestedAccess.Read | RequestedAccess.Write)) != 0) + { + return null; + } + + if ((requestedAccess & RequestedAccess.Enumerate) != 0) + { + return ObservationType.DirectoryEnumeration; + } + + if ((requestedAccess & (RequestedAccess.Probe | RequestedAccess.EnumerationProbe)) != 0) + { + return IsKnownAbsentError(error) + ? ObservationType.AbsentPathProbe + : ObservationType.ExistingProbe; + } + + return null; + } + private sealed class FileAccessesState : IDisposable { private readonly object _stateLock = new(); @@ -83,6 +147,10 @@ private sealed class FileAccessesState : IDisposable private List? _deletedDirectories = new(); + // Captured probe and enumeration observations, in arrival order. Null when + // EnableProbeAndEnumerationFingerprinting is off — AddFileAccess uses null as the signal to short-circuit. + private List? _observations; + private long _fileAccessCounter; private bool _isFinished; @@ -98,6 +166,13 @@ public FileAccessesState( _pluginSettings = pluginSettings; _processTable = processTable; + // Only allocate the observations list when the feature flag is on. AddFileAccess uses _observations + // being null vs non-null as the capture-or-skip signal so flag-off doesn't pay any per-probe cost. + if (_pluginSettings.EnableProbeAndEnumerationFingerprinting) + { + _observations = new List(); + } + string logFilePath = Path.Combine(nodeContext.LogDirectory, "fileAccesses.log"); _logFileStream = File.CreateText(logFilePath); } @@ -149,15 +224,6 @@ public void AddFileAccess(FileAccessData fileAccessData) DesiredAccess desiredAccess = fileAccessData.DesiredAccess; ReportedFileOperation operation = fileAccessData.Operation; - // TODO: Remove or uncomment once we figure out whether we want this. - // Ignore these operations as they're a bit too spammy for what we need - //if (operation == ReportedFileOperation.FindFirstFileEx - // || operation == ReportedFileOperation.GetFileAttributes - // || operation == ReportedFileOperation.GetFileAttributesEx) - //{ - // return; - //} - uint processId = fileAccessData.ProcessId; RequestedAccess requestedAccess = fileAccessData.RequestedAccess; uint error = fileAccessData.Error; @@ -180,6 +246,10 @@ public void AddFileAccess(FileAccessData fileAccessData) // Note: This is a hot path, so writing fields one at a time to avoid the overhead of a string.Format with many arguments. _logFileStream.Write(processId); _logFileStream.Write(", "); + _logFileStream.Write(fileAccessData.Id); + _logFileStream.Write(", "); + _logFileStream.Write(fileAccessData.CorrelationId); + _logFileStream.Write(", "); UInt32FlagsFormatter.Write(_logFileStream, (uint)desiredAccess); _logFileStream.Write(", "); UInt32FlagsFormatter.Write(_logFileStream, (uint)flagsAndAttributes); @@ -198,6 +268,40 @@ public void AddFileAccess(FileAccessData fileAccessData) _logFileStream.WriteLine(); + // Classify probes/enumerations BEFORE the generic `error != 0` short-circuit below — probes + // can have not-found errors (ERROR_FILE_NOT_FOUND for AbsentPathProbe) that must not be dropped. + // RemoveDirectory is handled by _deletedDirectories and is not an observation. + ObservationType? observationType = ClassifyObservation(requestedAccess, error); + + if (observationType.HasValue) + { + // Skip capture entirely when the feature flag is off (probes are the majority of events). + if (_observations != null && operation != ReportedFileOperation.RemoveDirectory) + { + string? enumerationPattern = null; + if (observationType.Value == ObservationType.DirectoryEnumeration) + { + // Reaching here means the feature is enabled, which is only possible when the + // host reports the pattern (see FileAccessDataCapabilities). "*" means the + // caller applied no filter, so it is normalized to null to match how an + // unreported pattern is represented — otherwise the same enumeration would + // fold into two distinct entries depending on how the caller spelled it. + enumerationPattern = FileAccessDataCapabilities.GetEnumeratePattern(ref fileAccessData); + if (string.IsNullOrEmpty(enumerationPattern) || enumerationPattern == "*") + { + enumerationPattern = null; + } + } + + _observations.Add(new ObservedAccess(path, observationType.Value, enumerationPattern)); + } + + // Bump the counter so probes participate in event ordering. Unused gaps are benign — + // only relative order matters (the counter governs RemoveDirectory↔write ordering). + _fileAccessCounter++; + return; + } + if (error != 0) { // we don't want to process failing file accesses- logging them with the error code @@ -213,13 +317,6 @@ public void AddFileAccess(FileAccessData fileAccessData) _deletedDirectories.Add(new RemoveDirectoryOperation(_fileAccessCounter, path)); } } - else if (requestedAccess == RequestedAccess.Enumerate - || requestedAccess == RequestedAccess.EnumerationProbe - || requestedAccess == RequestedAccess.Probe) - { - // Don't add enumerations and probes to fileAccessInfo as they are not needed. - // We still want to log them for debugging though which is why they're not filtered earlier. - } else if (_fileTable != null) { if (!_fileTable.TryGetValue(path, out FileAccessInfo? access)) @@ -279,6 +376,7 @@ public FileAccesses FinishProject() { Dictionary fileTable; List deletedDirectories; + List? observations; lock (_stateLock) { _isFinished = true; @@ -286,10 +384,12 @@ public FileAccesses FinishProject() fileTable = _fileTable!; deletedDirectories = _deletedDirectories!; + observations = _observations; // Allow memory to be reclaimed _fileTable = null; _deletedDirectories = null; + _observations = null; if (_pluginSettings.AllowFileAccessAfterProjectFinishFilePatterns.Count == 0 && _pluginSettings.AllowFileAccessAfterProjectFinishProcessPatterns.Count == 0 && @@ -299,7 +399,7 @@ public FileAccesses FinishProject() } } - return ProcessFileAccesses(fileTable, deletedDirectories); + return ProcessFileAccesses(fileTable, deletedDirectories, observations); } private Glob? IsAllowFileAccessAfterProjectFinishFilePatterns(string fileName) => @@ -313,7 +413,8 @@ public FileAccesses FinishProject() private static FileAccesses ProcessFileAccesses( Dictionary fileTable, - List deletedDirectories) + List deletedDirectories, + List? observations) { var outputs = new HashSet(StringComparer.OrdinalIgnoreCase); List allObservations = new(); @@ -350,6 +451,11 @@ private static FileAccesses ProcessFileAccesses( allObservations.Add(new ObservedAccess(filePath, ObservationType.FileContentRead)); } + if (observations != null) + { + allObservations.AddRange(observations); + } + return new FileAccesses(allObservations, outputs); } From 3b07610b0922e7d148806e66e229835f9f043611 Mon Sep 17 00:00:00 2001 From: David Federman Date: Fri, 31 Jul 2026 17:21:20 -0700 Subject: [PATCH 05/13] Subtract a project's own outputs from its observations A project observes its own build in progress. It probes an output before generating it, probes it again afterwards, and enumerates directories it is writing into. Those observations describe intra-build state, not the pre-build state a cache lookup is answering against, and keeping them means the entry can never match: the probe recorded absence, and by the next build the output is there. Two forms of this are handled. Probes and enumerations of any path this project ever wrote are dropped, along with every ancestor directory of such a path. The ancestor walk matters because MSBuild routinely probes an output directory before creating it. "Ever written" is deliberately broader than the output set, which is filtered to existing files and would leak transient temporaries and build-created directories back in. This is project-local, so probes of another project's outputs still count. Enumerations of a directory the project writes into cannot simply be dropped -- the directory usually holds real inputs too. Instead its contents are split into members the project wrote and members it did not. Only the latter go into the fingerprint; the former are recorded separately so lookup can subtract them from what it finds on disk. A project that enumerates a staging directory it also populates then matches whether or not the previous build's outputs are still there, which is the case that makes clean-to-incremental cycles work. Members are captured at this layer rather than at fingerprint time because the self-output set is only known here. The search pattern is compiled through a shared helper. Patterns come from whatever the build passed to the enumeration API, and Windows substitutes wildcards that have no glob equivalent -- DOS_STAR, DOS_QM and DOS_DOT are spelled `<`, `>` and `"` -- which the glob parser rejects, as it does braces and a closing bracket. An unparseable pattern falls back to matching every member. That can only over-invalidate, since the recorded member list becomes a superset; dropping the observation instead would lose the dependency and risk exactly the incorrect hit this feature exists to prevent. The helper is shared so that the capture and validation sides always reach the same conclusion about a pattern. --- src/Common.Tests/ObservationFilterTests.cs | 157 ++++++++++++++ src/Common/FileAccess/FileAccessRepository.cs | 194 +++++++++++++++++- .../DirectoryEnumerationReader.cs | 61 ++++++ 3 files changed, 407 insertions(+), 5 deletions(-) create mode 100644 src/Common/Fingerprinting/DirectoryEnumerationReader.cs diff --git a/src/Common.Tests/ObservationFilterTests.cs b/src/Common.Tests/ObservationFilterTests.cs index 711ea33..a51496c 100644 --- a/src/Common.Tests/ObservationFilterTests.cs +++ b/src/Common.Tests/ObservationFilterTests.cs @@ -1,6 +1,9 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; +using System.Collections.Generic; +using System.Linq; using Microsoft.Build.Experimental.FileAccess; using Microsoft.MSBuildCache.FileAccess; using Microsoft.MSBuildCache.Fingerprinting; @@ -67,4 +70,158 @@ public void ContentAccessIsNotAnObservation(RequestedAccess requestedAccess) FileAccessRepository.ClassifyObservation(requestedAccess, 0), "Accesses carrying Read or Write are content accesses and must reach the file table."); } + + [TestMethod] + public void TrimTrailingSeparatorRemovesBackslash() + { + Assert.AreEqual(@"X:\foo\bar", FileAccessRepository.TrimTrailingSeparator(@"X:\foo\bar\")); + } + + [TestMethod] + public void TrimTrailingSeparatorRemovesForwardSlash() + { + Assert.AreEqual("X:/foo/bar", FileAccessRepository.TrimTrailingSeparator("X:/foo/bar/")); + } + + [TestMethod] + public void TrimTrailingSeparatorNoOpWhenAbsent() + { + Assert.AreEqual(@"X:\foo\bar", FileAccessRepository.TrimTrailingSeparator(@"X:\foo\bar")); + } + + [TestMethod] + public void TrimTrailingSeparatorEmpty() + { + Assert.AreEqual(string.Empty, FileAccessRepository.TrimTrailingSeparator(string.Empty)); + } + + [TestMethod] + public void BuildEverWrittenOrAncestorSetIncludesAllAncestors() + { + HashSet result = FileAccessRepository.BuildEverWrittenOrAncestorSet(new List + { + @"X:\Repo\bin\Debug\net9.0\TestProject.dll", + }); + + // Must include the file itself plus every ancestor up to drive root. + Assert.IsTrue(result.Contains(@"X:\Repo\bin\Debug\net9.0\TestProject.dll")); + Assert.IsTrue(result.Contains(@"X:\Repo\bin\Debug\net9.0")); + Assert.IsTrue(result.Contains(@"X:\Repo\bin\Debug")); + Assert.IsTrue(result.Contains(@"X:\Repo\bin")); + Assert.IsTrue(result.Contains(@"X:\Repo")); + Assert.IsTrue(result.Contains(@"X:\")); + } + + [TestMethod] + public void BuildEverWrittenOrAncestorSetDeduplicatesSharedAncestors() + { + HashSet result = FileAccessRepository.BuildEverWrittenOrAncestorSet(new List + { + @"X:\Repo\bin\Debug\net9.0\TestProject.dll", + @"X:\Repo\bin\Debug\net9.0\TestProject.pdb", + }); + + // Two paths share most of the ancestor chain — they shouldn't double-count. + // Specifically, the early-exit when an ancestor is already in the set should kick in for + // the second path at "X:\Repo\bin\Debug\net9.0". + int net9Count = result.Count(p => string.Equals(p, @"X:\Repo\bin\Debug\net9.0", StringComparison.OrdinalIgnoreCase)); + Assert.AreEqual(1, net9Count, "Shared ancestor must appear exactly once."); + } + + [TestMethod] + public void BuildEverWrittenOrAncestorSetCaseInsensitive() + { + HashSet result = FileAccessRepository.BuildEverWrittenOrAncestorSet(new List + { + @"X:\Repo\BIN\Debug\TestProject.dll", + @"X:\repo\bin\debug\Other.dll", + }); + + // Case-insensitive comparison: both files contribute their ancestor chains, but the chains + // share the same logical entries (just different casings). + // Both files plus shared ancestor chain @ "X:\Repo\BIN\Debug" + "X:\Repo\BIN" + "X:\Repo" + "X:\" + // First write's ancestors get added with their casing; second write's ancestors are deduped via + // OrdinalIgnoreCase. + Assert.AreEqual(6, result.Count); + } + + [TestMethod] + public void BuildEverWrittenOrAncestorSetTrimsTrailingSeparator() + { + // Caller-supplied paths may have a trailing separator (e.g., directory writes recorded as + // "X:\Repo\bin\Debug\net9.0\"). BuildEverWrittenOrAncestorSet must trim once on entry so the + // ancestor walk does not produce an off-by-one parent of the same logical directory. + HashSet result = FileAccessRepository.BuildEverWrittenOrAncestorSet(new List + { + @"X:\Repo\bin\Debug\net9.0\", + }); + + Assert.IsTrue(result.Contains(@"X:\Repo\bin\Debug\net9.0")); + Assert.IsFalse(result.Contains(@"X:\Repo\bin\Debug\net9.0\")); + Assert.IsTrue(result.Contains(@"X:\Repo\bin\Debug")); + Assert.IsTrue(result.Contains(@"X:\Repo\bin")); + Assert.IsTrue(result.Contains(@"X:\Repo")); + Assert.IsTrue(result.Contains(@"X:\")); + } + + [TestMethod] + public void BuildEverWrittenOrAncestorSetEmptyInput() + { + HashSet result = FileAccessRepository.BuildEverWrittenOrAncestorSet(new List()); + Assert.AreEqual(0, result.Count); + } + + [TestMethod] + public void SelfOutputProbeIsExcluded() + { + HashSet everWrittenOrAncestor = FileAccessRepository.BuildEverWrittenOrAncestorSet(new List + { + @"X:\Repo\staging\Generated.dll", + }); + HashSet everWritten = new(StringComparer.OrdinalIgnoreCase) + { + @"X:\Repo\staging\Generated.dll", + }; + + var observation = new ObservedAccess(@"X:\Repo\staging", ObservationType.ExistingProbe); + + Assert.IsTrue(FileAccessRepository.ShouldExcludeSelfOutputObservation(observation, everWritten, everWrittenOrAncestor)); + } + + [TestMethod] + public void SelfOutputDirectoryEnumerationIsRetainedForPartitioning() + { + HashSet everWrittenOrAncestor = FileAccessRepository.BuildEverWrittenOrAncestorSet(new List + { + @"X:\Repo\staging\Generated.dll", + }); + HashSet everWritten = new(StringComparer.OrdinalIgnoreCase) + { + @"X:\Repo\staging\Generated.dll", + }; + + var observation = new ObservedAccess(@"X:\Repo\staging", ObservationType.DirectoryEnumeration); + + Assert.IsFalse( + FileAccessRepository.ShouldExcludeSelfOutputObservation(observation, everWritten, everWrittenOrAncestor), + "Directory enumerations must reach PartitionDirectoryMembers so external members remain fingerprint dependencies."); + } + + [TestMethod] + public void SelfCreatedDirectoryEnumerationIsExcluded() + { + var writtenPaths = new List + { + @"X:\Repo\obj\generated", + @"X:\Repo\obj\generated\Generated.gen", + }; + HashSet everWrittenOrAncestor = FileAccessRepository.BuildEverWrittenOrAncestorSet(writtenPaths); + HashSet everWritten = new(writtenPaths, StringComparer.OrdinalIgnoreCase); + + var observation = new ObservedAccess(@"X:\Repo\obj\generated", ObservationType.DirectoryEnumeration); + + Assert.IsTrue( + FileAccessRepository.ShouldExcludeSelfOutputObservation(observation, everWritten, everWrittenOrAncestor), + "A directory created by the project has no pre-build membership state to validate at lookup."); + } } diff --git a/src/Common/FileAccess/FileAccessRepository.cs b/src/Common/FileAccess/FileAccessRepository.cs index 9033a05..f3b3eb5 100644 --- a/src/Common/FileAccess/FileAccessRepository.cs +++ b/src/Common/FileAccess/FileAccessRepository.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft. All rights reserved. +// Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; @@ -65,6 +65,67 @@ public FileAccesses FinishProject(NodeContext nodeContext) private FileAccessesState GetFileAccessesState(NodeContext nodeContext) => _fileAccessStates.GetOrAdd(nodeContext, nodeContext => new FileAccessesState(nodeContext, _logger, _pluginSettings, _processTable)); + /// + /// Builds the "ever-written or ancestor-of-written" set used to filter self-output probe + /// observations. For every path in , the result includes the path itself + /// plus every ancestor directory back to the root. This extends the self-output probe filter with + /// ancestor-dir handling — MSBuild commonly probes output directories (e.g., bin\Debug\net9.0\) + /// before creating them, and we need to suppress those probes so re-observation at cache lookup doesn't + /// flip them to ExistingProbe after the build creates the directory. + /// + internal static HashSet BuildEverWrittenOrAncestorSet(List writtenPaths) + { + HashSet result = new(StringComparer.OrdinalIgnoreCase); + foreach (string path in writtenPaths) + { + string normalizedPath = TrimTrailingSeparator(path); + result.Add(normalizedPath); + + // Path.GetDirectoryName output is already trim-normalized (no trailing separator except at drive + // roots like "C:\", which then yields null on the next call and terminates the walk). So we only + // need to trim caller-supplied input once, not at every level. + string? ancestor = Path.GetDirectoryName(normalizedPath); + while (!string.IsNullOrEmpty(ancestor)) + { + if (!result.Add(ancestor!)) + { + // Already in the set — every shallower ancestor is too. Stop the walk. + break; + } + + ancestor = Path.GetDirectoryName(ancestor); + } + } + + return result; + } + + /// + /// Returns whether an observation should be removed because it targets a path written by this project + /// or an ancestor of one. Directory enumerations are retained when the project only wrote members + /// beneath them, so external members can be separated from self-outputs, but are removed when the + /// project created the directory itself. + /// + internal static bool ShouldExcludeSelfOutputObservation( + ObservedAccess observation, + HashSet everWritten, + HashSet everWrittenOrAncestor) + { + string normalizedPath = TrimTrailingSeparator(observation.Path); + return observation.Type == ObservationType.DirectoryEnumeration + ? everWritten.Contains(normalizedPath) + : everWrittenOrAncestor.Contains(normalizedPath); + } + + /// + /// Trims a single trailing directory separator (forward or back slash) from the given path. No-op when + /// the path doesn't end with a separator. + /// + internal static string TrimTrailingSeparator(string path) + => path.Length > 0 && (path[path.Length - 1] == Path.DirectorySeparatorChar || path[path.Length - 1] == Path.AltDirectorySeparatorChar) + ? path.Substring(0, path.Length - 1) + : path; + // Win32 error codes that mean the probed path definitively did not exist. private const uint ErrorFileNotFound = 2; private const uint ErrorPathNotFound = 3; @@ -399,7 +460,11 @@ public FileAccesses FinishProject() } } - return ProcessFileAccesses(fileTable, deletedDirectories, observations); + return ProcessFileAccesses( + fileTable, + deletedDirectories, + observations, + _pluginSettings.IgnoredInputPatterns); } private Glob? IsAllowFileAccessAfterProjectFinishFilePatterns(string fileName) => @@ -414,7 +479,8 @@ public FileAccesses FinishProject() private static FileAccesses ProcessFileAccesses( Dictionary fileTable, List deletedDirectories, - List? observations) + List? observations, + IReadOnlyCollection ignoredInputPatterns) { var outputs = new HashSet(StringComparer.OrdinalIgnoreCase); List allObservations = new(); @@ -451,14 +517,132 @@ private static FileAccesses ProcessFileAccesses( allObservations.Add(new ObservedAccess(filePath, ObservationType.FileContentRead)); } - if (observations != null) + // Drop probe observations for paths written by this project — post-write probes reflect + // intra-build state, not the pre-build state that drives cache lookup. Keeping them would + // cause false misses for probe-then-write patterns (e.g. a target + // that probes `obj/A.GeneratedCode.cs`, generates it if missing, then re-probes). + // + // We use a broader "ever-written" set than `outputs`: outputs filters to existing-file outputs + // only, which would leak transient temp files, build-created directories, and orphan-parent + // files. Cross-project probes still survive — this is project-local, not graph-wide. + // + // Ancestor directories of every written file are also filtered: MSBuild commonly probes an + // output directory (e.g., `bin\Debug\net9.0\`) before creating it; without this filter, + // re-observation would promote the cached AbsentPathProbe to ExistingProbe and miss. + // + // For DirectoryEnumeration observations that survive the filter, PartitionDirectoryMembers + // splits the directory's contents into Members (external) and WrittenMembers (self-outputs). + // At lookup, the WrittenMembers list cancels whatever the previous build wrote, so cache hits + // remain correct whether outputs are still on disk or not. + if (observations != null && observations.Count > 0) { - allObservations.AddRange(observations); + // Single pass over fileTable: collect writtenPaths and build a per-directory leaf-name + // index of self-writes so we can partition each surviving DirectoryEnumeration's members + // in O(membersInDir) time. + List writtenPaths = new(); + Dictionary> writtenLeafNamesByDir = new(StringComparer.OrdinalIgnoreCase); + foreach (KeyValuePair kvp in fileTable) + { + if (!EverWritten(kvp.Value)) + { + continue; + } + + string writtenPath = kvp.Key; + writtenPaths.Add(writtenPath); + + string parent = TrimTrailingSeparator(Path.GetDirectoryName(writtenPath) ?? string.Empty); + string leaf = Path.GetFileName(writtenPath); + if (string.IsNullOrEmpty(parent) || string.IsNullOrEmpty(leaf)) + { + continue; + } + + if (!writtenLeafNamesByDir.TryGetValue(parent, out HashSet? leafSet)) + { + leafSet = new HashSet(StringComparer.OrdinalIgnoreCase); + writtenLeafNamesByDir[parent] = leafSet; + } + + leafSet.Add(leaf); + } + + HashSet everWritten = new( + writtenPaths.Select(TrimTrailingSeparator), + StringComparer.OrdinalIgnoreCase); + HashSet everWrittenOrAncestor = BuildEverWrittenOrAncestorSet(writtenPaths); + + foreach (ObservedAccess obs in observations) + { + if (ShouldExcludeSelfOutputObservation(obs, everWritten, everWrittenOrAncestor)) + { + continue; + } + + if (obs.Type != ObservationType.DirectoryEnumeration) + { + allObservations.Add(obs); + continue; + } + + // Enrich the DirectoryEnumeration observation with the partitioned member lists. + string dirAbsolute = TrimTrailingSeparator(obs.Path); + HashSet selfOutputLeafNames = writtenLeafNamesByDir.TryGetValue(dirAbsolute, out HashSet? leaves) + ? leaves + : new HashSet(StringComparer.OrdinalIgnoreCase); + + (IReadOnlyList? members, IReadOnlyList? writtenMembers) = PartitionDirectoryMembers( + dirAbsolute, + obs.EnumerationPattern, + selfOutputLeafNames, + ignoredInputPatterns); + allObservations.Add(obs with { Members = members, WrittenMembers = writtenMembers }); + } } return new FileAccesses(allObservations, outputs); } + /// + /// Enumerates the directory and partitions members into Members (external dependencies) and + /// WrittenMembers (this build's outputs). Both lists are leaf names, sorted + /// OrdinalIgnoreCase. Returns (null, null) if the directory is missing or inaccessible. + /// + private static (IReadOnlyList? Members, IReadOnlyList? WrittenMembers) PartitionDirectoryMembers( + string absoluteDirectoryPath, + string? enumerationPattern, + HashSet selfOutputLeafNames, + IReadOnlyCollection ignoredInputPatterns) + { + IReadOnlyList? enumeratedMembers = + DirectoryEnumerationReader.EnumerateLeafNames( + absoluteDirectoryPath, + enumerationPattern, + ignoredInputPatterns); + if (enumeratedMembers is null) + { + return (null, null); + } + + List members = new(); + List writtenMembers = new(); + foreach (string leaf in enumeratedMembers) + { + if (selfOutputLeafNames.Contains(leaf)) + { + writtenMembers.Add(leaf); + } + else + { + members.Add(leaf); + } + } + + members.Sort(StringComparer.OrdinalIgnoreCase); + writtenMembers.Sort(StringComparer.OrdinalIgnoreCase); + return (members, writtenMembers); + } + private static bool IsOutput(FileAccessInfo fileInfo) { // Ignore temporary files: files that were created but deleted later diff --git a/src/Common/Fingerprinting/DirectoryEnumerationReader.cs b/src/Common/Fingerprinting/DirectoryEnumerationReader.cs new file mode 100644 index 0000000..d517d96 --- /dev/null +++ b/src/Common/Fingerprinting/DirectoryEnumerationReader.cs @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Generic; +using System.IO; +using BuildXL.Native.IO; +using DotNet.Globbing; + +namespace Microsoft.MSBuildCache.Fingerprinting; + +/// +/// Enumerates directory member names with the exact Win32 search pattern reported by the sandbox. +/// +internal static class DirectoryEnumerationReader +{ + /// + /// Returns the matching, non-ignored leaf names, or null when the directory could not be + /// enumerated. + /// + /// + /// A general-purpose glob is not equivalent to Win32 matching: for example, Win32 *.* also + /// matches extensionless names. BuildXL's native filesystem layer preserves those semantics, + /// including DOS_STAR, DOS_QM, and DOS_DOT patterns, on both target frameworks. + /// + public static IReadOnlyList? EnumerateLeafNames( + string absoluteDirectoryPath, + string? enumerationPattern, + IReadOnlyCollection? ignoredInputPatterns = null) + { + var members = new List(); + string searchPattern = string.IsNullOrEmpty(enumerationPattern) ? "*" : enumerationPattern!; + + EnumerateDirectoryResult result = FileUtilities.EnumerateDirectoryEntries( + absoluteDirectoryPath, + recursive: false, + searchPattern, + (_, name, _) => + { + string absoluteMemberPath = Path.Combine(absoluteDirectoryPath, name); + if (ignoredInputPatterns is null || !MatchesAny(absoluteMemberPath, ignoredInputPatterns)) + { + members.Add(name); + } + }); + + return result.Succeeded ? members : null; + } + + private static bool MatchesAny(string path, IReadOnlyCollection patterns) + { + foreach (Glob pattern in patterns) + { + if (pattern.IsMatch(path)) + { + return true; + } + } + + return false; + } +} From b308e07f14f8283c172c336334b4d29ce34f3f06 Mon Sep 17 00:00:00 2001 From: David Federman Date: Fri, 31 Jul 2026 17:21:57 -0700 Subject: [PATCH 06/13] Validate probes and enumerations against the filesystem at lookup Everything so far records observations. Nothing yet checks them, and the strong fingerprint cannot: a probe entry contributes only its identity and an enumeration entry hashes the member list stored on itself, neither of which touches the disk. Recomputing the fingerprint from a cached PathSet therefore always reproduces the fingerprint stored in the selector no matter what the filesystem now holds. This check is what enforces those entries, and cache lookup gains nothing by reaching it any earlier than it must. That is the opposite of how it reads. For file content reads the same check is purely an optimization, because their bytes are hashed into the fingerprint and a change is caught by the comparison regardless. Someone could reasonably conclude the whole thing is redundant and delete it, expecting to pay in cache hits; they would instead start silently producing incorrect ones. Both call sites say so. An existing probe requires the path to still exist and an absent probe requires it to still be missing -- the latter is the clean-to-dirty case that motivated the feature, where a file the build only checked for has since appeared. Directory enumerations re-enumerate, subtract the member names the project wrote during the populate build, and compare what remains against the recorded members. Subtracting the self-outputs is what lets the comparison hold whether or not the previous build's outputs are still on disk. A recorded member list of null is not an empty directory: it is how the capture side records a directory it could not enumerate at all, and the fingerprint already encodes the two differently. So it is validated as absence rather than by re-enumeration. Re-enumerating would report a miss precisely because nothing had changed -- the directory is still missing -- making any project that enumerates a path that does not exist permanently uncacheable, which is an ordinary consequence of a wildcard over an absent directory. In the other direction the member comparison treats null and empty alike, so falling through would let a directory that did not exist and now exists empty validate as unchanged. An enumeration that fails mid-way returns no observation at all and forces a miss, so an IO error can never be mistaken for an empty directory. --- src/Common.Tests/FingerprintFactoryTests.cs | 460 +++++++++++++++++- src/Common/Caching/CacheClient.cs | 11 +- .../Fingerprinting/FingerprintFactory.cs | 174 +++++++ .../Fingerprinting/IFingerprintFactory.cs | 2 + 4 files changed, 645 insertions(+), 2 deletions(-) diff --git a/src/Common.Tests/FingerprintFactoryTests.cs b/src/Common.Tests/FingerprintFactoryTests.cs index 3621751..7c008c6 100644 --- a/src/Common.Tests/FingerprintFactoryTests.cs +++ b/src/Common.Tests/FingerprintFactoryTests.cs @@ -205,7 +205,9 @@ public async Task StrongFingerprintExistingProbeIgnoresContentChange() /// /// Adding a member to an enumerated directory must change the strong fingerprint. Under the /// schema-driven model the strong-FP comes from the entry's Members field directly, - /// so two PathSets that differ only in their Members lists produce different fingerprints. + /// so two PathSets that differ only in their Members lists produce different fingerprints. (Detection + /// of filesystem changes between populate and lookup happens via MatchesCurrentState — see + /// MatchesCurrentStateDirectoryEnumerationDetectsExternalMemberAdded.) /// [TestMethod] public async Task StrongFingerprintDirectoryMemberHashDetectsAddition() @@ -372,6 +374,462 @@ public async Task StrongFingerprintIsDeterministic() "Strong fingerprint must be deterministic across factory instances."); } + // ========================================================================================= + // MatchesCurrentState: cheap probe/enumeration verification at cache lookup time. + // + // Returns true if every non-FCR observation in the cached PathSet still matches current filesystem + // state. The caller (CacheClient) combines this with a standard strong-FP comparison: if + // MatchesCurrentState is false, skip the selector entirely; if true, compute the strong FP from the + // cached PathSet (which hashes current FCR content) and compare to the cached selector's FP. + // ========================================================================================= + + /// + /// Cached AbsentPathProbe with current path now present → MatchesCurrentState returns false (cache MISS + /// without strong-FP computation). This is the clean→dirty→miss fix. + /// + [TestMethod] + public void MatchesCurrentStateAbsentBecomesPresentReturnsFalse() + { + using TempDirectory tempRepo = TempDirectory.Create("MSBuildCacheTest-MatchAbsentBecomesPresent"); + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(tempRepo.Path, @"X:\Nuget"); + + PathSet cachedPathSet = new(new List + { + new(@"{RepoRoot}\generated.txt", ObservationType.AbsentPathProbe), + }); + + // The path now exists on disk; the cached AbsentPathProbe no longer reflects reality. + WriteTextSync(Path.Combine(tempRepo.Path, "generated.txt"), "now exists"); + + Assert.IsFalse(CreateFactory(hasher, pathNormalizer: pathNormalizer).MatchesCurrentState(cachedPathSet)); + } + + /// + /// Cached ExistingProbe with current path now absent → MatchesCurrentState returns false. + /// + [TestMethod] + public void MatchesCurrentStateExistingBecomesAbsentReturnsFalse() + { + using TempDirectory tempRepo = TempDirectory.Create("MSBuildCacheTest-MatchExistingBecomesAbsent"); + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(tempRepo.Path, @"X:\Nuget"); + + string filePath = Path.Combine(tempRepo.Path, "tooling.dll"); + WriteTextSync(filePath, ""); + + PathSet cachedPathSet = new(new List + { + new(@"{RepoRoot}\tooling.dll", ObservationType.ExistingProbe), + }); + + File.Delete(filePath); + + Assert.IsFalse(CreateFactory(hasher, pathNormalizer: pathNormalizer).MatchesCurrentState(cachedPathSet)); + } + + /// + /// State unchanged → MatchesCurrentState returns true, AND the strong fingerprint of the cached PathSet + /// equals the populate-time fingerprint. This is the cache-hit case. + /// + [TestMethod] + public async Task MatchesCurrentStateUnchangedHitsAndFingerprintMatches() + { + using TempDirectory tempRepo = TempDirectory.Create("MSBuildCacheTest-MatchUnchanged"); + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(tempRepo.Path, @"X:\Nuget"); + + WriteTextSync(Path.Combine(tempRepo.Path, "stable.dll"), ""); + + PathSet cachedPathSet = new(new List + { + new(@"{RepoRoot}\stable.dll", ObservationType.ExistingProbe), + new(@"{RepoRoot}\never-existed.dll", ObservationType.AbsentPathProbe), + }); + + FingerprintFactory factoryAtPopulate = CreateFactory(hasher, pathNormalizer: pathNormalizer); + Fingerprint? populateFp = await factoryAtPopulate.GetStrongFingerprintAsync(cachedPathSet); + + FingerprintFactory factoryAtLookup = CreateFactory(hasher, pathNormalizer: pathNormalizer); + Assert.IsTrue(factoryAtLookup.MatchesCurrentState(cachedPathSet), + "MatchesCurrentState must return true when probes still match reality."); + + Fingerprint? lookupFp = await factoryAtLookup.GetStrongFingerprintAsync(cachedPathSet); + Assert.IsNotNull(populateFp); + Assert.IsNotNull(lookupFp); + CollectionAssert.AreEqual(populateFp.Hash, lookupFp.Hash, + "Cached strong FP must match recomputed FP when state is unchanged."); + } + + /// + /// Three-build clean → dirty → clean cycle. MatchesCurrentState must return true → false → true across + /// the cycle, and the Build 3 fingerprint matches the Build 1 fingerprint (cache hit recovered). + /// + [TestMethod] + public async Task MatchesCurrentStateCleanDirtyCleanCycleRecoversHit() + { + using TempDirectory tempRepo = TempDirectory.Create("MSBuildCacheTest-MatchCleanDirtyClean"); + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(tempRepo.Path, @"X:\Nuget"); + + string fsPath = Path.Combine(tempRepo.Path, "ephemeral.dat"); + PathSet cachedPathSet = new(new List + { + new(@"{RepoRoot}\ephemeral.dat", ObservationType.AbsentPathProbe), + }); + + // Build 1: clean, file absent → matches → cache the strong FP. + FingerprintFactory build1 = CreateFactory(hasher, pathNormalizer: pathNormalizer); + Assert.IsTrue(build1.MatchesCurrentState(cachedPathSet)); + Fingerprint? build1Fp = await build1.GetStrongFingerprintAsync(cachedPathSet); + + // Build 2: dirty, file appears → MatchesCurrentState false; skip selector. + WriteTextSync(fsPath, ""); + FingerprintFactory build2 = CreateFactory(hasher, pathNormalizer: pathNormalizer); + Assert.IsFalse(build2.MatchesCurrentState(cachedPathSet)); + + // Build 3: clean again, file removed → MatchesCurrentState true again; FP recomputes to build1's. + File.Delete(fsPath); + FingerprintFactory build3 = CreateFactory(hasher, pathNormalizer: pathNormalizer); + Assert.IsTrue(build3.MatchesCurrentState(cachedPathSet)); + Fingerprint? build3Fp = await build3.GetStrongFingerprintAsync(cachedPathSet); + + Assert.IsNotNull(build1Fp); + Assert.IsNotNull(build3Fp); + CollectionAssert.AreEqual(build1Fp.Hash, build3Fp.Hash, + "Build 3 (clean again) must produce the Build 1 fingerprint so the cache hit is recovered."); + } + + /// + /// All-FCR PathSet → MatchesCurrentState returns true vacuously (no probes/enums to check); FCR + /// validation happens implicitly via the strong-FP computation's content hashing. + /// + [TestMethod] + public void MatchesCurrentStateAllFileContentReadReturnsTrue() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + + PathSet cachedPathSet = new(new List + { + new(@"{RepoRoot}\a.cs", ObservationType.FileContentRead), + new(@"{RepoRoot}\b.cs", ObservationType.FileContentRead), + }); + + Assert.IsTrue(CreateFactory(hasher).MatchesCurrentState(cachedPathSet)); + } + + /// + /// Null or empty cached PathSet → MatchesCurrentState returns true vacuously. + /// + [TestMethod] + public void MatchesCurrentStateNullOrEmptyReturnsTrue() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + FingerprintFactory factory = CreateFactory(hasher); + + Assert.IsTrue(factory.MatchesCurrentState(null)); + Assert.IsTrue(factory.MatchesCurrentState(new PathSet(new List()))); + } + + [TestMethod] + public void MatchesCurrentStateUnknownObservationTypeReturnsFalse() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + PathSet cachedPathSet = new(new List + { + new(@"{RepoRoot}\future-observation", (ObservationType)byte.MaxValue), + }); + + Assert.IsFalse( + CreateFactory(hasher).MatchesCurrentState(cachedPathSet), + "Observation types unknown to this client must force a miss rather than silently losing a dependency."); + } + + // ========================================================================================= + // Self-output enumeration cycle — the schema-driven fix. + // + // These tests pin the load-bearing property: when a project enumerates a directory it also + // writes into, populate-time captures the partition into Members (external dependency) and + // WrittenMembers (project's own outputs). Lookup-time MatchesCurrentState subtracts cached.WrittenMembers + // from the current contents and compares against cached.Members — which makes cache hits + // robust to the previous build's outputs being either present (incremental) or absent (clean). + // ========================================================================================= + + /// + /// The headline cycle case: cached PathSet has DirectoryEnumeration with WrittenMembers=[Foo.dll, Foo.pdb] + /// (the project's self-outputs) and Members=[] (no external dependency members). MatchesCurrentState + /// against an empty directory (clean state) subtracts cached.WrittenMembers from {} → effective is [] + /// → matches cached.Members → returns true. + /// + [TestMethod] + public void MatchesCurrentStateDirectoryEnumerationCleanStateMatchesCachedSelfOutputs() + { + using TempDirectory tempRepo = TempDirectory.Create("MSBuildCacheTest-MatchDirEnumCleanCycle"); + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(tempRepo.Path, @"X:\Nuget"); + + string normalizedDir = pathNormalizer.Normalize(tempRepo.Path); + PathSet cachedPathSet = new(new List + { + new(normalizedDir, ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: Array.Empty(), + writtenMembers: new[] { "Foo.dll", "Foo.pdb" }), + }); + + // Lookup with empty directory (clean rebuild): subtract WrittenMembers from {} → [] → matches. + Assert.IsTrue(CreateFactory(hasher, pathNormalizer: pathNormalizer).MatchesCurrentState(cachedPathSet), + "MatchesCurrentState must return true for an empty directory when cached.WrittenMembers cancels out the previous build's contribution."); + + // Lookup with leftover outputs from the previous build (incremental scenario): subtract + // {Foo.dll, Foo.pdb} → [] → matches. + WriteTextSync(Path.Combine(tempRepo.Path, "Foo.dll"), ""); + WriteTextSync(Path.Combine(tempRepo.Path, "Foo.pdb"), ""); + + Assert.IsTrue(CreateFactory(hasher, pathNormalizer: pathNormalizer).MatchesCurrentState(cachedPathSet), + "MatchesCurrentState must return true for an incremental rebuild where leftover self-outputs are still on disk."); + } + + /// + /// External-member regression: if a NEW non-self-output file appears in the enumerated directory + /// between populate and lookup, MatchesCurrentState must return false. + /// + [TestMethod] + public void MatchesCurrentStateDirectoryEnumerationDetectsExternalMemberAdded() + { + using TempDirectory tempRepo = TempDirectory.Create("MSBuildCacheTest-MatchDirEnumExternalAdded"); + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(tempRepo.Path, @"X:\Nuget"); + + string normalizedDir = pathNormalizer.Normalize(tempRepo.Path); + PathSet cachedPathSet = new(new List + { + new(normalizedDir, ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: Array.Empty(), + writtenMembers: new[] { "Foo.dll" }), + }); + + // A sibling project's output now sits in the directory. + WriteTextSync(Path.Combine(tempRepo.Path, "Sibling.dll"), ""); + + Assert.IsFalse(CreateFactory(hasher, pathNormalizer: pathNormalizer).MatchesCurrentState(cachedPathSet)); + } + + /// + /// Stable external membership: cached Members=[Manifest.json], WrittenMembers=[Foo.dll]. Both clean + /// (only Manifest.json) and incremental (Manifest.json + Foo.dll) states match. + /// + [TestMethod] + public void MatchesCurrentStateDirectoryEnumerationStableExternalMembers() + { + using TempDirectory tempRepo = TempDirectory.Create("MSBuildCacheTest-MatchDirEnumStableExternal"); + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(tempRepo.Path, @"X:\Nuget"); + + string normalizedDir = pathNormalizer.Normalize(tempRepo.Path); + PathSet cachedPathSet = new(new List + { + new(normalizedDir, ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "Manifest.json" }, + writtenMembers: new[] { "Foo.dll" }), + }); + + WriteTextSync(Path.Combine(tempRepo.Path, "Manifest.json"), "{}"); + Assert.IsTrue(CreateFactory(hasher, pathNormalizer: pathNormalizer).MatchesCurrentState(cachedPathSet), + "Clean lookup with stable external member must match."); + + WriteTextSync(Path.Combine(tempRepo.Path, "Foo.dll"), ""); + Assert.IsTrue(CreateFactory(hasher, pathNormalizer: pathNormalizer).MatchesCurrentState(cachedPathSet), + "Incremental lookup with stable external member must match."); + } + + /// + /// External-member removed: cached Members=[Manifest.json], WrittenMembers=[Foo.dll]. If the external + /// file disappears between populate and lookup, MatchesCurrentState must return false. + /// + [TestMethod] + public void MatchesCurrentStateDirectoryEnumerationDetectsExternalMemberRemoved() + { + using TempDirectory tempRepo = TempDirectory.Create("MSBuildCacheTest-MatchDirEnumExternalRemoved"); + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(tempRepo.Path, @"X:\Nuget"); + + string normalizedDir = pathNormalizer.Normalize(tempRepo.Path); + PathSet cachedPathSet = new(new List + { + new(normalizedDir, ObservationType.DirectoryEnumeration, enumerationPattern: null, + members: new[] { "Manifest.json" }, + writtenMembers: new[] { "Foo.dll" }), + }); + + // Lookup with NEITHER file present — the external dependency Manifest.json is gone. + Assert.IsFalse(CreateFactory(hasher, pathNormalizer: pathNormalizer).MatchesCurrentState(cachedPathSet)); + } + + /// + /// Cached DirectoryEnumeration of a path that is no longer a directory at lookup time → returns false. + /// + [TestMethod] + public void MatchesCurrentStateDirectoryEnumerationOfAbsentDirReturnsFalse() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + var pathNormalizer = new PathNormalizer(@"X:\NonExistentRepo", @"X:\Nuget"); + + PathSet cachedPathSet = new(new List + { + new(@"{RepoRoot}\definitely-not-here", ObservationType.DirectoryEnumeration, + enumerationPattern: null, + members: Array.Empty(), + writtenMembers: null), + }); + + Assert.IsFalse(CreateFactory(hasher, pathNormalizer: pathNormalizer).MatchesCurrentState(cachedPathSet)); + } + + /// + /// A directory that was absent when the entry was produced, and is still absent, must re-validate. + /// PartitionDirectoryMembers records a null member list for a directory it could not enumerate, + /// and ComputeDirectoryMemberHash encodes that as distinct from an empty one, so the null case + /// has to be validated as absence. Validating it by re-enumeration reports a miss precisely because + /// nothing changed, which would make any project enumerating a missing directory — an ordinary + /// consequence of a wildcard over a directory that does not exist — permanently uncacheable. + /// + [TestMethod] + public void MatchesCurrentStateDirectoryEnumerationAbsentAtPopulateAndStillAbsentReturnsTrue() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + using TempDirectory tempRepo = TempDirectory.Create("MSBuildCacheTest-DirEnumStillAbsent"); + var pathNormalizer = new PathNormalizer(tempRepo.Path, @"X:\Nuget"); + + // members: null is the "could not enumerate" state, as opposed to an empty list, which means the + // directory existed and had no members. + PathSet cachedPathSet = new(new List + { + new(@"{RepoRoot}\never-created", ObservationType.DirectoryEnumeration, + enumerationPattern: null, + members: null, + writtenMembers: null), + }); + + Assert.IsTrue(CreateFactory(hasher, pathNormalizer: pathNormalizer).MatchesCurrentState(cachedPathSet)); + } + + /// + /// The other direction: a directory that was absent when the entry was produced but exists now has + /// changed, so the entry must not re-validate. + /// + [TestMethod] + public void MatchesCurrentStateDirectoryEnumerationAbsentAtPopulateNowExistsReturnsFalse() + { + using IContentHasher hasher = HashInfoLookup.Find(HashType.Murmur).CreateContentHasher(); + using TempDirectory tempRepo = TempDirectory.Create("MSBuildCacheTest-DirEnumNowExists"); + var pathNormalizer = new PathNormalizer(tempRepo.Path, @"X:\Nuget"); + + PathSet cachedPathSet = new(new List + { + new(@"{RepoRoot}\appears-later", ObservationType.DirectoryEnumeration, + enumerationPattern: null, + members: null, + writtenMembers: null), + }); + + Directory.CreateDirectory(Path.Combine(tempRepo.Path, "appears-later")); + + Assert.IsFalse(CreateFactory(hasher, pathNormalizer: pathNormalizer).MatchesCurrentState(cachedPathSet)); + } + + /// + /// Patterns reported by the sandbox may contain the native DOS wildcard tokens. Re-enumeration must + /// pass them through to the native filesystem layer rather than rejecting them as invalid managed + /// glob syntax. + /// + [TestMethod] + [DataRow("<", DisplayName = "DOS_STAR")] + [DataRow(">", DisplayName = "DOS_QM")] + [DataRow("\"", DisplayName = "DOS_DOT")] + [DataRow("*.\"", DisplayName = "extension-less files")] + [DataRow("{", DisplayName = "open brace")] + [DataRow("]", DisplayName = "close bracket")] + public void EnumerateAndSubtractAcceptsNativePattern(string pattern) + { + using TempDirectory tempDir = TempDirectory.Create("MSBuildCacheTest-BadPattern"); + WriteTextSync(Path.Combine(tempDir.Path, "a.cs"), ""); + WriteTextSync(Path.Combine(tempDir.Path, "b.txt"), ""); + + IReadOnlyList? result = FingerprintFactory.EnumerateAndSubtract(tempDir.Path, pattern, writtenMembersToSubtract: null); + + Assert.IsNotNull(result, "A pattern accepted and reported by the sandbox must remain enumerable at lookup."); + } + + [TestMethod] + public void EnumerateAndSubtractUsesWin32StarDotStarSemantics() + { + using TempDirectory tempDir = TempDirectory.Create("MSBuildCacheTest-StarDotStar"); + WriteTextSync(Path.Combine(tempDir.Path, "README"), ""); + WriteTextSync(Path.Combine(tempDir.Path, "a.cs"), ""); + + IReadOnlyList? result = + FingerprintFactory.EnumerateAndSubtract(tempDir.Path, "*.*", writtenMembersToSubtract: null); + + Assert.IsNotNull(result); + CollectionAssert.AreEquivalent(new[] { "README", "a.cs" }, result.ToArray()); + } + + [TestMethod] + public void EnumerateAndSubtractExcludesIgnoredMembers() + { + using TempDirectory tempDir = TempDirectory.Create("MSBuildCacheTest-IgnoredEnumerationMember"); + string ignoredPath = Path.Combine(tempDir.Path, "noisy.marker"); + WriteTextSync(ignoredPath, ""); + WriteTextSync(Path.Combine(tempDir.Path, "stable.input"), ""); + + IReadOnlyList? result = FingerprintFactory.EnumerateAndSubtract( + tempDir.Path, + enumerationPattern: null, + writtenMembersToSubtract: null, + ignoredInputPatterns: new[] { Glob.Parse(ignoredPath) }); + + Assert.IsNotNull(result); + CollectionAssert.AreEqual(new[] { "stable.input" }, result.ToArray()); + } + + /// + /// must return null (not an empty list) + /// on IO failure, so the caller can distinguish "couldn't observe" from "observed an empty directory". + /// Passing a file path (rather than a directory) reliably triggers . + /// + [TestMethod] + public void EnumerateAndSubtractReturnsNullOnIOFailure() + { + using TempDirectory tempDir = TempDirectory.Create("MSBuildCacheTest-EnumerateAndSubtractIOFailure"); + string filePath = Path.Combine(tempDir.Path, "this-is-a-file-not-a-directory.txt"); + File.WriteAllText(filePath, string.Empty); + + IReadOnlyList? result = FingerprintFactory.EnumerateAndSubtract(filePath, enumerationPattern: null, writtenMembersToSubtract: null); + + Assert.IsNull(result, "EnumerateAndSubtract must return null (not an empty list) when the underlying FS call throws IOException."); + } + + /// + /// Windows filesystem matching is case-insensitive (NtQueryDirectoryFile / FindFirstFileEx), so + /// pattern matching in must be too. A + /// pattern of *.cs recorded at populate time must match a file named Foo.CS on + /// disk at lookup time — otherwise re-enumeration silently filters away files that would have + /// matched the original syscall, producing false cache misses. + /// + [TestMethod] + public void EnumerateAndSubtractIsCaseInsensitive() + { + using TempDirectory tempDir = TempDirectory.Create("MSBuildCacheTest-EnumerateAndSubtractCase"); + File.WriteAllText(Path.Combine(tempDir.Path, "Foo.CS"), string.Empty); + File.WriteAllText(Path.Combine(tempDir.Path, "bar.cs"), string.Empty); + File.WriteAllText(Path.Combine(tempDir.Path, "skip.txt"), string.Empty); + + IReadOnlyList? result = FingerprintFactory.EnumerateAndSubtract(tempDir.Path, enumerationPattern: "*.cs", writtenMembersToSubtract: null); + + Assert.IsNotNull(result); + CollectionAssert.AreEquivalent(new[] { "Foo.CS", "bar.cs" }, result.ToArray()); + } + // ========================================================================================= // Same-path same-type pattern-fold (multi-pattern handling). // diff --git a/src/Common/Caching/CacheClient.cs b/src/Common/Caching/CacheClient.cs index 62a906b..11407e0 100644 --- a/src/Common/Caching/CacheClient.cs +++ b/src/Common/Caching/CacheClient.cs @@ -575,7 +575,16 @@ async Task PlaceFilesAsync(CancellationToken ct) continue; } - // Create a strong fingerprint from the PathSet and see if it matches the selector's strong fingerprint. + // Required for correctness, not just speed: probe and enumeration entries hash their recorded + // state into the strong fingerprint rather than anything read from disk, so the comparison below + // cannot detect that they no longer hold. This check is what enforces them. The fingerprint + // comparison still covers file-content changes. + if (!_fingerprintFactory.MatchesCurrentState(pathSet)) + { + Tracer.Debug(context, $"Skipping selector with PathSet hash {pathSetHash}. Probes/enumerations no longer match current filesystem state."); + continue; + } + Fingerprint? possibleStrongFingerprint = await _fingerprintFactory.GetStrongFingerprintAsync(pathSet); if (possibleStrongFingerprint != null && ByteArrayComparer.ArraysEqual(possibleStrongFingerprint.Hash, selectorStrongFingerprint)) { diff --git a/src/Common/Fingerprinting/FingerprintFactory.cs b/src/Common/Fingerprinting/FingerprintFactory.cs index 884f6c0..e6574da 100644 --- a/src/Common/Fingerprinting/FingerprintFactory.cs +++ b/src/Common/Fingerprinting/FingerprintFactory.cs @@ -383,6 +383,180 @@ internal static List FoldPathSetEntries( return CreateFingerprint(entries); }); + /// + /// Returns true if every non-FCR observation in still matches the + /// current filesystem state. Probes verify presence/absence; directory enumerations verify the effective + /// member list (after subtracting WrittenMembers) still matches Members. FileContentRead + /// entries are not checked here — their content is validated implicitly by + /// , which hashes the current file contents. + /// + /// + /// + /// For FileContentRead entries this is an optimization: their content is hashed by + /// , so a change is caught by the fingerprint comparison whether + /// or not this method runs. + /// + /// + /// For probe and directory-enumeration entries it is not an optimization — it is the only thing + /// enforcing them. Those entries contribute their recorded state to the strong fingerprint rather than + /// anything read from disk, so the fingerprint recomputed at lookup always equals the one stored in the + /// selector no matter what the filesystem now looks like. Removing or short-circuiting this check would + /// therefore not cost cache hits; it would silently start producing incorrect ones. + /// + /// + public bool MatchesCurrentState(PathSet? cachedPathSet) + { + if (cachedPathSet?.Entries == null) + { + return true; + } + + foreach (ObservedPathEntry cached in cachedPathSet.Entries) + { + if (cached.Type == ObservationType.FileContentRead) + { + continue; + } + + string absolutePath = _pathNormalizer.Unnormalize(cached.Path); + bool fileExists = File.Exists(absolutePath); + bool dirExists = !fileExists && Directory.Exists(absolutePath); + + switch (cached.Type) + { + case ObservationType.ExistingProbe: + if (!fileExists && !dirExists) + { + return false; + } + break; + + case ObservationType.AbsentPathProbe: + if (fileExists || dirExists) + { + return false; + } + break; + + case ObservationType.DirectoryEnumeration: + // A null member list is the populate-time "directory was not enumerable" state, which + // ComputeDirectoryMemberHash deliberately encodes as distinct from an empty one. It has + // to be validated as absence rather than falling through to re-enumeration, otherwise a + // directory that was missing when the entry was produced could never re-validate: the + // check below would report a miss precisely because nothing had changed. + if (cached.Members is null) + { + if (dirExists) + { + return false; + } + + break; + } + + if (!dirExists) + { + return false; + } + + // Re-enumerate, subtract cached.WrittenMembers, and compare against cached.Members. + // Subtracting self-outputs makes the comparison robust to whether the previous build's + // outputs are still on disk — the load-bearing trick for hitting on incremental rebuilds. + IReadOnlyList? effective = EnumerateAndSubtract( + absolutePath, + cached.EnumerationPattern, + cached.WrittenMembers, + _pluginSettings.IgnoredInputPatterns); + if (effective is null) + { + // IO failure mid-enumeration — couldn't observe; force MISS to avoid a false hit + // against a populate-time empty observation. + return false; + } + + if (!SequenceEqualsOIC(effective, cached.Members)) + { + return false; + } + break; + + default: + // A newer client may add observation types that this version cannot validate. + // Treat them as mismatches so forward-incompatible cache data degrades to a miss. + return false; + } + } + + return true; + } + + /// + /// Enumerates the directory, applies the optional filter, subtracts + /// (case-insensitive leaf names), and returns the result + /// sorted OrdinalIgnoreCase. Returns null when the directory cannot be enumerated, + /// distinct from an empty list, which means the directory was observed and found empty. The caller + /// uses null to force a cache MISS. + /// + internal static IReadOnlyList? EnumerateAndSubtract( + string absoluteDirectoryPath, + string? enumerationPattern, + IReadOnlyList? writtenMembersToSubtract, + IReadOnlyCollection? ignoredInputPatterns = null) + { + HashSet subtract = writtenMembersToSubtract is null + ? new HashSet(StringComparer.OrdinalIgnoreCase) + : new HashSet(writtenMembersToSubtract, StringComparer.OrdinalIgnoreCase); + + IReadOnlyList? enumeratedMembers = + DirectoryEnumerationReader.EnumerateLeafNames( + absoluteDirectoryPath, + enumerationPattern, + ignoredInputPatterns); + if (enumeratedMembers is null) + { + return null; + } + + List effective = new(); + foreach (string leaf in enumeratedMembers) + { + if (subtract.Contains(leaf)) + { + continue; + } + + effective.Add(leaf); + } + + effective.Sort(StringComparer.OrdinalIgnoreCase); + return effective; + } + + private static bool SequenceEqualsOIC(IReadOnlyList? a, IReadOnlyList? b) + { + if (ReferenceEquals(a, b)) + { + return true; + } + if (a is null || b is null) + { + // Treat null and empty as equal for re-observation purposes — both mean "no external members". + return (a is null ? 0 : a.Count) == (b is null ? 0 : b.Count); + } + if (a.Count != b.Count) + { + return false; + } + for (int i = 0; i < a.Count; i++) + { + if (!string.Equals(a[i], b[i], StringComparison.OrdinalIgnoreCase)) + { + return false; + } + } + return true; + } + private async Task SortAndAddPathSetEntriesAsync(List entries, IReadOnlyList pathSetEntries, bool pathsAreNormalized) { // PathSet.Entries are sorted by (Path OrdinalIgnoreCase, Type ascending, Pattern Ordinal) per the diff --git a/src/Common/Fingerprinting/IFingerprintFactory.cs b/src/Common/Fingerprinting/IFingerprintFactory.cs index a8dbd69..bc46bcb 100644 --- a/src/Common/Fingerprinting/IFingerprintFactory.cs +++ b/src/Common/Fingerprinting/IFingerprintFactory.cs @@ -14,4 +14,6 @@ public interface IFingerprintFactory PathSet? GetPathSet(NodeContext nodeContext, IReadOnlyCollection observations); Task GetStrongFingerprintAsync(PathSet? pathSet); + + bool MatchesCurrentState(PathSet? cachedPathSet); } From 4e45e51b1cac06b0deafa6f062f3e4c8652d1dbe Mon Sep 17 00:00:00 2001 From: David Federman Date: Fri, 31 Jul 2026 17:22:56 -0700 Subject: [PATCH 07/13] Enable probe and enumeration fingerprinting where MSBuild supports it Everything the feature needs is in place, so the setting defaults on and the documented limitation goes away: MSBuildCache no longer assumes the repo is clean before building. Entries produced before this sit under a different weak fingerprint and age out through normal eviction, so nothing needs migrating. Turning it on unconditionally would be wrong on a host that does not report enumeration patterns. There the capture side records every filtered enumeration as if it were unfiltered, which costs hits broadly rather than producing incorrect ones -- but the weak fingerprint would still claim the feature was on. The setting is therefore forced off on such a host, and forced rather than merely ignored, so the property stays the single source of truth for behavior, for logging and for the weak fingerprint instead of reporting a value that disagrees with what the build did. An explicit opt-out on a capable host is still honored. The clamp names the running MSBuild version when it logs, since being told a feature is off is not actionable without knowing what you are running or that upgrading would fix it. The new MSBuild property is added to GlobalPropertiesToIgnore so toggling it with -p: does not also perturb the global-property hash. --- CONTRIBUTING.md | 2 - README.md | 4 +- .../PluginSettingsExtensibilityTests.cs | 30 ++++- src/Common.Tests/PluginSettingsTests.cs | 124 +++++++++++++++++- src/Common/MSBuildCachePluginBase.cs | 6 +- src/Common/PluginSettings.cs | 33 ++++- .../Microsoft.MSBuildCache.Common.targets | 2 + 7 files changed, 180 insertions(+), 21 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bdf4582..e840515 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -56,8 +56,6 @@ Finally, add a `PackageReference` to MSBuildCache to your test repo with version **NOTE!** Because you're using a locally built package, you may need to clear it from your package cache after each iteration via a command like `rmdir /S /Q %NUGET_PACKAGES%\Microsoft.MSBuildCache` (if you set `%NUGET_PACKAGES%`) or `rmdir /S /Q %USERPROFILE%\.nuget\packages\Microsoft.MSBuildCache` if you're using the dfault package cache location. Additionally, to ensure you're not using the head version of the package, you may need to create a branch and dummy commit locally to ensure the version is higher. -**NOTE!** MSBuildCache currently does not handle incremental builds well! The current target scenario is for CI environments, so **it's expected that the repo is always clean before building**. The main reason for this gap is because file probes and directory enumerations are not currently considered. - To enable file reporting via detours in MSBuild, ensure `/graph` and `/reportfileaccesses` are used. Example of a set of commands to test MSBuildCache e2e in some repo: diff --git a/README.md b/README.md index 538f4d9..1875b49 100644 --- a/README.md +++ b/README.md @@ -3,9 +3,6 @@ This project provides plugin implementations for the experimental [MSBuild Project Cache](https://github.com/dotnet/msbuild/blob/main/documentation/specs/project-cache.md), which enables project-level caching within MSBuild. -> [!IMPORTANT] -> Currently MSBuildCache assumes that the build is running in a clean repo. Incremental builds, e.g. local developer builds, are not supported. Target scenarios include PR builds and CI builds. - ## Usage This feature requires Visual Studio 17.9 or later. @@ -76,6 +73,7 @@ These settings are common across all plugins, although different implementations | `$(MSBuildCacheSkipUnchangedOutputFiles)` | `bool` | false | Whether to avoid writing output files on cache hit if the file is unchanged, which can improve performance for incremental builds. A file is considered unchanged if it exists, the previously placed file and file to be placed have the same hash, and the the previously placed file and current file on disk have the same timestamp and file size. | | `$(MSBuildCacheTouchOutputFiles)` | `bool` | false | Whether to update the last write time for output files on cache hit. All files for a given cache entry will have the same timestamp. Note that outputs which skip materialization via `MSBuildCacheSkipUnchangedOutputFiles` are still touched. | | `$(MSBuildCacheIgnoreDotNetSdkPatchVersion)` | `bool` | false | Whether to ignore the patch version when doing cache lookups. This trades off some correctness for the sake of getting cache hits when the SDK version isn't exactly the same. The default behavior is to consider the exact SDK version, eg. "8.0.404". With this setting set to true, it will instead use something like "8.0.4XX". Note that the major version, minor version, and feature bands are still considered. | +| `$(MSBuildCacheEnableProbeAndEnumerationFingerprinting)` | `bool` | true | Whether file probes (existence checks) and directory enumerations contribute to the strong fingerprint, enabling correct caching for incremental builds — including cases where MSBuild source globs match different files. Requires an MSBuild that reports directory enumeration patterns; on older versions this is forced to `false` and a message is logged. | When configuring settings which are list types, you should always append to the existing value to avoid overriding the defaults: diff --git a/src/Common.Tests/PluginSettingsExtensibilityTests.cs b/src/Common.Tests/PluginSettingsExtensibilityTests.cs index 5887045..073bf2e 100644 --- a/src/Common.Tests/PluginSettingsExtensibilityTests.cs +++ b/src/Common.Tests/PluginSettingsExtensibilityTests.cs @@ -23,7 +23,10 @@ public void EffectiveSettingsLogging() { Dictionary settings = new(StringComparer.OrdinalIgnoreCase); MockPluginLogger logger = new(); - _ = PluginSettings.Create(settings, logger, RepoRoot); + + // Pin the capability so this stays a test of settings logging; without it the probe-and-enumeration + // clamp would log a second entry whenever the test host's MSBuild predates the required fields. + _ = PluginSettings.Create(settings, logger, RepoRoot, supportsProbeAndEnumerationCapture: true); Assert.HasCount(1, logger.LogEntries); @@ -52,7 +55,11 @@ public void DefaultValue() { Dictionary settings = new(StringComparer.OrdinalIgnoreCase); MockPluginLogger logger = new(); - MockPluginSettings pluginSettings = PluginSettings.Create(settings, logger, RepoRoot); + MockPluginSettings pluginSettings = PluginSettings.Create( + settings, + logger, + RepoRoot, + supportsProbeAndEnumerationCapture: true); Assert.AreEqual(DefaultMockPluginSettings.StringSetting, pluginSettings.StringSetting); @@ -119,7 +126,11 @@ public void InvalidValues() { nameof(DefaultMockPluginSettings.ISetSetting), "InvalidValue" }, }; MockPluginLogger logger = new(); - MockPluginSettings pluginSettings = PluginSettings.Create(settings, logger, RepoRoot); + MockPluginSettings pluginSettings = PluginSettings.Create( + settings, + logger, + RepoRoot, + supportsProbeAndEnumerationCapture: true); AssertInvalidValueHandled(nameof(MockPluginSettings.EnumSetting), pluginSettings => pluginSettings.EnumSetting); @@ -196,7 +207,11 @@ public void ExplicitValues() { nameof(DefaultMockPluginSettings.ISetSetting), "4; 5; 6" }, }; MockPluginLogger logger = new(); - MockPluginSettings pluginSettings = PluginSettings.Create(settings, logger, RepoRoot); + MockPluginSettings pluginSettings = PluginSettings.Create( + settings, + logger, + RepoRoot, + supportsProbeAndEnumerationCapture: true); Assert.AreEqual("B", pluginSettings.StringSetting); @@ -241,7 +256,12 @@ public void UnsupportedTypeSetting() { nameof(MockPluginSettingsWithUnsupportedType.UnsupportedTypeSetting), "Baz" }, }; MockPluginLogger logger = new(); - MockPluginSettingsWithUnsupportedType pluginSettings = PluginSettings.Create(settings, logger, RepoRoot); + MockPluginSettingsWithUnsupportedType pluginSettings = + PluginSettings.Create( + settings, + logger, + RepoRoot, + supportsProbeAndEnumerationCapture: true); AssertNotLogged(logger, PluginLogLevel.Warning, "has invalid value"); AssertLogged(logger, PluginLogLevel.Warning, "has unsupported type"); diff --git a/src/Common.Tests/PluginSettingsTests.cs b/src/Common.Tests/PluginSettingsTests.cs index d7024fa..e50f7ee 100644 --- a/src/Common.Tests/PluginSettingsTests.cs +++ b/src/Common.Tests/PluginSettingsTests.cs @@ -6,6 +6,8 @@ using System.Linq; using System.Reflection; using DotNet.Globbing; +using Microsoft.Build.Evaluation; +using Microsoft.MSBuildCache.FileAccess; using Microsoft.MSBuildCache.Tests.Mocks; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -23,7 +25,10 @@ public void EffectiveSettingsLogging() { Dictionary settings = new(StringComparer.OrdinalIgnoreCase); MockPluginLogger logger = new(); - _ = PluginSettings.Create(settings, logger, RepoRoot); + + // Pin the capability so this stays a test of settings logging; without it the probe-and-enumeration + // clamp would log a second entry whenever the test host's MSBuild predates the required fields. + _ = PluginSettings.Create(settings, logger, RepoRoot, supportsProbeAndEnumerationCapture: true); Assert.HasCount(1, logger.LogEntries); @@ -61,7 +66,11 @@ public void LogDirectorySetting(string? logDirectorySetting, string expectedLogD settings.Add(nameof(PluginSettings.LogDirectory), logDirectorySetting); } - PluginSettings pluginSettings = PluginSettings.Create(settings, NullPluginLogger.Instance, RepoRoot); + PluginSettings pluginSettings = PluginSettings.Create( + settings, + NullPluginLogger.Instance, + RepoRoot, + supportsProbeAndEnumerationCapture: true); Assert.AreEqual(expectedLogDirectory, pluginSettings.LogDirectory); } @@ -94,6 +103,99 @@ public void LocalCacheSizeInMegabytesSetting() pluginSettings => pluginSettings.LocalCacheSizeInMegabytes, new[] { 123u, 456u, 789u }); + // ========================================================================================= + // EnableProbeAndEnumerationFingerprinting capability clamp. + // + // Probe and enumeration observations are only sound when the running MSBuild reports the + // enumeration pattern. On an older MSBuild the setting is forced to false so that the property + // itself is always the single source of truth — including for the weak fingerprint, which is what + // keeps caches from being shared between hosts that do and don't capture these observations. + // ========================================================================================= + + [TestMethod] + [DataRow(true, DisplayName = "explicitly requested")] + [DataRow(false, DisplayName = "left at default")] + public void ProbeAndEnumerationFingerprintingForcedOffWithoutCapability(bool requestExplicitly) + { + Dictionary settings = new(StringComparer.OrdinalIgnoreCase); + if (requestExplicitly) + { + settings[nameof(PluginSettings.EnableProbeAndEnumerationFingerprinting)] = "true"; + } + + MockPluginLogger logger = new(); + PluginSettings pluginSettings = PluginSettings.Create( + settings, + logger, + RepoRoot, + supportsProbeAndEnumerationCapture: false); + + Assert.IsFalse( + pluginSettings.EnableProbeAndEnumerationFingerprinting, + "The setting must be forced off when the host MSBuild cannot report the required file access fields, " + + "even when the user explicitly asked for it."); + + Assert.IsTrue( + logger.LogEntries.Any(entry => entry.Message.Contains( + nameof(PluginSettings.EnableProbeAndEnumerationFingerprinting), StringComparison.Ordinal) + && entry.Message.Contains("forced to false", StringComparison.Ordinal)), + "Forcing the setting off must be logged so the cache-behavior change is diagnosable."); + + // Naming the running version is what makes the message actionable — otherwise a user is told the + // feature is off but not what they are on or that upgrading would fix it. + if (FileAccessDataCapabilities.MSBuildVersion is string msbuildVersion) + { + Assert.IsTrue( + logger.LogEntries.Any(entry => entry.Message.Contains(msbuildVersion, StringComparison.Ordinal)), + $"The message must name the running MSBuild version ('{msbuildVersion}')."); + } + } + + [TestMethod] + public void MSBuildVersionUsesAssemblyInformationalVersion() + { + string? expected = typeof(ProjectCollection).Assembly + .GetCustomAttribute() + ?.InformationalVersion; + + Assert.AreEqual(expected, FileAccessDataCapabilities.MSBuildVersion); + } + + [TestMethod] + public void ProbeAndEnumerationFingerprintingHonoredWithCapability() + { + Dictionary settings = new(StringComparer.OrdinalIgnoreCase); + MockPluginLogger logger = new(); + PluginSettings pluginSettings = PluginSettings.Create( + settings, + logger, + RepoRoot, + supportsProbeAndEnumerationCapture: true); + + Assert.IsTrue(pluginSettings.EnableProbeAndEnumerationFingerprinting); + } + + /// + /// The clamp must not resurrect the feature for a user who explicitly turned it off on a capable host. + /// + [TestMethod] + public void ProbeAndEnumerationFingerprintingRespectsExplicitOptOutWithCapability() + { + Dictionary settings = new(StringComparer.OrdinalIgnoreCase) + { + [nameof(PluginSettings.EnableProbeAndEnumerationFingerprinting)] = "false", + }; + + MockPluginLogger logger = new(); + PluginSettings pluginSettings = PluginSettings.Create( + settings, + logger, + RepoRoot, + supportsProbeAndEnumerationCapture: true); + + Assert.IsFalse(pluginSettings.EnableProbeAndEnumerationFingerprinting); + } + [TestMethod] [DynamicData(nameof(GlobTestCases), DynamicDataDisplayName = nameof(GetTestCaseDisplayName))] public void IgnoredInputPatternsSetting(GlobTestCase testCase) @@ -202,7 +304,11 @@ void TestBasicSettingValue(string? settingValue, T expectedValue) settings.Add(settingName, settingValue); } - PluginSettings pluginSettings = PluginSettings.Create(settings, NullPluginLogger.Instance, RepoRoot); + PluginSettings pluginSettings = PluginSettings.Create( + settings, + NullPluginLogger.Instance, + RepoRoot, + supportsProbeAndEnumerationCapture: true); Assert.AreEqual(expectedValue, valueAccessor(pluginSettings)); } @@ -218,7 +324,11 @@ private static void TestGlobListSetting( { settingName, testCase.Glob }, }; - PluginSettings pluginSettings = PluginSettings.Create(settings, NullPluginLogger.Instance, RepoRoot); + PluginSettings pluginSettings = PluginSettings.Create( + settings, + NullPluginLogger.Instance, + RepoRoot, + supportsProbeAndEnumerationCapture: true); foreach (string path in testCase.ExpectedMatching) { @@ -255,7 +365,11 @@ private static void TestStringListSetting( settings.Add(settingName, testCase.SettingValue); } - PluginSettings pluginSettings = PluginSettings.Create(settings, NullPluginLogger.Instance, RepoRoot); + PluginSettings pluginSettings = PluginSettings.Create( + settings, + NullPluginLogger.Instance, + RepoRoot, + supportsProbeAndEnumerationCapture: true); CollectionAssert.AreEqual(testCase.ExpectedValues.ToList(), valueAccessor(pluginSettings).ToList()); } diff --git a/src/Common/MSBuildCachePluginBase.cs b/src/Common/MSBuildCachePluginBase.cs index cc62712..a84ce13 100644 --- a/src/Common/MSBuildCachePluginBase.cs +++ b/src/Common/MSBuildCachePluginBase.cs @@ -223,7 +223,11 @@ private async Task BeginBuildInnerAsync(CacheContext context, PluginLoggerBase l _buildId = GetBuildId(); - Settings = PluginSettings.Create(context.PluginSettings, logger, _repoRoot); + Settings = PluginSettings.Create( + context.PluginSettings, + logger, + _repoRoot, + FileAccessDataCapabilities.IsSupported); // The local cache does not allow multiple processes to access it at the same time and will block indefinitely while waiting for a lock on the directory. // In certain scenarios where MSBuild is invoked recursively, such as is done for Fakes projects, this can lead to a hang as the child MSBuild waits for the diff --git a/src/Common/PluginSettings.cs b/src/Common/PluginSettings.cs index 4d76621..9a8a051 100644 --- a/src/Common/PluginSettings.cs +++ b/src/Common/PluginSettings.cs @@ -9,6 +9,7 @@ using System.Text; using DotNet.Globbing; using Microsoft.Build.Experimental.ProjectCache; +using Microsoft.MSBuildCache.FileAccess; namespace Microsoft.MSBuildCache; @@ -116,16 +117,20 @@ public string LocalCacheRootPath /// reads contribute to the fingerprint, matching pre-feature behavior. /// /// - /// This contributes to the weak fingerprint, which keeps caches produced with and without the feature - /// from being shared — important because an entry produced without it records no probe or enumeration - /// dependencies at all, and reusing that entry on a host that does track them would be an incorrect hit. + /// Forced to false when the running MSBuild does not report the required file-access fields (see + /// ), so this always reflects what the build actually did. + /// It contributes to the weak fingerprint, which keeps caches produced with and without the feature + /// from being shared — important because an entry produced without it records no probe or + /// enumeration dependencies at all, and reusing that entry on a host that does track them would be + /// an incorrect hit. /// - public bool EnableProbeAndEnumerationFingerprinting { get; init; } + public bool EnableProbeAndEnumerationFingerprinting { get; init; } = true; public static T Create( IReadOnlyDictionary settings, PluginLoggerBase logger, - string repoRoot) + string repoRoot, + bool supportsProbeAndEnumerationCapture) where T : PluginSettings { T? pluginSettings = Activator.CreateInstance(); @@ -189,6 +194,24 @@ public static T Create( settingValue = getMethod.Invoke(pluginSettings, Array.Empty()); } + // Probe and enumeration fingerprinting is unsound on a host that doesn't report enumeration + // patterns, so an explicit `true` is overridden rather than honored. Clamping the setting + // itself keeps it the single source of truth, including for the weak fingerprint, instead of + // leaving a configured value that disagrees with behavior. + if (!supportsProbeAndEnumerationCapture + && settingValue is true + && propertyName.Equals(nameof(EnableProbeAndEnumerationFingerprinting), StringComparison.Ordinal)) + { + settingValue = false; + MethodInfo setMethod = property.GetSetMethod(nonPublic: true)!; + _ = setMethod.Invoke(pluginSettings, new object[] { false }); + + logger.LogMessage( + $"Setting '{propertyName}' was forced to false because the running MSBuild " + + $"({FileAccessDataCapabilities.MSBuildVersion ?? "unknown version"}) does not report directory " + + "enumeration patterns. A newer MSBuild is required to enable it."); + } + logMessage.Append(propertyName); logMessage.Append(": "); AppendFormattedSettingValue(logMessage, settingValue); diff --git a/src/Common/build/Microsoft.MSBuildCache.Common.targets b/src/Common/build/Microsoft.MSBuildCache.Common.targets index 5cbbbf2..485ec72 100644 --- a/src/Common/build/Microsoft.MSBuildCache.Common.targets +++ b/src/Common/build/Microsoft.MSBuildCache.Common.targets @@ -25,6 +25,7 @@ $(MSBuildCacheGlobalPropertiesToIgnore);MSBuildCacheIgnoreDotNetSdkPatchVersion $(MSBuildCacheGlobalPropertiesToIgnore);MSBuildCacheSkipUnchangedOutputFiles $(MSBuildCacheGlobalPropertiesToIgnore);MSBuildCacheTouchOutputFiles + $(MSBuildCacheGlobalPropertiesToIgnore);MSBuildCacheEnableProbeAndEnumerationFingerprinting @@ -49,6 +50,7 @@ $(MSBuildCacheIgnoreDotNetSdkPatchVersion) $(MSBuildCacheSkipUnchangedOutputFiles) $(MSBuildCacheTouchOutputFiles) + $(MSBuildCacheEnableProbeAndEnumerationFingerprinting) From c455c251e9ce907d25d84e752209b90ddc6f0613 Mon Sep 17 00:00:00 2001 From: David Federman Date: Fri, 31 Jul 2026 17:23:59 -0700 Subject: [PATCH 08/13] Cover the feature end to end The unit tests exercise the pieces in isolation against synthetic PathSets. What they cannot show is that a real build, through MSBuild and the sandbox, hits and misses where it should. Each scenario builds a small generated repo two or three times, perturbs exactly one thing between builds, and asserts the hit and miss counts. The set is chosen so a failure localizes: * A file added under a target-time dynamic glob misses, and an unchanged source tree hits -- the enumeration channel in both directions. * A marker file appearing where the build only probed for it misses, and a marker whose state does not change hits -- the probe channel, and the clean-to-dirty case the feature exists for. * The same marker outside the repo and package roots hits, as does one matched by IgnoredInputPatterns, which is what proves the scope and pattern filters are load-bearing rather than incidental. * A project that enumerates a directory it also writes into hits across a clean, incremental and clean cycle. * Editing a source file misses and rebuilding hits, deliberately routed through neither new channel, so a failure there points at cache fundamentals rather than this feature. * Three consecutive no-change builds all hit. The third replays over the second build's replayed outputs, which catches an observation that only re-validates against outputs a real build produced -- something a two-build test passes. The suite skips with a warning when the MSBuild under test does not report enumeration patterns, since the feature disables itself there and the scenarios would assert misses that cannot happen. Failing instead would block every PR until a suitable MSBuild ships. The warning is raised as a pipeline log issue so a skip is visible in the build summary rather than buried in a log. A skip is otherwise indistinguishable from a pass, so the pipeline job that runs against a bootstrap of dotnet/msbuild main -- the only one carrying the field today -- requires the capability and fails without it. If the bootstrap layout or the API changes, that job fails rather than quietly running nothing. The production Visual Studio jobs keep skipping, and will start running the scenarios on their own once the field ships. Builds now tolerate post-completion accesses under the Windows directory: Code Integrity touching a finished worker otherwise throws from a BuildXL IO-completion thread and terminates the build. Those paths are outside the repo and package roots and so are already excluded from observations. test.ps1 becomes smoke.ps1 now that there is more than one suite. --- .azuredevops/pipelines/pr-ci.yml | 3 + .azuredevops/pipelines/templates/e2e-test.yml | 20 +- tests/lib.ps1 | 151 ++++ tests/scenarios.ps1 | 748 ++++++++++++++++++ tests/smoke.ps1 | 138 ++++ tests/test.ps1 | 225 ------ 6 files changed, 1056 insertions(+), 229 deletions(-) create mode 100644 tests/lib.ps1 create mode 100644 tests/scenarios.ps1 create mode 100644 tests/smoke.ps1 delete mode 100644 tests/test.ps1 diff --git a/.azuredevops/pipelines/pr-ci.yml b/.azuredevops/pipelines/pr-ci.yml index d2243db..eec4da5 100644 --- a/.azuredevops/pipelines/pr-ci.yml +++ b/.azuredevops/pipelines/pr-ci.yml @@ -181,6 +181,9 @@ jobs: parameters: RepoRoot: $(Build.SourcesDirectory)/MSBuildCache MSBuildPath: $(MSBuildPath) + # This job builds MSBuild from tip of main, which carries the enumeration pattern, so the + # scenarios must actually run here rather than skip. + RequireEnumerationCapability: true - publish: $(LogDirectory) displayName: Publish Logs diff --git a/.azuredevops/pipelines/templates/e2e-test.yml b/.azuredevops/pipelines/templates/e2e-test.yml index 340d8ab..aa1c78e 100644 --- a/.azuredevops/pipelines/templates/e2e-test.yml +++ b/.azuredevops/pipelines/templates/e2e-test.yml @@ -4,22 +4,34 @@ parameters: - name: MSBuildPath type: string default: "" +# Set by jobs whose MSBuild is expected to report FileAccessData.EnumeratePattern, so that a +# missing capability fails instead of silently skipping the scenarios. +- name: RequireEnumerationCapability + type: boolean + default: false steps: - task: PowerShell@2 - displayName: "E2E Test: Microsoft.MSBuildCache.Local" + displayName: "E2E Smoke: Microsoft.MSBuildCache.Local" inputs: - filePath: ${{ parameters.RepoRoot }}\tests\test.ps1 + filePath: ${{ parameters.RepoRoot }}\tests\smoke.ps1 arguments: -MSBuildPath "${{ parameters.MSBuildPath }}" -Configuration $(BuildConfiguration) -LogDirectory "$(LogDirectory)\Tests\Local" -LocalPackageDir "$(Pipeline.Workspace)\artifacts\$(BuildConfiguration)\packages" -CachePackage Microsoft.MSBuildCache.Local pwsh: true - task: PowerShell@2 - displayName: "E2E Test: Microsoft.MSBuildCache.AzurePipelines" + displayName: "E2E Scenarios: probe and enumeration fingerprinting" + inputs: + filePath: ${{ parameters.RepoRoot }}\tests\scenarios.ps1 + arguments: -MSBuildPath "${{ parameters.MSBuildPath }}" -Configuration $(BuildConfiguration) -LogDirectory "$(LogDirectory)\Scenarios" -LocalPackageDir "$(Pipeline.Workspace)\artifacts\$(BuildConfiguration)\packages" -CachePackage Microsoft.MSBuildCache.Local -RequireEnumerationCapability ${{ parameters.RequireEnumerationCapability }} + pwsh: true + +- task: PowerShell@2 + displayName: "E2E Smoke: Microsoft.MSBuildCache.AzurePipelines" # The access token from forks do not have enough scopes to access the pipeline cache, so skip these tests. # Note to repo maintainers: You can manually run the pipeline against the commit, even if the commit is from a fork, if you wish to test this. condition: ne(variables['System.PullRequest.IsFork'], 'True') inputs: - filePath: ${{ parameters.RepoRoot }}\tests\test.ps1 + filePath: ${{ parameters.RepoRoot }}\tests\smoke.ps1 arguments: -MSBuildPath "${{ parameters.MSBuildPath }}" -Configuration $(BuildConfiguration) -LogDirectory "$(LogDirectory)\Tests\AzurePipelines" -LocalPackageDir "$(Pipeline.Workspace)\artifacts\$(BuildConfiguration)\packages" -CachePackage Microsoft.MSBuildCache.AzurePipelines pwsh: true env: diff --git a/tests/lib.ps1 b/tests/lib.ps1 new file mode 100644 index 0000000..2b2a47f --- /dev/null +++ b/tests/lib.ps1 @@ -0,0 +1,151 @@ +function New-MSBuildCacheTestProject +{ + param( + [Parameter(Mandatory = $true)] + [string] $ProjectDir, + + [Parameter(Mandatory = $false)] + [string] $GitUserName = "msbuildcache-test", + + [Parameter(Mandatory = $false)] + [string] $GitUserEmail = "msbuildcache-test@local" + ) + + Copy-Item -Path (Join-Path $PSScriptRoot "TestProject") -Destination $ProjectDir -Recurse + + Push-Location $ProjectDir + try + { + & git init *> $null + & git config user.email $GitUserEmail + & git config user.name $GitUserName + & git add . *> $null + & git commit -m "init" *> $null + } + finally + { + Pop-Location + } +} + +function Invoke-MSBuildCacheBuild +{ + param( + [Parameter(Mandatory = $true)] + [string] $MSBuildPath, + + [Parameter(Mandatory = $true)] + [string] $ProjectDir, + + [Parameter(Mandatory = $true)] + [string] $LogDirectory, + + [Parameter(Mandatory = $true)] + [string] $CachePackage, + + [Parameter(Mandatory = $true)] + [string] $CacheUniverse, + + [Parameter(Mandatory = $true)] + [string] $CacheRoot, + + [Parameter(Mandatory = $false)] + [hashtable] $ExtraProperties = @{}, + + [Parameter(Mandatory = $false)] + [string] $Context = "MSBuildCache test" + ) + + New-Item -ItemType Directory -Path $LogDirectory -Force > $null + + $arguments = @( + "-graph", + "-reportfileaccesses", + "-p:MSBuildCachePackage=$CachePackage", + "-p:MSBuildCacheCacheUniverse=$CacheUniverse", + "-p:MSBuildCacheLocalCacheRootPath=$CacheRoot", + "-p:MSBuildCacheLogDirectory=$LogDirectory\MSBuildCacheLogs", + "-binaryLogger:$LogDirectory\msbuild.binlog" + ) + + foreach ($key in $ExtraProperties.Keys) + { + $arguments += "-p:$key=$($ExtraProperties[$key])" + } + + $stdout = Join-Path $LogDirectory "stdout.txt" + $stderr = Join-Path $LogDirectory "stderr.txt" + $process = Start-Process -FilePath $MSBuildPath -ArgumentList $arguments ` + -WorkingDirectory $ProjectDir ` + -RedirectStandardOutput $stdout ` + -RedirectStandardError $stderr ` + -PassThru -NoNewWindow + + # Start-Process only populates ExitCode reliably after the handle has been accessed. + $null = $process.Handle + $process.WaitForExit() + if ($process.ExitCode -ne 0) + { + Get-Content $stdout -ErrorAction SilentlyContinue | Select-Object -Last 40 | Write-Host + Get-Content $stderr -ErrorAction SilentlyContinue | Select-Object -Last 40 | Write-Host + throw "[$Context] build failed (exit=$($process.ExitCode); see $stdout)." + } + + $output = Get-Content $stdout -Raw + $hitMatch = [regex]::Match($output, 'Cache Hit Count: (?\d+)') + $missMatch = [regex]::Match($output, 'Cache Miss Count: (?\d+)') + $ratioMatch = [regex]::Match($output, 'Cache Hit Ratio: (?\d+\.\d+%)') + if (-not ($hitMatch.Success -and $missMatch.Success -and $ratioMatch.Success)) + { + throw "[$Context] could not parse cache statistics from $stdout." + } + + return [pscustomobject]@{ + Hits = [int] $hitMatch.Groups['Value'].Value + Misses = [int] $missMatch.Groups['Value'].Value + HitRatio = $ratioMatch.Groups['Value'].Value + LogDir = $LogDirectory + } +} + +function Assert-CacheStats +{ + param( + [Parameter(Mandatory = $true)] + [pscustomobject] $Result, + + [Parameter(Mandatory = $true)] + [int] $ExpectedHits, + + [Parameter(Mandatory = $true)] + [int] $ExpectedMisses, + + [Parameter(Mandatory = $false)] + [string] $Context, + + [Parameter(Mandatory = $false)] + [string] $ScenarioName, + + [Parameter(Mandatory = $false)] + [string] $Step + ) + + if (-not $Context) + { + $Context = "$ScenarioName :: $Step" + } + + $expectedRatio = "{0:P1}" -f ($ExpectedHits / ($ExpectedHits + $ExpectedMisses)) + $matches = $Result.Hits -eq $ExpectedHits ` + -and $Result.Misses -eq $ExpectedMisses ` + -and $Result.HitRatio -eq $expectedRatio + + $marker = if ($matches) { "PASS" } else { "FAIL" } + Write-Host (" [{0,4}] {1} hits={2} misses={3} ratio={4} (expected hits={5} misses={6} ratio={7})" ` + -f $marker, $Context, $Result.Hits, $Result.Misses, $Result.HitRatio, $ExpectedHits, $ExpectedMisses, $expectedRatio) + + if (-not $matches) + { + throw "[$Context] cache stats mismatch." + } +} diff --git a/tests/scenarios.ps1 b/tests/scenarios.ps1 new file mode 100644 index 0000000..f14c87e --- /dev/null +++ b/tests/scenarios.ps1 @@ -0,0 +1,748 @@ +# tests/scenarios.ps1 +# +# End-to-end scenario tests for the probe and enumeration fingerprinting feature. +# Each scenario sets up a fresh TestProject copy, runs MSBuild builds with state mutations +# between them, and asserts the expected cache hit/miss profile. The scenarios collectively +# pin the headline behaviors of the feature so they're regression-protected: +# +# GlobAddCsFile — Adding a new .cs file the SDK glob now matches must miss +# GlobUnchangedHit — Same source between builds must hit (continuity) +# InScopeMarkerProbeMiss — A previously-absent in-scope marker that now exists must miss +# InScopeMarkerStableHit — Same marker state across builds must hit +# OutOfScopeMarkerHit — A marker outside repo + NuGet scope must NOT invalidate +# IgnoredObservationHit — A noisy in-scope path matched by IgnoredInputPatterns +# must NOT invalidate even when its existence flips +# EnumerateOutputDirCycleHit— A project that enumerates a directory it also writes into must +# still hit, across both clean and incremental rebuilds +# SourceFileChangeMiss — Editing a source file must miss, and rebuilding must then hit +# ThirdBuildAllHits — Three builds, no change: builds 2 AND 3 must both be 100% hits +# +# These exercise the scenarios at the build-system level (the unit tests pin the +# strong-FP behavior in isolation; these prove the feature delivers in real MSBuild builds). +# +# NOTE: these require an MSBuild that reports FileAccessData.EnumeratePattern. On an older +# MSBuild the feature self-disables and the suite skips with a warning rather than failing. +# +# To run: +# pwsh -NoProfile -File tests/scenarios.ps1 + +param +( + [Parameter(Mandatory = $false)] + [string] $LogDirectory = $env:LogDirectory, + + [Parameter(Mandatory = $false)] + [string] $LocalPackageDir = $env:LocalPackageDir, + + [Parameter(Mandatory = $false)] + [string] $TestRoot, + + [Parameter(Mandatory = $false)] + [string] $CachePackage = "Microsoft.MSBuildCache.Local", + + [Parameter(Mandatory = $false)] + [string] $MSBuildPath = $null, + + [Parameter(Mandatory = $false)] + [string] $Configuration = "Debug", + + [Parameter(Mandatory = $false)] + [string[]] $Scenarios = @(), + + # Set by callers that supply an MSBuild which is expected to support the feature (the + # tip-of-MSBuild-repo pipeline job). Turns the capability skip into a hard failure so that + # coverage can't silently disappear if the bootstrap layout or the API changes. + [Parameter(Mandatory = $false)] + [bool] $RequireEnumerationCapability = $false +) + +Set-StrictMode -Version latest +$ErrorActionPreference = "Stop" +. (Join-Path $PSScriptRoot "lib.ps1") + +Push-Location (Join-Path $PSScriptRoot "..") +$RepoRoot = "$PWD" +Pop-Location + +if (-not $LocalPackageDir) { + $LocalPackageDir = Join-Path $RepoRoot "artifacts\$Configuration\packages" +} + +if (-not $LogDirectory) { + $LogDirectory = Join-Path $RepoRoot "logs\Scenarios" +} + +if (-not $TestRoot) { + $TestRoot = Join-Path $RepoRoot "TestResult\Scenarios" +} + +if (-not $MSBuildPath) { + $MSBuildPath = (Get-Command "msbuild").Path +} + +$env:LocalPackageDir = $LocalPackageDir + +Write-Host "Log directory: $LogDirectory" +Write-Host "Test root: $TestRoot" +Write-Host "MSBuild path: $MSBuildPath" +Write-Host "Cache package: $CachePackage" +Write-Host "" + +if (Test-Path $LogDirectory) { + Remove-Item -Path $LogDirectory -Recurse -Force +} +New-Item -ItemType Directory -Path $LogDirectory > $null + +# ---------------------------------------------------------------------------------------- +# Helpers +# ---------------------------------------------------------------------------------------- + +function Test-EnumerationCapability +{ + <# + .SYNOPSIS + Whether the MSBuild under test reports FileAccessData.EnumeratePattern. + + .DESCRIPTION + The probe and enumeration fingerprinting feature self-disables when the host MSBuild does + not report enumeration patterns, so on an older MSBuild these scenarios would assert misses + that legitimately cannot happen. Inspecting the assembly mirrors what the plugin itself + does, rather than guessing from a version number. The load happens in a child process so + the probed assembly is not pinned into this session. + #> + param( + [Parameter(Mandatory = $true)] [string] $MSBuildPath + ) + + $dll = Join-Path (Split-Path -Parent $MSBuildPath) "Microsoft.Build.dll" + if (-not (Test-Path $dll)) { + return $false + } + + $probe = @" +try { + `$assembly = [System.Reflection.Assembly]::LoadFrom('$dll') + `$type = `$assembly.GetType('Microsoft.Build.Experimental.FileAccess.FileAccessData') + if (`$type -and `$type.GetProperty('EnumeratePattern')) { 'YES' } else { 'NO' } +} +catch { 'NO' } +"@ + + return ((& pwsh -NoProfile -Command $probe) -eq 'YES') +} + +function New-ScenarioSandbox +{ + param( + [Parameter(Mandatory = $true)] [string] $ScenarioName + ) + + $sandboxRoot = Join-Path $TestRoot $ScenarioName + if (Test-Path $sandboxRoot) { + Remove-Item -Path $sandboxRoot -Recurse -Force + } + New-Item -ItemType Directory -Path $sandboxRoot > $null + + $projectDir = Join-Path $sandboxRoot "src" + New-MSBuildCacheTestProject ` + -ProjectDir $projectDir ` + -GitUserName "scenario" ` + -GitUserEmail "scenario-test@local" + + $env:NUGET_PACKAGES = Join-Path $sandboxRoot ".nuget" + + return [pscustomobject]@{ + Name = $ScenarioName + SandboxRoot = $sandboxRoot + ProjectDir = $projectDir + CacheRoot = Join-Path $sandboxRoot "MSBuildCache" + Universe = (New-Guid).ToString() + LogRoot = Join-Path $LogDirectory $ScenarioName + } +} + +function Invoke-ScenarioBuild +{ + param( + [Parameter(Mandatory = $true)] [pscustomobject] $Sandbox, + [Parameter(Mandatory = $true)] [string] $Step, + [Parameter(Mandatory = $false)] [hashtable] $ExtraProperties = @{} + ) + + $stepLogDir = Join-Path $Sandbox.LogRoot $Step + New-Item -ItemType Directory -Path $stepLogDir -Force > $null + + $properties = @{ + # Skip writing outputs that match the cache — avoids cache-replay overwriting existing + # outputs from a previous build in the same sandbox. + MSBuildCacheSkipUnchangedOutputFiles = "true" + # OS components (Code Integrity, AV) can touch a finished worker process and report an + # access after the project completed, which otherwise throws from a BuildXL IO-completion + # thread and takes down the build. These paths are outside the repo and NuGet roots, so + # they are already excluded from observations; allowing them only avoids the crash. + MSBuildCacheAllowFileAccessAfterProjectFinishFilePatterns = "$env:SystemRoot\**" + } + + foreach ($key in $ExtraProperties.Keys) { + $properties[$key] = $ExtraProperties[$key] + } + + return Invoke-MSBuildCacheBuild ` + -MSBuildPath $MSBuildPath ` + -ProjectDir $Sandbox.ProjectDir ` + -LogDirectory $stepLogDir ` + -CachePackage $CachePackage ` + -CacheUniverse $Sandbox.Universe ` + -CacheRoot $Sandbox.CacheRoot ` + -ExtraProperties $properties ` + -Context "$($Sandbox.Name) :: $Step" +} + +function Reset-OutputDirectories +{ + # Wipes bin\ and obj\ in the project directory so a subsequent build can write fresh + # outputs without colliding with the cold build's read-only cache materializations. + # Optionally preserves a list of paths under those directories (e.g., scenario markers). + param( + [Parameter(Mandatory = $true)] [string] $ProjectDir, + [Parameter(Mandatory = $false)] [string[]] $PreservePaths = @() + ) + + $preserved = @{} + foreach ($relPath in $PreservePaths) { + $full = Join-Path $ProjectDir $relPath + if (Test-Path $full) { + $preserved[$relPath] = Get-Content -Path $full -Raw -ErrorAction SilentlyContinue + } + } + + foreach ($subdir in @('bin', 'obj')) { + $path = Join-Path $ProjectDir $subdir + if (Test-Path $path) { + # Cache materializations are read-only; clear that flag before delete. + Get-ChildItem -Path $path -Recurse -Force -ErrorAction SilentlyContinue | ForEach-Object { + if (-not $_.PSIsContainer) { + try { $_.IsReadOnly = $false } catch { } + } + } + Remove-Item -Path $path -Recurse -Force -ErrorAction SilentlyContinue + } + } + + foreach ($relPath in $preserved.Keys) { + $full = Join-Path $ProjectDir $relPath + $parent = Split-Path -Parent $full + if (-not (Test-Path $parent)) { + New-Item -ItemType Directory -Path $parent -Force > $null + } + Set-Content -Path $full -Value $preserved[$relPath] -NoNewline + } +} + +function Add-MarkerProbeTarget +{ + # Writes a Directory.Build.targets fragment that probes for a marker file. The probe is + # via MSBuild's Exists() condition, which goes through Detours and therefore shows up in + # the sandbox file-access log as an ExistingProbe / AbsentPathProbe. Used by scenarios + # that exercise the probe channel. + param( + [Parameter(Mandatory = $true)] [string] $ProjectDir, + [Parameter(Mandatory = $true)] [string] $MarkerPath + ) + + $targetsContent = @" + + + + <_MarkerProbeFound>true + + + +"@ + + Set-Content -Path (Join-Path $ProjectDir "Directory.Build.targets") -Value $targetsContent -NoNewline +} + +function Add-DynamicGlobTarget +{ + # Writes a Directory.Build.targets fragment that adds items via a target-time + # dynamic glob over a subdir under obj\ . Two reasons to put the source under obj\: + # + # 1) The SDK's DefaultExcludesInProjectFolder excludes obj\ from ``, + # so the SDK's evaluation-time predictor doesn't see these files. That's the canonical + # "MSBuildPrediction didn't predict it" condition we want to exercise. + # 2) Files added at target-time via `` go through MSBuild's glob expander, + # which calls Directory.GetFiles — Detoured by reportfileaccesses — so the dynamic-glob + # directory shows up as a DirectoryEnumeration entry in the PathSet. + # + # The target also explicitly removes any matching items from the SDK glob's evaluation-time + # set as a defense in depth (in case the SDK glob is ever updated to peek into obj\). + param( + [Parameter(Mandatory = $true)] [string] $ProjectDir, + [Parameter(Mandatory = $true)] [string] $DynamicSubdir + ) + + $targetsContent = @" + + + + + + + + + + +"@ + + Set-Content -Path (Join-Path $ProjectDir "Directory.Build.targets") -Value $targetsContent -NoNewline +} + +# ---------------------------------------------------------------------------------------- +# Scenarios +# ---------------------------------------------------------------------------------------- + +$ScenarioFunctions = [ordered]@{} + +# Scenario A — Adding a new .cs file matched by a TARGET-TIME dynamic glob must produce a +# cache miss. This is the headline value of the feature. Pre-Phase-3 the cache would hit +# because: +# - MSBuildPrediction's evaluation-time predictors don't see target-time `` adds, +# so the new file isn't in the predicted-input set (no weak-FP change). +# - Without probe/enumeration fingerprinting, there's nothing in the cached PathSet that +# references the dynamic glob's directory, so re-observation doesn't notice the new file. +# +# With probe/enumeration fingerprinting, the target-time glob's `Directory.GetFiles` call is +# Detoured, captured as a DirectoryEnumeration entry on the dynamic\ subdir; adding a file +# changes the member-hash and produces a correct miss. +$ScenarioFunctions['GlobAddCsFile'] = { + $sandbox = New-ScenarioSandbox -ScenarioName 'GlobAddCsFile' + $dynamicRel = 'obj\dynamic' + $dynamicDir = Join-Path $sandbox.ProjectDir $dynamicRel + New-Item -ItemType Directory -Path $dynamicDir -Force > $null + Set-Content -Path (Join-Path $dynamicDir 'Initial.cs') -NoNewline -Value @" +public static class Initial { public static int Value => 1; } +"@ + + Add-DynamicGlobTarget -ProjectDir $sandbox.ProjectDir -DynamicSubdir $dynamicRel + + Push-Location $sandbox.ProjectDir + & git add . *> $null + & git commit -m "add dynamic glob target + initial source" *> $null + Pop-Location + + # Build 1: cold cache, dynamic\ contains only Initial.cs. + $cold = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'cold' + Assert-CacheStats -Result $cold -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'GlobAddCsFile' -Step 'cold' + + # Mutate: add a new file to the dynamic-glob directory. Reset bin/obj to clear cold's + # read-only cache outputs, but PRESERVE the dynamic source files (which live under obj\). + Reset-OutputDirectories -ProjectDir $sandbox.ProjectDir -PreservePaths @( + "$dynamicRel\Initial.cs" + ) + Set-Content -Path (Join-Path $dynamicDir 'Added.cs') -NoNewline -Value @" +public static class Added { public static int Value => 2; } +"@ + + # Build 2: dynamic glob's directory now has 2 files. The cached DirectoryEnumeration entry + # re-observes against the current FS, finds a different member-hash, produces a different + # strong FP, and we correctly miss. (Without the feature, this silently HITs — the bug.) + $afterAdd = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'after-add' + Assert-CacheStats -Result $afterAdd -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'GlobAddCsFile' -Step 'after-add' +} + +# Scenario B — Same source between builds must produce a cache HIT. Regression check that +# the new fingerprinting doesn't false-miss on stable inputs (i.e., that noise-reduction +# filters are doing their job). +$ScenarioFunctions['GlobUnchangedHit'] = { + $sandbox = New-ScenarioSandbox -ScenarioName 'GlobUnchangedHit' + $dynamicRel = 'obj\dynamic' + $dynamicDir = Join-Path $sandbox.ProjectDir $dynamicRel + New-Item -ItemType Directory -Path $dynamicDir -Force > $null + Set-Content -Path (Join-Path $dynamicDir 'Initial.cs') -NoNewline -Value @" +public static class Initial { public static int Value => 1; } +"@ + + Add-DynamicGlobTarget -ProjectDir $sandbox.ProjectDir -DynamicSubdir $dynamicRel + + Push-Location $sandbox.ProjectDir + & git add . *> $null + & git commit -m "add dynamic glob target" *> $null + Pop-Location + + $cold = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'cold' + Assert-CacheStats -Result $cold -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'GlobUnchangedHit' -Step 'cold' + + # Reset outputs but preserve the dynamic source. + Reset-OutputDirectories -ProjectDir $sandbox.ProjectDir -PreservePaths @( + "$dynamicRel\Initial.cs" + ) + + # No source change; the dynamic-glob directory's member-hash is unchanged; expect HIT. + $warm = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'warm' + Assert-CacheStats -Result $warm -ExpectedHits 1 -ExpectedMisses 0 -ScenarioName 'GlobUnchangedHit' -Step 'warm' +} + +# Scenario C — A previously-absent in-scope marker file that now exists must produce a MISS. +# Pins the probe-channel clean→dirty→miss fix: the AbsentPathProbe in the cached PathSet is +# re-observed at lookup, finds the marker present, and the strong fingerprint differs. +# +# We place the marker under obj\ so the SDK's default item glob doesn't pick it up as +# a predicted input. Otherwise the marker would also change the WEAK fingerprint, and the +# scenario wouldn't isolate that the probe-channel fix is what's firing — the test could +# pass for the wrong reason. +$ScenarioFunctions['InScopeMarkerProbeMiss'] = { + $sandbox = New-ScenarioSandbox -ScenarioName 'InScopeMarkerProbeMiss' + $objDir = Join-Path $sandbox.ProjectDir 'obj' + New-Item -ItemType Directory -Path $objDir -Force > $null + $markerPath = Join-Path $objDir 'feature-flag.marker' + Add-MarkerProbeTarget -ProjectDir $sandbox.ProjectDir -MarkerPath $markerPath + + # Re-init git so the new Directory.Build.targets is part of the source-control state. + Push-Location $sandbox.ProjectDir + & git add . *> $null + & git commit -m "add probe target" *> $null + Pop-Location + + # Build 1: marker absent. PathSet captures AbsentPathProbe of the marker. + $cold = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'cold' + Assert-CacheStats -Result $cold -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'InScopeMarkerProbeMiss' -Step 'cold' + + # Mutate: create the marker (NOT a project output, just an external action). + New-Item -ItemType Directory -Path $objDir -Force > $null + Set-Content -Path $markerPath -NoNewline -Value 'on' + + # Don't `git clean` — that would wipe obj\, including our marker. The cache plugin runs + # before MSBuild's incremental-skip logic, so leftover obj/ contents don't interfere with + # the cache lookup. + + # Build 2: marker now present. Re-observation should promote AbsentPathProbe → ExistingProbe + # for the marker path; strong FP differs; expect MISS. This is the clean→dirty→miss fix + # firing through the probe channel. + $afterCreate = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'after-create-marker' + Assert-CacheStats -Result $afterCreate -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'InScopeMarkerProbeMiss' -Step 'after-create-marker' +} + +# Scenario D — Marker state stable across builds must HIT. Companion to scenario C: confirms +# the probe channel doesn't false-miss when the probed state is unchanged. +$ScenarioFunctions['InScopeMarkerStableHit'] = { + $sandbox = New-ScenarioSandbox -ScenarioName 'InScopeMarkerStableHit' + $objDir = Join-Path $sandbox.ProjectDir 'obj' + New-Item -ItemType Directory -Path $objDir -Force > $null + $markerPath = Join-Path $objDir 'feature-flag.marker' + Add-MarkerProbeTarget -ProjectDir $sandbox.ProjectDir -MarkerPath $markerPath + Set-Content -Path $markerPath -NoNewline -Value 'on' + + Push-Location $sandbox.ProjectDir + & git add . *> $null + & git commit -m "add probe target" *> $null + Pop-Location + + # Build 1: marker present. PathSet captures ExistingProbe of the marker. + $cold = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'cold' + Assert-CacheStats -Result $cold -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'InScopeMarkerStableHit' -Step 'cold' + + # Don't `git clean` — preserves the marker for the second build. + + # Build 2: marker still present. Re-observation finds ExistingProbe still satisfied; strong + # FP matches; expect HIT. + $warm = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'warm' + Assert-CacheStats -Result $warm -ExpectedHits 1 -ExpectedMisses 0 -ScenarioName 'InScopeMarkerStableHit' -Step 'warm' +} + +# Scenario E — A marker outside the repo + NuGet scope must NOT invalidate the cache when its +# existence flips. This pins the scope-based exclusion: system / out-of-scope state changes +# are ignored. Without scope filtering, the warm cache would miss. +$ScenarioFunctions['OutOfScopeMarkerHit'] = { + $sandbox = New-ScenarioSandbox -ScenarioName 'OutOfScopeMarkerHit' + $oosMarker = Join-Path ([System.IO.Path]::GetTempPath()) "msbuildcache-oos-$([guid]::NewGuid().ToString('N')).marker" + Add-MarkerProbeTarget -ProjectDir $sandbox.ProjectDir -MarkerPath $oosMarker + + Push-Location $sandbox.ProjectDir + & git add . *> $null + & git commit -m "add out-of-scope probe target" *> $null + Pop-Location + + # Make sure the marker doesn't exist initially. + if (Test-Path $oosMarker) { Remove-Item -Path $oosMarker -Force } + + try { + # Build 1: marker absent. PathSet does NOT capture this probe (filtered by scope). + $cold = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'cold' + Assert-CacheStats -Result $cold -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'OutOfScopeMarkerHit' -Step 'cold' + + # Mutate: create the marker outside repo + NuGet scope. + Set-Content -Path $oosMarker -NoNewline -Value 'on' + + Push-Location $sandbox.ProjectDir + & git clean -fdx *> $null + Pop-Location + + # Build 2: marker now present, but it's out of scope; cache should still HIT. + $warm = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'after-create-oos-marker' + Assert-CacheStats -Result $warm -ExpectedHits 1 -ExpectedMisses 0 -ScenarioName 'OutOfScopeMarkerHit' -Step 'after-create-oos-marker' + } + finally { + if (Test-Path $oosMarker) { Remove-Item -Path $oosMarker -Force -ErrorAction SilentlyContinue } + } +} + +# Scenario F — An in-scope path matched by IgnoredInputPatterns must NOT invalidate the +# cache when its existence flips. Pins the configurable noise filter; the inverse of scenario C. +# +# We place the marker under obj\ so the SDK's default doesn't pick it +# up as a predicted input (the SDK's DefaultExcludesInProjectFolder excludes obj/ and bin/). +# Otherwise the marker would change the WEAK fingerprint regardless of any observation filter. +$ScenarioFunctions['IgnoredObservationHit'] = { + $sandbox = New-ScenarioSandbox -ScenarioName 'IgnoredObservationHit' + $objDir = Join-Path $sandbox.ProjectDir 'obj' + New-Item -ItemType Directory -Path $objDir -Force > $null + $markerPath = Join-Path $objDir 'noisy.marker' + Add-MarkerProbeTarget -ProjectDir $sandbox.ProjectDir -MarkerPath $markerPath + + Push-Location $sandbox.ProjectDir + & git add . *> $null + & git commit -m "add probe target" *> $null + Pop-Location + + $ignorePatterns = @{ MSBuildCacheIgnoredInputPatterns = '**\noisy.marker' } + + # Build 1: marker absent; pattern excludes the probe from the strong fingerprint. + $cold = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'cold' -ExtraProperties $ignorePatterns + Assert-CacheStats -Result $cold -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'IgnoredObservationHit' -Step 'cold' + + # Mutate: create the marker file under obj\ (which the SDK's default item exclusion list + # ignores, so it doesn't enter the predicted-input set and doesn't change the weak FP). + New-Item -ItemType Directory -Path $objDir -Force > $null + Set-Content -Path $markerPath -NoNewline -Value 'on' + + # Don't `git clean` — that would wipe obj\, including our marker. Just rebuild. + + # Build 2: marker now present, but the probe is filtered by IgnoredInputPatterns; + # strong FP unchanged; cache HIT. + $warm = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'after-create-marker' -ExtraProperties $ignorePatterns + Assert-CacheStats -Result $warm -ExpectedHits 1 -ExpectedMisses 0 -ScenarioName 'IgnoredObservationHit' -Step 'after-create-marker' +} + +function Add-EnumerateOutputDirTarget +{ + # Writes a Directory.Build.targets fragment that: + # 1) Writes a generated file into an existing directory before CoreCompile. + # 2) Causes MSBuild to enumerate that directory via a glob, so the target + # produces a self-output in the enumerated directory. + # + # Together these reproduce the canonical "project enumerates a directory it also writes + # into" pattern (e.g., a CI build that copies its outputs to a staging dir and then + # enumerates the dir to assemble a manifest). Without the schema-driven self-output filter, + # the populate-time member hash includes the generated file, and a clean rebuild's + # re-observation would see an empty directory and produce a false miss. + # + # The scenario keeps a tracked member in obj\generated\ so the directory exists in clean + # state while remaining outside the SDK's default item globs. + param( + [Parameter(Mandatory = $true)] [string] $ProjectDir, + [Parameter(Mandatory = $true)] [string] $EnumeratedSubdir + ) + + $targetsContent = @" + + + + + + <_GeneratedDirContents Include="`$(MSBuildThisFileDirectory)$EnumeratedSubdir\**\*" /> + <_GeneratedDirCount Include="@(_GeneratedDirContents->Count())" /> + + + +"@ + + Set-Content -Path (Join-Path $ProjectDir "Directory.Build.targets") -Value $targetsContent -NoNewline +} + +# Scenario G — The headline self-output enumeration cycle. A project enumerates a directory it +# also writes into. Build clean (cache populates with a partitioned member list); rebuild after +# git clean (only the tracked external member remains); the cache must HIT because the only +# removed member is the project's own output, which the schema cancels out. +# +# Without the schema-driven self-output filter, the populate-time member hash would include +# the generated file, the lookup-time re-observation would see only the tracked member, and +# the cache would falsely miss — even though the project's external view is unchanged. +$ScenarioFunctions['EnumerateOutputDirCycleHit'] = { + $sandbox = New-ScenarioSandbox -ScenarioName 'EnumerateOutputDirCycleHit' + $enumeratedRel = 'obj\generated' + $enumeratedDir = Join-Path $sandbox.ProjectDir $enumeratedRel + $trackedMember = Join-Path $enumeratedDir 'Tracked.input' + + New-Item -ItemType Directory -Path $enumeratedDir -Force > $null + Set-Content -Path $trackedMember -NoNewline -Value 'tracked external member' + + Add-EnumerateOutputDirTarget -ProjectDir $sandbox.ProjectDir -EnumeratedSubdir $enumeratedRel + + Push-Location $sandbox.ProjectDir + & git add . *> $null + & git add -f $trackedMember *> $null + & git commit -m "add enumerate-output-dir target" *> $null + Pop-Location + + # Build 1: cold. Project writes Generated.gen and enumerates the existing directory. + # End-of-build: PathSet captures Members=[Tracked.input] (external) and + # WrittenMembers=[Generated.gen] (the project's self-output). + $cold = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'cold' + Assert-CacheStats -Result $cold -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'EnumerateOutputDirCycleHit' -Step 'cold' + + # Build 2: clean state. git clean removes Generated.gen but preserves the tracked member, + # so the directory remains enumerable: + # - cached.Members = [Tracked.input], cached.WrittenMembers = [Generated.gen] + # - lookup re-enumerates obj\generated\ → currentMembers = [Tracked.input] + # - effective = [Tracked.input] - [Generated.gen] = [Tracked.input] + # - matches cached.Members → HIT ✓ + Push-Location $sandbox.ProjectDir + & git clean -fdx *> $null + Pop-Location + + $cleanRebuild = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'clean-rebuild' + Assert-CacheStats -Result $cleanRebuild -ExpectedHits 1 -ExpectedMisses 0 -ScenarioName 'EnumerateOutputDirCycleHit' -Step 'clean-rebuild' + + # Build 3: incremental rebuild (no clean, leftover obj\ contents from cache replay). + # Re-enumeration finds Tracked.input and Generated.gen (replayed from cache); subtract + # cached.WrittenMembers → [Tracked.input]; + # matches cached.Members → HIT. + $incrementalRebuild = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'incremental-rebuild' + Assert-CacheStats -Result $incrementalRebuild -ExpectedHits 1 -ExpectedMisses 0 -ScenarioName 'EnumerateOutputDirCycleHit' -Step 'incremental-rebuild' +} + +# Scenario H — Editing an existing source file must MISS, and rebuilding with that edit in +# place must then HIT. This is the most basic correctness contract of the cache and is +# deliberately independent of the probe/enumeration channels: the edited file is a predicted +# input, so the WEAK fingerprint changes. It is here as a control — if this ever fails, the +# failure is in the cache fundamentals rather than in this feature, which makes triaging the +# other scenarios much faster. +$ScenarioFunctions['SourceFileChangeMiss'] = { + $sandbox = New-ScenarioSandbox -ScenarioName 'SourceFileChangeMiss' + $programPath = Join-Path $sandbox.ProjectDir 'Program.cs' + + $cold = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'cold' + Assert-CacheStats -Result $cold -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'SourceFileChangeMiss' -Step 'cold' + + # Edit an existing, predicted source file. + Reset-OutputDirectories -ProjectDir $sandbox.ProjectDir + Add-Content -Path $programPath -Value @" + +public static class ScenarioEdit { public static int Value => 42; } +"@ + + $afterEdit = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'after-edit' + Assert-CacheStats -Result $afterEdit -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'SourceFileChangeMiss' -Step 'after-edit' + + # Same content as the previous build — the edit is now the cached state, so expect a HIT. + Reset-OutputDirectories -ProjectDir $sandbox.ProjectDir + $rebuild = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'rebuild-after-edit' + Assert-CacheStats -Result $rebuild -ExpectedHits 1 -ExpectedMisses 0 -ScenarioName 'SourceFileChangeMiss' -Step 'rebuild-after-edit' +} + +# Scenario I — Three consecutive builds with no source change: the first populates, and both +# subsequent builds must be 100% hits. The third build is the one that matters. A feature that +# records an observation whose re-validation depends on state the *replay* itself produces can +# still pass a two-build test — build 2 runs against the populate build's leftovers — and then +# fail on build 3, which runs against cache-replayed outputs instead. This pins that the +# observation set is stable across repeated replay rather than merely correct once. +$ScenarioFunctions['ThirdBuildAllHits'] = { + $sandbox = New-ScenarioSandbox -ScenarioName 'ThirdBuildAllHits' + $dynamicRel = 'obj\dynamic' + $dynamicDir = Join-Path $sandbox.ProjectDir $dynamicRel + New-Item -ItemType Directory -Path $dynamicDir -Force > $null + Set-Content -Path (Join-Path $dynamicDir 'Initial.cs') -NoNewline -Value @" +public static class Initial { public static int Value => 1; } +"@ + + # Include the enumeration and probe channels so all three observation types are exercised + # across the repeated replays, not just file content reads. + Add-DynamicGlobTarget -ProjectDir $sandbox.ProjectDir -DynamicSubdir $dynamicRel + + Push-Location $sandbox.ProjectDir + & git add . *> $null + & git commit -m "add dynamic glob target" *> $null + Pop-Location + + $first = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'first' + Assert-CacheStats -Result $first -ExpectedHits 0 -ExpectedMisses 1 -ScenarioName 'ThirdBuildAllHits' -Step 'first' + + Reset-OutputDirectories -ProjectDir $sandbox.ProjectDir -PreservePaths @("$dynamicRel\Initial.cs") + $second = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'second' + Assert-CacheStats -Result $second -ExpectedHits 1 -ExpectedMisses 0 -ScenarioName 'ThirdBuildAllHits' -Step 'second' + + # Build 3 replays over build 2's replayed outputs. Any observation that is only stable + # against a real build's outputs, rather than a replayed one, fails here. + Reset-OutputDirectories -ProjectDir $sandbox.ProjectDir -PreservePaths @("$dynamicRel\Initial.cs") + $third = Invoke-ScenarioBuild -Sandbox $sandbox -Step 'third' + Assert-CacheStats -Result $third -ExpectedHits 1 -ExpectedMisses 0 -ScenarioName 'ThirdBuildAllHits' -Step 'third' +} + +# ---------------------------------------------------------------------------------------- +# Run scenarios +# ---------------------------------------------------------------------------------------- + +# Wrap in @() so a single -Scenarios value stays an array; PowerShell unrolls a one-element +# array returned from `if` to a scalar, and $toRun.Count would then fail under StrictMode. +$toRun = @(if ($Scenarios -and $Scenarios.Count -gt 0) { $Scenarios } else { $ScenarioFunctions.Keys }) +$failed = @() + +if (-not (Test-EnumerationCapability -MSBuildPath $MSBuildPath)) { + $message = "MSBuild at '$MSBuildPath' does not report FileAccessData.EnumeratePattern, so probe " + + "and enumeration fingerprinting self-disables and these $($toRun.Count) scenarios cannot pass." + + if ($RequireEnumerationCapability) { + # This caller supplies an MSBuild that is supposed to support the feature, so a missing + # capability means the setup is broken rather than the MSBuild being old. Skipping here would + # silently drop all end-to-end coverage. + Write-Host "##vso[task.logissue type=error]$message Expected this MSBuild to support it." + Write-Host "$message Expected this MSBuild to support it." + exit 1 + } + + # Otherwise skip rather than fail: released MSBuild doesn't carry the field yet, and failing would + # block every PR on something no PR can fix. Raised as a pipeline log issue so the skip surfaces in + # the build summary instead of being buried in log output. + $message += " SKIPPING. Re-run against an MSBuild that contains the field to actually exercise them." + Write-Host "##vso[task.logissue type=warning]$message" + Write-Warning $message + exit 0 +} + +foreach ($name in $toRun) { + if (-not $ScenarioFunctions.Contains($name)) { + Write-Host "Unknown scenario: $name. Available: $($ScenarioFunctions.Keys -join ', ')" + $failed += $name + continue + } + + Write-Host "" + Write-Host "================================================================" + Write-Host "Scenario: $name" + Write-Host "================================================================" + + try { + & $ScenarioFunctions[$name] + Write-Host "[ OK ] $name" + } + catch { + Write-Host "[FAIL] $name -- $($_.Exception.Message)" + $failed += $name + } +} + +Write-Host "" +Write-Host "================================================================" +if ($failed.Count -eq 0) { + Write-Host "All $($toRun.Count) scenarios passed." + exit 0 +} +else { + Write-Host "$($failed.Count) of $($toRun.Count) scenarios failed:" + foreach ($name in $failed) { Write-Host " - $name" } + exit 1 +} diff --git a/tests/smoke.ps1 b/tests/smoke.ps1 new file mode 100644 index 0000000..b0ee227 --- /dev/null +++ b/tests/smoke.ps1 @@ -0,0 +1,138 @@ +param +( + [Parameter(Mandatory = $false)] + [string] $LogDirectory = $env:LogDirectory, + + [Parameter(Mandatory = $false)] + [string] $LocalPackageDir = $env:LocalPackageDir, + + [Parameter(Mandatory = $false)] + [string] $TestRoot, + + [Parameter(Mandatory = $false)] + [string] $CachePackage = "Microsoft.MSBuildCache.Local", + + [Parameter(Mandatory = $false)] + [string] $MSBuildPath = $null, + + [Parameter(Mandatory = $false)] + [string] $Configuration = "Debug" +) + +Set-StrictMode -Version latest +$ErrorActionPreference = "Stop" +. (Join-Path $PSScriptRoot "lib.ps1") + +function Run-Test { + param ( + [Parameter(Mandatory = $true)] + [string] $TestName, + + [Parameter(Mandatory = $true)] + [int] $ExpectedCacheHits, + + [Parameter(Mandatory = $true)] + [int] $ExpectedCacheMisses + ) + + Write-Host "[$TestName] Starting test" + + Write-Host "[$TestName] Cleaning" + Push-Location $ProjectDir + & git clean -fdx + Pop-Location + + Write-Host "[$TestName] Building" + $result = Invoke-MSBuildCacheBuild ` + -MSBuildPath $MSBuildPath ` + -ProjectDir $ProjectDir ` + -LogDirectory (Join-Path $LogDirectory $TestName) ` + -CachePackage $CachePackage ` + -CacheUniverse $CacheUniverse ` + -CacheRoot "$TestRoot\MSBuildCache" ` + -Context $TestName + + Assert-CacheStats ` + -Result $result ` + -ExpectedHits $ExpectedCacheHits ` + -ExpectedMisses $ExpectedCacheMisses ` + -Context $TestName + + Write-Host "[$TestName] Test complete" +} + +Push-Location (Join-Path $PSScriptRoot "..") +$RepoRoot = "$PWD" +Pop-Location + +if (-not $LocalPackageDir) +{ + $LocalPackageDir = Join-Path $RepoRoot "artifacts\$Configuration\packages" +} + +if (-not $LogDirectory) +{ + $LogDirectory = Join-Path $RepoRoot "logs\Tests" +} + +if (-not $TestRoot) +{ + $TestRoot = Join-Path $RepoRoot "TestResult\$CachePackage" +} + +if (-not $MSBuildPath) +{ + # Find it on the PATH + $MSBuildPath = (Get-Command "msbuild").Path +} +# Use a unique cache universe for every test run +$CacheUniverse = (New-Guid).ToString() + +$env:LocalPackageDir = $LocalPackageDir + +Write-Host "Log Directory: $LogDirectory" +Remove-Item -Path $LogDirectory -Recurse -Force -ErrorAction SilentlyContinue +New-Item -ItemType Directory -Path $LogDirectory > $null + +# set up original run +Write-Host "Running test in $TestRoot" + +$env:NUGET_PACKAGES="$TestRoot\.nuget" +$ProjectDir = Join-Path $TestRoot "src" + +Remove-Item -Path $TestRoot -Recurse -Force -ErrorAction SilentlyContinue + +Write-Host "Creating Git repo in $ProjectDir" +New-MSBuildCacheTestProject ` + -ProjectDir $ProjectDir ` + -GitUserName $Env:UserName ` + -GitUserEmail "$Env:UserName@microsoft.com" + +Run-Test ` + -TestName "ColdCache" ` + -ExpectedCacheHits 0 ` + -ExpectedCacheMisses 1 + +Run-Test ` + -TestName "WarmCache" ` + -ExpectedCacheHits 1 ` + -ExpectedCacheMisses 0 + +# set up junction run +try { + cmd /c mklink /J "$RepoRoot-OtherPath" "$RepoRoot" + $TestRoot = $TestRoot.Replace($RepoRoot, "$RepoRoot-OtherPath") + Write-Host "Running test in $TestRoot" + + $env:NUGET_PACKAGES="$TestRoot\.nuget" + $ProjectDir = Join-Path $TestRoot "src" + + Run-Test ` + -TestName "WarmCacheOtherRoot" ` + -ExpectedCacheHits 1 ` + -ExpectedCacheMisses 0 +} +finally { + # weird way to delete a junction in PowerShell + (Get-Item "$RepoRoot-OtherPath").Delete() +} diff --git a/tests/test.ps1 b/tests/test.ps1 deleted file mode 100644 index 4307a9f..0000000 --- a/tests/test.ps1 +++ /dev/null @@ -1,225 +0,0 @@ -param -( - [Parameter(Mandatory = $false)] - [string] $LogDirectory = $env:LogDirectory, - - [Parameter(Mandatory = $false)] - [string] $LocalPackageDir = $env:LocalPackageDir, - - [Parameter(Mandatory = $false)] - [string] $TestRoot, - - [Parameter(Mandatory = $false)] - [string] $CachePackage = "Microsoft.MSBuildCache.Local", - - [Parameter(Mandatory = $false)] - [string] $MSBuildPath = $null, - - [Parameter(Mandatory = $false)] - [string] $Configuration = "Debug" -) - -Set-StrictMode -Version latest -$ErrorActionPreference = "Stop" - -function Run-Test { - param ( - [Parameter(Mandatory = $true)] - [string] $TestName, - - [Parameter(Mandatory = $true)] - [int] $ExpectedCacheHits, - - [Parameter(Mandatory = $true)] - [int] $ExpectedCacheMisses - ) - - $LogSubDir = Join-Path $LogDirectory $TestName - New-Item -ItemType Directory -Path $LogSubDir > $null - - Write-Host "[$TestName] Starting test" - - Write-Host "[$TestName] Cleaning" - Push-Location $ProjectDir - & git clean -fdx - Pop-Location - - Write-Host "[$TestName] Building" - $ProcessOptions = @{ - FilePath = $MSBuildPath - ArgumentList = @( - "-graph" - "-reportfileaccesses" - "-p:MSBuildCachePackage=$CachePackage" - "-p:MSBuildCacheCacheUniverse=$CacheUniverse" - "-p:MSBuildCacheLocalCacheRootPath=$TestRoot\MSBuildCache" - "-p:MSBuildCacheLogDirectory=$LogSubDir\MSBuildCacheLogs" - "-binaryLogger:$LogSubDir\msbuild.binlog" - ) - WorkingDirectory = $ProjectDir - RedirectStandardOutput = "$LogSubDir\stdout.txt" - RedirectStandardError = "$LogSubDir\stderr.txt" - PassThru = $true - NoNewWindow = $true - } - Write-Host "[$TestName] Running: ""$($ProcessOptions.FilePath)"" $($ProcessOptions.ArgumentList)" - $Process = Start-Process @ProcessOptions - - # Not using -Wait because that waits for all children to exit too, which may included the compiler server. - # But WaitForExit for some reason makes the exit code not work unless we access the process handle. - # See: https://stackoverflow.com/a/23797762 - $ProcessHandle = $Process.Handle - $Process.WaitForExit() - - if ($Process.ExitCode -ne 0) - { - throw "[$TestName] Build failed with exit code $($Process.ExitCode). See: $LogSubDir\msbuild.binlog" - } - - Write-Host "[$TestName] Built successfully" - - Write-Host "[$TestName] Checking cache statistics" - $BuildOutput = Get-Content -Path $ProcessOptions.RedirectStandardOutput -Raw - if ($BuildOutput -Match "Project cache statistics:") - { - Write-Host "[$TestName] Project cache statistics found" - } - else - { - throw "[$TestName] Could not find cache statistics" - } - - if ($BuildOutput -Match "Cache Hit Count: (?\d+)") - { - if ($Matches.CacheHits -eq $ExpectedCacheHits) - { - Write-Host "[$TestName] Found $($Matches.CacheHits) cache hits" - } - else - { - throw "[$TestName] Unexpected number of cache hits. Expected $ExpectedCacheHits. Actual: $($Matches.CacheHits)" - } - } - else - { - throw "[$TestName] Could not find cache hit count" - } - - if ($BuildOutput -Match "Cache Miss Count: (?\d+)") - { - if ($Matches.CacheMisses -eq $ExpectedCacheMisses) - { - Write-Host "[$TestName] Found $($Matches.CacheMisses) cache misses" - } - else - { - throw "[$TestName] Unexpected number of cache misses. Expected $ExpectedCacheMisses. Actual: $($Matches.CacheMisses)" - } - } - else - { - throw "[$TestName] Could not find cache miss count" - } - - $ExpectedCacheHitRatio = "{0:P1}" -f (($ExpectedCacheHits) / ($ExpectedCacheHits + $ExpectedCacheMisses)) - if ($BuildOutput -Match "Cache Hit Ratio: (?\d+\.\d+%)") - { - if ($Matches.CacheHitRatio -eq $ExpectedCacheHitRatio) - { - Write-Host "[$TestName] Found $($Matches.CacheHitRatio) cache hit ratio" - } - else - { - throw "[$TestName] Unexpected cache hit ratio. Expected $ExpectedCacheHitRatio. Actual: $($Matches.CacheHitRatio)" - } - } - else - { - throw "[$TestName] Could not find cache hit ratio" - } - - Write-Host "[$TestName] Cache statistics validated successfully" - - Write-Host "[$TestName] Test complete" -} - -Push-Location (Join-Path $PSScriptRoot "..") -$RepoRoot = "$PWD" -Pop-Location - -if (-not $LocalPackageDir) -{ - $LocalPackageDir = Join-Path $RepoRoot "artifacts\$Configuration\packages" -} - -if (-not $LogDirectory) -{ - $LogDirectory = Join-Path $RepoRoot "logs\Tests" -} - -if (-not $TestRoot) -{ - $TestRoot = Join-Path $RepoRoot "TestResult\$CachePackage" -} - -if (-not $MSBuildPath) -{ - # Find it on the PATH - $MSBuildPath = (Get-Command "msbuild").Path -} -# Use a unique cache universe for every test run -$CacheUniverse = (New-Guid).ToString() - -$env:LocalPackageDir = $LocalPackageDir - -Write-Host "Log Directory: $LogDirectory" -Remove-Item -Path $LogDirectory -Recurse -Force -ErrorAction SilentlyContinue -New-Item -ItemType Directory -Path $LogDirectory > $null - -# set up original run -Write-Host "Running test in $TestRoot" - -$env:NUGET_PACKAGES="$TestRoot\.nuget" -$ProjectDir = Join-Path $TestRoot "src" - -Remove-Item -Path $TestRoot -Recurse -Force -ErrorAction SilentlyContinue -Copy-Item -Path (Join-Path $PSScriptRoot "TestProject") -Destination $ProjectDir -Recurse - -# Create a new git repo with an initial commit so hashing works properly -Write-Host "Creating Git repo in $ProjectDir" -Push-Location $ProjectDir -& git init -& git config user.email "$Env:UserName@microsoft.com" -& git config user.name "$Env:UserName" -& git add . -& git commit -m "Dummy" -Pop-Location - -Run-Test ` - -TestName "ColdCache" ` - -ExpectedCacheHits 0 ` - -ExpectedCacheMisses 1 - -Run-Test ` - -TestName "WarmCache" ` - -ExpectedCacheHits 1 ` - -ExpectedCacheMisses 0 - -# set up junction run -try { - cmd /c mklink /J "$RepoRoot-OtherPath" "$RepoRoot" - $TestRoot = $TestRoot.Replace($RepoRoot, "$RepoRoot-OtherPath") - Write-Host "Running test in $TestRoot" - - $env:NUGET_PACKAGES="$TestRoot\.nuget" - $ProjectDir = Join-Path $TestRoot "src" - - Run-Test ` - -TestName "WarmCacheOtherRoot" ` - -ExpectedCacheHits 1 ` - -ExpectedCacheMisses 0 -} -finally { - # weird way to delete a junction in PowerShell - (Get-Item "$RepoRoot-OtherPath").Delete() -} From 9d90f8e6414904fe7b2f60dc7705e4214d326bd4 Mon Sep 17 00:00:00 2001 From: David Federman Date: Wed, 5 Aug 2026 20:36:31 -0700 Subject: [PATCH 09/13] Fix boolean pipeline argument binding Render the Azure Pipelines boolean template parameter as a PowerShell boolean literal so scenarios.ps1 can bind it to its typed parameter. Copilot-Session: 50484828-3f84-40b9-9775-c576cc297508 --- .azuredevops/pipelines/templates/e2e-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.azuredevops/pipelines/templates/e2e-test.yml b/.azuredevops/pipelines/templates/e2e-test.yml index aa1c78e..880dd28 100644 --- a/.azuredevops/pipelines/templates/e2e-test.yml +++ b/.azuredevops/pipelines/templates/e2e-test.yml @@ -22,7 +22,7 @@ steps: displayName: "E2E Scenarios: probe and enumeration fingerprinting" inputs: filePath: ${{ parameters.RepoRoot }}\tests\scenarios.ps1 - arguments: -MSBuildPath "${{ parameters.MSBuildPath }}" -Configuration $(BuildConfiguration) -LogDirectory "$(LogDirectory)\Scenarios" -LocalPackageDir "$(Pipeline.Workspace)\artifacts\$(BuildConfiguration)\packages" -CachePackage Microsoft.MSBuildCache.Local -RequireEnumerationCapability ${{ parameters.RequireEnumerationCapability }} + arguments: -MSBuildPath "${{ parameters.MSBuildPath }}" -Configuration $(BuildConfiguration) -LogDirectory "$(LogDirectory)\Scenarios" -LocalPackageDir "$(Pipeline.Workspace)\artifacts\$(BuildConfiguration)\packages" -CachePackage Microsoft.MSBuildCache.Local -RequireEnumerationCapability $${{ parameters.RequireEnumerationCapability }} pwsh: true - task: PowerShell@2 From bf7f0bd62c77632755a6e346a4f2c322d1740071 Mon Sep 17 00:00:00 2001 From: David Federman Date: Thu, 6 Aug 2026 09:03:49 -0700 Subject: [PATCH 10/13] Narrow scenario late-access allowlist Allow only the proven Code Integrity and MSBuild telemetry files to arrive after project completion, keeping the diagnostic while avoiding a callback crash. Copilot-Session: 99f10cf3-31a4-4b84-9afa-bc2a6028bdcf --- tests/scenarios.ps1 | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/scenarios.ps1 b/tests/scenarios.ps1 index f14c87e..21d957f 100644 --- a/tests/scenarios.ps1 +++ b/tests/scenarios.ps1 @@ -176,11 +176,11 @@ function Invoke-ScenarioBuild # Skip writing outputs that match the cache — avoids cache-replay overwriting existing # outputs from a previous build in the same sandbox. MSBuildCacheSkipUnchangedOutputFiles = "true" - # OS components (Code Integrity, AV) can touch a finished worker process and report an - # access after the project completed, which otherwise throws from a BuildXL IO-completion - # thread and takes down the build. These paths are outside the repo and NuGet roots, so - # they are already excluded from observations; allowing them only avoids the crash. - MSBuildCacheAllowFileAccessAfterProjectFinishFilePatterns = "$env:SystemRoot\**" + # Code Integrity and MSBuild telemetry can access machine-local files after the project + # completed. These paths are outside the repo and NuGet roots, so they cannot enter the + # PathSet; allowing only the known files avoids terminating the BuildXL callback while + # retaining the late-access diagnostic. + MSBuildCacheAllowFileAccessAfterProjectFinishFilePatterns = "$env:SystemRoot\System32\ci.dll;$env:LOCALAPPDATA\Microsoft\Windows\INetCache\IE\**\dyntelconfig*.cache" } foreach ($key in $ExtraProperties.Keys) { From 6c6429f71193c439e986cfc3cbc7bfc5abfda515 Mon Sep 17 00:00:00 2001 From: David Federman Date: Thu, 6 Aug 2026 10:06:14 -0700 Subject: [PATCH 11/13] Use stable late-access categories in scenarios Allow Windows and Visual Studio telemetry state by location rather than individual filenames, preserving diagnostics without brittle file-specific exemptions. Copilot-Session: 99f10cf3-31a4-4b84-9afa-bc2a6028bdcf --- tests/scenarios.ps1 | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/scenarios.ps1 b/tests/scenarios.ps1 index 21d957f..5e59cd2 100644 --- a/tests/scenarios.ps1 +++ b/tests/scenarios.ps1 @@ -172,15 +172,21 @@ function Invoke-ScenarioBuild $stepLogDir = Join-Path $Sandbox.LogRoot $Step New-Item -ItemType Directory -Path $stepLogDir -Force > $null + $lateFileAccessPatterns = @( + "**\ApplicationInsights.config" + "$env:LOCALAPPDATA\Microsoft\VSApplicationInsights\**" + "$env:LOCALAPPDATA\Microsoft\Windows\INetCache\**" + "$env:SystemRoot\**" + ) -join ";" + $properties = @{ # Skip writing outputs that match the cache — avoids cache-replay overwriting existing # outputs from a previous build in the same sandbox. MSBuildCacheSkipUnchangedOutputFiles = "true" - # Code Integrity and MSBuild telemetry can access machine-local files after the project - # completed. These paths are outside the repo and NuGet roots, so they cannot enter the - # PathSet; allowing only the known files avoids terminating the BuildXL callback while - # retaining the late-access diagnostic. - MSBuildCacheAllowFileAccessAfterProjectFinishFilePatterns = "$env:SystemRoot\System32\ci.dll;$env:LOCALAPPDATA\Microsoft\Windows\INetCache\IE\**\dyntelconfig*.cache" + # Windows services and Visual Studio telemetry can access machine-local state after the + # project completes. These locations cannot enter the PathSet, and matches remain visible + # through the late-access diagnostic. + MSBuildCacheAllowFileAccessAfterProjectFinishFilePatterns = $lateFileAccessPatterns } foreach ($key in $ExtraProperties.Keys) { From 4bf05b56dc1a92a92533c1cd47d5bf07f9eca41e Mon Sep 17 00:00:00 2001 From: David Federman Date: Thu, 6 Aug 2026 10:31:10 -0700 Subject: [PATCH 12/13] Test external late-access categories Keep Application Insights patterns outside repo rooting and cover the assembled Windows and telemetry allowlist against representative positive and negative paths. Copilot-Session: 99f10cf3-31a4-4b84-9afa-bc2a6028bdcf --- src/Common.Tests/PluginSettingsTests.cs | 26 +++++++++++++++++++++++++ tests/scenarios.ps1 | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/Common.Tests/PluginSettingsTests.cs b/src/Common.Tests/PluginSettingsTests.cs index e50f7ee..27615ad 100644 --- a/src/Common.Tests/PluginSettingsTests.cs +++ b/src/Common.Tests/PluginSettingsTests.cs @@ -248,6 +248,32 @@ public void AllowFileAccessAfterProjectFinishFilePatternsSetting(GlobTestCase te testCase, pluginSettings => pluginSettings.AllowFileAccessAfterProjectFinishFilePatterns); + [TestMethod] + public void AllowFileAccessAfterProjectFinishFilePatternsSupportsMachineLocalCategories() + { + Dictionary settings = new(StringComparer.OrdinalIgnoreCase) + { + [nameof(PluginSettings.AllowFileAccessAfterProjectFinishFilePatterns)] = + @"\**\ApplicationInsights.config;" + + @"C:\Users\Test\AppData\Local\Microsoft\VSApplicationInsights\**;" + + @"C:\Users\Test\AppData\Local\Microsoft\Windows\INetCache\**;" + + @"C:\Windows\**", + }; + + PluginSettings pluginSettings = PluginSettings.Create( + settings, + NullPluginLogger.Instance, + RepoRoot, + supportsProbeAndEnumerationCapture: true); + + IReadOnlyCollection patterns = pluginSettings.AllowFileAccessAfterProjectFinishFilePatterns; + Assert.IsTrue(patterns.Any(pattern => pattern.IsMatch(@"C:\Program Files\Telemetry\ApplicationInsights.config"))); + Assert.IsTrue(patterns.Any(pattern => pattern.IsMatch(@"C:\Users\Test\AppData\Local\Microsoft\VSApplicationInsights\config.json"))); + Assert.IsTrue(patterns.Any(pattern => pattern.IsMatch(@"C:\Users\Test\AppData\Local\Microsoft\Windows\INetCache\IE\ABC\dyntelconfig[2].cache"))); + Assert.IsTrue(patterns.Any(pattern => pattern.IsMatch(@"C:\Windows\System32\ci.dll"))); + Assert.IsFalse(patterns.Any(pattern => pattern.IsMatch(@"X:\Repo\src\Program.cs"))); + } + [TestMethod] [DynamicData(nameof(GlobTestCases), DynamicDataDisplayName = nameof(GetTestCaseDisplayName))] public void AllowProcessCloseAfterProjectFinishProcessPatternsSetting(GlobTestCase testCase) diff --git a/tests/scenarios.ps1 b/tests/scenarios.ps1 index 5e59cd2..7906554 100644 --- a/tests/scenarios.ps1 +++ b/tests/scenarios.ps1 @@ -173,7 +173,7 @@ function Invoke-ScenarioBuild New-Item -ItemType Directory -Path $stepLogDir -Force > $null $lateFileAccessPatterns = @( - "**\ApplicationInsights.config" + "\**\ApplicationInsights.config" "$env:LOCALAPPDATA\Microsoft\VSApplicationInsights\**" "$env:LOCALAPPDATA\Microsoft\Windows\INetCache\**" "$env:SystemRoot\**" From abcc0e69ea6e2945651d9fb5d4a71fc6d62e4473 Mon Sep 17 00:00:00 2001 From: David Federman Date: Thu, 6 Aug 2026 11:16:09 -0700 Subject: [PATCH 13/13] Escape semicolons in scenario properties MSBuild treats semicolons in /p values as assignment separators. Encode property values at the shared scenario invocation boundary so list-valued settings reach the plugin intact. Copilot-Session: 99f10cf3-31a4-4b84-9afa-bc2a6028bdcf --- tests/lib.ps1 | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/lib.ps1 b/tests/lib.ps1 index 2b2a47f..c68ee9f 100644 --- a/tests/lib.ps1 +++ b/tests/lib.ps1 @@ -28,6 +28,19 @@ function New-MSBuildCacheTestProject } } +function ConvertTo-MSBuildCommandLinePropertyValue +{ + param( + [Parameter(Mandatory = $true)] + [AllowEmptyString()] + [string] $Value + ) + + # MSBuild uses semicolons to separate assignments within a /p switch. Escape both the + # separator and the escape marker so the property receives the original value. + return $Value.Replace("%", "%25").Replace(";", "%3B") +} + function Invoke-MSBuildCacheBuild { param( @@ -70,7 +83,8 @@ function Invoke-MSBuildCacheBuild foreach ($key in $ExtraProperties.Keys) { - $arguments += "-p:$key=$($ExtraProperties[$key])" + $value = ConvertTo-MSBuildCommandLinePropertyValue ([string] $ExtraProperties[$key]) + $arguments += "-p:$key=$value" } $stdout = Join-Path $LogDirectory "stdout.txt"