Skip to content

Commit

Permalink
[automated] Merge branch 'vs17.11' => 'main' (#10279)
Browse files Browse the repository at this point in the history
* Final branding for 17.11 (#10270)

* Final branding and public API version update

* Update the regex for initial commit detection

* Disable CustomAnalyzerTest

* Delete CompatibilitySuppressions file

* Add inter-branch merge flow file (#10274)

* Add version to BuildResult 2 (#10288)

Fixes #10208

Context
We are adding a version field to this class to make the ResultsCache backwards compatible with at least 2 previous releases (meaning the newer VS can read a cache created by older VS). The cache is not forwards compatible (older versions of VS cannot read cache created by newer versions). The adding of a version field is done without a breaking change in 3 steps, each separated with at least 1 intermediate release.

Execution plan:

1st step (done): Add a special key to the _savedEnvironmentVariables dictionary during the serialization. A workaround overload of the TranslateDictionary function is created to achieve it. The presence of this key will indicate that the version is serialized next. When serializing, add a key to the dictionary and serialize a version field. Do not actually save the special key to dictionary during the deserialization but read a version as a next field if it presents.

2nd step: Stop serialize a special key with the dictionary _savedEnvironmentVariables using the TranslateDictionary function workaround overload. Always serialize and de-serialize the version field. Continue to deserialize _savedEnvironmentVariables with the TranslateDictionary function workaround overload in order not to deserialize dictionary with the special keys.

3rd step: Stop using the TranslateDictionary function workaround overload during _savedEnvironmentVariables deserialization.

Changes Made
1st step from above description.

Testing
Unit tests, manual tests, experimental insertion

* Add CompatibilitySuppressions.xml

---------

Co-authored-by: AR-May <67507805+AR-May@users.noreply.github.com>
Co-authored-by: Farhad Alizada <104755925+f-alizada@users.noreply.github.com>
Co-authored-by: Jan Krivanek <jankrivanek@microsoft.com>
  • Loading branch information
4 people committed Jul 8, 2024
1 parent df5c50a commit 74c90d6
Show file tree
Hide file tree
Showing 10 changed files with 327 additions and 13 deletions.
15 changes: 15 additions & 0 deletions .github/workflows/inter-branch-merge-flow.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: Inter-branch merge workflow
on:
push:
branches:
- vs1**

permissions:
contents: write
pull-requests: write

jobs:
Merge:
uses: dotnet/arcade/.github/workflows/inter-branch-merge-base.yml@main
with:
configuration_file_path: '.config/git-merge-flow-config.jsonc'
6 changes: 3 additions & 3 deletions .vsts-dotnet-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ jobs:
$isVersionBumped = $false
if ($changedVersionsFile -ne $null) {
$difference = git diff HEAD~1 $versionsFile
$changedContent = $difference -join " "
$changedContent = $difference -join "%"
# 'DotNetFinalVersionKind' is expected to be added only during the initial setup of the release branch
$initialCommitPattern = '-\s*<VersionPrefix>\d+\.\d+\.\d+<\/VersionPrefix> \+\s*<VersionPrefix>\d+\.\d+\.\d+<\/VersionPrefix>.*<DotNetFinalVersionKind>release<\/DotNetFinalVersionKind>'
$initialCommitPattern = '-\s*<VersionPrefix>\d+\.\d+\.\d+<\/VersionPrefix>%.*\+\s*<VersionPrefix>\d+\.\d+\.\d+<\/VersionPrefix><DotNetFinalVersionKind>release<\/DotNetFinalVersionKind>'
$isInitialCommit = $changedContent -match $initialCommitPattern
$pattern = '-\s*<VersionPrefix>\d+\.\d+\.(?<previous>\d+)<\/VersionPrefix>.* \+\s*<VersionPrefix>\d+\.\d+\.(?<current>\d+)<\/VersionPrefix>'
$pattern = '-\s*<VersionPrefix>\d+\.\d+\.(?<previous>\d+)<\/VersionPrefix>.*%\+\s*<VersionPrefix>\d+\.\d+\.(?<current>\d+)<\/VersionPrefix>'
if (!($isInitialCommit) -and ($changedContent -match $pattern)) {
try {
$previousPatch = [Convert]::ToInt32($Matches.previous)
Expand Down
46 changes: 42 additions & 4 deletions src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ public static IEnumerable<object[]> CacheSerializationTestData
}
}

// Serialize latest version and deserialize latest version of the cache
[Theory]
[MemberData(nameof(CacheSerializationTestData))]
public void TestResultsCacheTranslation(object obj)
Expand All @@ -393,12 +394,49 @@ public void TestResultsCacheTranslation(object obj)

var copy = new ResultsCache(TranslationHelpers.GetReadTranslator());

copy.ResultsDictionary.Keys.ToHashSet().SetEquals(resultsCache.ResultsDictionary.Keys.ToHashSet()).ShouldBeTrue();
CompareResultsCache(resultsCache, copy);
}

[Theory]
[InlineData(1, 1)] // Serialize version 0 and deserialize version 0
[InlineData(1, 0)] // Serialize version 0 and deserialize latest version
public void TestResultsCacheTranslationAcrossVersions(int envValue1, int envValue2)
{
using (var env = TestEnvironment.Create())
{
env.SetEnvironmentVariable("MSBUILDDONOTVERSIONBUILDRESULT", $"{envValue1}");

// Create a ResultsCache
var request1 = new BuildRequest(1, 2, 3, new[] { "target1" }, null, BuildEventContext.Invalid, null);
var request2 = new BuildRequest(4, 5, 6, new[] { "target2" }, null, BuildEventContext.Invalid, null);

var br1 = new BuildResult(request1);
var br2 = new BuildResult(request2);
br2.AddResultsForTarget("target2", BuildResultUtilities.GetEmptyFailingTargetResult());

var resultsCache = new ResultsCache();
resultsCache.AddResult(br1.Clone());
resultsCache.AddResult(br2.Clone());

resultsCache.Translate(TranslationHelpers.GetWriteTranslator());

env.SetEnvironmentVariable("MSBUILDDONOTVERSIONBUILDRESULT", $"{envValue2}");
Traits.UpdateFromEnvironment();

var copy = new ResultsCache(TranslationHelpers.GetReadTranslator());

CompareResultsCache(resultsCache, copy);
}
}

private void CompareResultsCache(ResultsCache resultsCache1, ResultsCache resultsCache2)
{
resultsCache2.ResultsDictionary.Keys.ToHashSet().SetEquals(resultsCache1.ResultsDictionary.Keys.ToHashSet()).ShouldBeTrue();

foreach (var configId in copy.ResultsDictionary.Keys)
foreach (var configId in resultsCache2.ResultsDictionary.Keys)
{
var copiedBuildResult = copy.ResultsDictionary[configId];
var initialBuildResult = resultsCache.ResultsDictionary[configId];
var copiedBuildResult = resultsCache2.ResultsDictionary[configId];
var initialBuildResult = resultsCache1.ResultsDictionary[configId];

copiedBuildResult.SubmissionId.ShouldBe(initialBuildResult.SubmissionId);
copiedBuildResult.ConfigurationId.ShouldBe(initialBuildResult.ConfigurationId);
Expand Down
9 changes: 7 additions & 2 deletions src/Build/BackEnd/Components/Caching/ResultsCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,14 @@ private static bool CheckResults(BuildResult result, List<string> targets, HashS
/// <param name="buildResult">The candidate build result.</param>
/// <returns>True if the flags and project state filter of the build request is compatible with the build result.</returns>
private static bool AreBuildResultFlagsCompatible(BuildRequest buildRequest, BuildResult buildResult)
{
{
if (buildResult.BuildRequestDataFlags is null)
{
return true;
}

BuildRequestDataFlags buildRequestDataFlags = buildRequest.BuildRequestDataFlags;
BuildRequestDataFlags buildResultDataFlags = buildResult.BuildRequestDataFlags;
BuildRequestDataFlags buildResultDataFlags = (BuildRequestDataFlags) buildResult.BuildRequestDataFlags;

if ((buildRequestDataFlags & FlagsAffectingBuildResults) != (buildResultDataFlags & FlagsAffectingBuildResults))
{
Expand Down
88 changes: 85 additions & 3 deletions src/Build/BackEnd/Shared/BuildResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.Build.BackEnd;
using Microsoft.Build.Shared;
using Microsoft.Build.Shared.FileSystem;
using Microsoft.Build.Framework;

namespace Microsoft.Build.Execution
{
Expand All @@ -31,6 +32,9 @@ public enum BuildResultCode
/// <summary>
/// Contains the current results for all of the targets which have produced results for a particular configuration.
/// </summary>
/// <remarks>
/// When modifying serialization/deserialization, bump the version and support previous versions in order to keep <see cref="ResultsCache"/> backwards compatible.
/// </remarks>
public class BuildResult : BuildResultBase, INodePacket, IBuildResults
{
/// <summary>
Expand Down Expand Up @@ -75,6 +79,14 @@ public class BuildResult : BuildResultBase, INodePacket, IBuildResults
/// </summary>
private ConcurrentDictionary<string, TargetResult> _resultsByTarget;

/// <summary>
/// Version of the build result.
/// </summary>
/// <remarks>
/// Allows to serialize and deserialize different versions of the build result.
/// </remarks>
private int _version = Traits.Instance.EscapeHatches.DoNotVersionBuildResult ? 0 : 1;

/// <summary>
/// The request caused a circular dependency in scheduling.
/// </summary>
Expand All @@ -98,6 +110,16 @@ public class BuildResult : BuildResultBase, INodePacket, IBuildResults
/// </summary>
private Dictionary<string, string>? _savedEnvironmentVariables;

/// <summary>
/// When this key is in the dictionary <see cref="_savedEnvironmentVariables"/>, serialize the build result version.
/// </summary>
private const string SpecialKeyForVersion = "=MSBUILDFEATUREBUILDRESULTHASVERSION=";

/// <summary>
/// Set of additional keys tat might be added to the dictionary <see cref="_savedEnvironmentVariables"/>.
/// </summary>
private static readonly HashSet<string> s_additionalEntriesKeys = new HashSet<string> { SpecialKeyForVersion };

/// <summary>
/// Snapshot of the current directory from the configuration this result comes from.
/// This should only be populated when the configuration for this result is moved between nodes.
Expand All @@ -117,6 +139,9 @@ public class BuildResult : BuildResultBase, INodePacket, IBuildResults
/// <summary>
/// The flags provide additional control over the build results and may affect the cached value.
/// </summary>
/// <remarks>
/// Is optional, the field is expected to be present starting <see cref="_version"/> 1.
/// </remarks>
private BuildRequestDataFlags _buildRequestDataFlags;

private string? _schedulerInducedError;
Expand Down Expand Up @@ -395,7 +420,10 @@ public ProjectInstance? ProjectStateAfterBuild
/// Gets the flags that were used in the build request to which these results are associated.
/// See <see cref="Execution.BuildRequestDataFlags"/> for examples of the available flags.
/// </summary>
public BuildRequestDataFlags BuildRequestDataFlags => _buildRequestDataFlags;
/// <remarks>
/// Is optional, this property exists starting <see cref="_version"/> 1.
/// </remarks>
public BuildRequestDataFlags? BuildRequestDataFlags => (_version > 0) ? _buildRequestDataFlags : null;

/// <summary>
/// Returns the node packet type.
Expand Down Expand Up @@ -597,8 +625,62 @@ void ITranslatable.Translate(ITranslator translator)
translator.Translate(ref _projectStateAfterBuild, ProjectInstance.FactoryForDeserialization);
translator.Translate(ref _savedCurrentDirectory);
translator.Translate(ref _schedulerInducedError);
translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase);
translator.TranslateEnum(ref _buildRequestDataFlags, (int)_buildRequestDataFlags);

// This is a work-around for the bug https://github.com/dotnet/msbuild/issues/10208
// We are adding a version field to this class to make the ResultsCache backwards compatible with at least 2 previous releases.
// The adding of a version field is done without a breaking change in 3 steps, each separated with at least 1 intermediate release.
//
// 1st step (done): Add a special key to the _savedEnvironmentVariables dictionary during the serialization. A workaround overload of the TranslateDictionary function is created to achieve it.
// The presence of this key will indicate that the version is serialized next.
// When serializing, add a key to the dictionary and serialize a version field.
// Do not actually save the special key to dictionary during the deserialization, but read a version as a next field if it presents.
//
// 2nd step: Stop serialize a special key with the dictionary _savedEnvironmentVariables using the TranslateDictionary function workaround overload. Always serialize and de-serialize the version field.
// Continue to deserialize _savedEnvironmentVariables with the TranslateDictionary function workaround overload in order not to deserialize dictionary with the special keys.
//
// 3rd step: Stop using the TranslateDictionary function workaround overload during _savedEnvironmentVariables deserialization.
if (_version == 0)
{
// Escape hatch: serialize/deserialize without version field.
translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase);
}
else
{
Dictionary<string, string> additionalEntries = new();

if (translator.Mode == TranslationDirection.WriteToStream)
{
// Add the special key SpecialKeyForVersion to additional entries indicating the presence of a version to the _savedEnvironmentVariables dictionary.
additionalEntries.Add(SpecialKeyForVersion, String.Empty);

// Serialize the special key together with _savedEnvironmentVariables dictionary using the workaround overload of TranslateDictionary:
translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase, ref additionalEntries, s_additionalEntriesKeys);

// Serialize version
translator.Translate(ref _version);
}
else if (translator.Mode == TranslationDirection.ReadFromStream)
{
// Read the dictionary using the workaround overload of TranslateDictionary: special keys (additionalEntriesKeys) would be read to additionalEntries instead of the _savedEnvironmentVariables dictionary.
translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase, ref additionalEntries, s_additionalEntriesKeys);

// If the special key SpecialKeyForVersion present in additionalEntries, also read a version, otherwise set it to 0.
if (additionalEntries is not null && additionalEntries.ContainsKey(SpecialKeyForVersion))
{
translator.Translate(ref _version);
}
else
{
_version = 0;
}
}
}

// Starting version 1 this _buildRequestDataFlags field is present.
if (_version > 0)
{
translator.TranslateEnum(ref _buildRequestDataFlags, (int)_buildRequestDataFlags);
}
}

/// <summary>
Expand Down
32 changes: 32 additions & 0 deletions src/Build/CompatibilitySuppressions.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,37 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/en-us/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<!-- BuildResult.get_BuildRequestDataFlags backward compat -->
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Microsoft.Build.Execution.BuildResult.get_BuildRequestDataFlags</Target>
<Left>lib/net472/Microsoft.Build.dll</Left>
<Right>lib/net472/Microsoft.Build.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Microsoft.Build.Execution.BuildResult.get_BuildRequestDataFlags</Target>
<Left>lib/net8.0/Microsoft.Build.dll</Left>
<Right>lib/net8.0/Microsoft.Build.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Microsoft.Build.Execution.BuildResult.get_BuildRequestDataFlags</Target>
<Left>ref/net472/Microsoft.Build.dll</Left>
<Right>ref/net472/Microsoft.Build.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Microsoft.Build.Execution.BuildResult.get_BuildRequestDataFlags</Target>
<Left>ref/net8.0/Microsoft.Build.dll</Left>
<Right>ref/net8.0/Microsoft.Build.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>

<!-- BuildCheck API refactor -->
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>F:Microsoft.Build.Experimental.BuildCheck.BuildAnalyzerResultSeverity.Info</Target>
Expand Down Expand Up @@ -85,4 +116,5 @@
<Right>ref/net8.0/Microsoft.Build.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>

</Suppressions>
2 changes: 1 addition & 1 deletion src/BuildCheck.UnitTests/EndToEndTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ private void PrepareSampleProjectsAndConfig(
_env.SetEnvironmentVariable("MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION", "1");
}

[Theory]
[Theory(Skip = "https://github.com/dotnet/msbuild/issues/10277")]
[InlineData("AnalysisCandidate", new[] { "CustomRule1", "CustomRule2" })]
[InlineData("AnalysisCandidateWithMultipleAnalyzersInjected", new[] { "CustomRule1", "CustomRule2", "CustomRule3" }, true)]
public void CustomAnalyzerTest(string analysisCandidate, string[] expectedRegisteredRules, bool expectedRejectedAnalyzers = false)
Expand Down
Loading

0 comments on commit 74c90d6

Please sign in to comment.