Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

ChangeWave 16.8 Becomes Default Behavior #6634

Merged
merged 3 commits into from
Jul 14, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 0 additions & 70 deletions src/Build.UnitTests/Evaluation/Expander_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1547,76 +1547,6 @@ public void ExpandAllIntoStringTruncated()
Assert.Equal(expected, expander.ExpandIntoStringAndUnescape(xmlattribute.Value, ExpanderOptions.ExpandAll | ExpanderOptions.Truncate, MockElementLocation.Instance));
}

/// <summary>
/// Exercises ExpandIntoStringAndUnescape and ExpanderOptions.Truncate
/// </summary>
[Fact]
public void ExpandAllIntoStringNotTruncated()
{
using (TestEnvironment env = TestEnvironment.Create())
{
ChangeWaves.ResetStateForTests();
env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave16_8.ToString());
BuildEnvironmentHelper.ResetInstance_ForUnitTestsOnly();
ProjectInstance project = ProjectHelpers.CreateEmptyProjectInstance();
var manySpaces = "".PadLeft(2000);
var pg = new PropertyDictionary<ProjectPropertyInstance>();
pg.Set(ProjectPropertyInstance.Create("ManySpacesProperty", manySpaces));
var itemMetadataTable = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
{
{ "ManySpacesMetadata", manySpaces }
};
var itemMetadata = new StringMetadataTable(itemMetadataTable);
var projectItemGroups = new ItemDictionary<ProjectItemInstance>();
var itemGroup = new List<ProjectItemInstance>();
StringBuilder longFileName = new StringBuilder();
StringBuilder longMetadataName = new StringBuilder();
for (int i = 0; i < 50; i++)
{
var item = new ProjectItemInstance(project, "ManyItems", $"ThisIsAFairlyLongFileName_{i}.bmp", project.FullPath);
item.SetMetadata("Foo", $"ThisIsAFairlyLongMetadataValue_{i}");
longFileName.Append($"ThisIsAFairlyLongFileName_{i}.bmp" + (i == 49 ? string.Empty : ";"));
longMetadataName.Append($"ThisIsAFairlyLongMetadataValue_{i}" + (i == 49 ? string.Empty : ";"));
itemGroup.Add(item);
}
var lookup = new Lookup(projectItemGroups, pg);
lookup.EnterScope("x");
lookup.PopulateWithItems("ManySpacesItem", new[]
{
new ProjectItemInstance (project, "ManySpacesItem", "Foo", project.FullPath),
new ProjectItemInstance (project, "ManySpacesItem", manySpaces, project.FullPath),
new ProjectItemInstance (project, "ManySpacesItem", "Bar", project.FullPath),
});
lookup.PopulateWithItems("Exactly1024", new[]
{
new ProjectItemInstance (project, "Exactly1024", "".PadLeft(1024), project.FullPath),
new ProjectItemInstance (project, "Exactly1024", "Foo", project.FullPath),
});
lookup.PopulateWithItems("ManyItems", itemGroup);

Expander<ProjectPropertyInstance, ProjectItemInstance> expander = new Expander<ProjectPropertyInstance, ProjectItemInstance>(lookup, lookup, itemMetadata, FileSystems.Default);

XmlAttribute xmlattribute = (new XmlDocument()).CreateAttribute("dummy");
xmlattribute.Value = "'%(ManySpacesMetadata)' != '' and '$(ManySpacesProperty)' != '' and '@(ManySpacesItem)' != '' and '@(Exactly1024)' != '' and '@(ManyItems)' != '' and '@(ManyItems->'%(Foo)')' != '' and '@(ManyItems->'%(Nonexistent)')' != ''";

var expected =
$"'{"",2000}' != '' and " +
$"'{"",2000}' != '' and " +
$"'Foo;{"",2000};Bar' != '' and " +
$"'{"",1024};Foo' != '' and " +
$"'{longFileName}' != '' and " +
$"'{longMetadataName}' != '' and " +
"';;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;' != ''";
var actual = expander.ExpandIntoStringAndUnescape(xmlattribute.Value, ExpanderOptions.ExpandAll | ExpanderOptions.Truncate, MockElementLocation.Instance);
// NOTE: semicolons in the last part are *weird* because they don't actually mean anything and you get logging like
// Target "Build" skipped, due to false condition; ( '@(I->'%(nonexistent)')' == '' ) was evaluated as ( ';' == '' ).
// but that goes back to MSBuild 4.something so I'm codifying it in this test. If you're here because you cleaned it up
// and want to fix the test my current opinion is that's fine.
actual.ShouldBe(expected);
ChangeWaves.ResetStateForTests();
}
}

/// <summary>
/// Exercises ExpandAllIntoString with a string that does not need expanding.
/// In this case the expanded string should be reference identical to the passed in string.
Expand Down
2 changes: 1 addition & 1 deletion src/Build/Evaluation/Expander.cs
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ private static bool IsValidPropertyName(string propertyName)
/// </summary>
private static bool IsTruncationEnabled(ExpanderOptions options)
{
return (options & ExpanderOptions.Truncate) != 0 && !Traits.Instance.EscapeHatches.DoNotTruncateConditions && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave16_8);
return (options & ExpanderOptions.Truncate) != 0 && !Traits.Instance.EscapeHatches.DoNotTruncateConditions;
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Build/Evaluation/LazyItemEvaluator.IncludeOperation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ protected override ImmutableList<I> SelectItems(ImmutableList<ItemData>.Builder
{
// If this item is behind a false condition and represents a full drive/filesystem scan, expanding it is
// almost certainly undesired. It should be skipped to avoid evaluation taking an excessive amount of time.
bool skipGlob = !_conditionResult && globFragment.IsFullFileSystemScan && !Traits.Instance.EscapeHatches.AlwaysEvaluateDangerousGlobs && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave16_8);
bool skipGlob = !_conditionResult && globFragment.IsFullFileSystemScan && !Traits.Instance.EscapeHatches.AlwaysEvaluateDangerousGlobs;
if (!skipGlob)
{
string glob = globFragment.TextFragment;
Expand Down
3 changes: 1 addition & 2 deletions src/Framework/ChangeWaves.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ internal enum ChangeWaveConversionState
/// For dev docs: https://github.com/dotnet/msbuild/blob/master/documentation/wiki/ChangeWaves-Dev.md
internal class ChangeWaves
{
internal static readonly Version Wave16_8 = new Version(16, 8);
internal static readonly Version Wave16_10 = new Version(16, 10);
internal static readonly Version Wave17_0 = new Version(17, 0);
internal static readonly Version[] AllWaves = { Wave16_8, Wave16_10, Wave17_0 };
internal static readonly Version[] AllWaves = { Wave16_10, Wave17_0 };

/// <summary>
/// Special value indicating that all features behind all Change Waves should be enabled.
Expand Down
3 changes: 0 additions & 3 deletions src/Shared/NativeMethodsShared.cs
Original file line number Diff line number Diff line change
Expand Up @@ -514,9 +514,6 @@ public static int GetLogicalCoreCount()
// https://github.com/dotnet/runtime/issues/29686
// so always double-check it.
if (IsWindows
#if !CLR2COMPATIBILITY && !MICROSOFT_BUILD_ENGINE_OM_UNITTESTS
&& ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave16_8)
#endif
#if NETFRAMEWORK
// .NET Framework calls Windows APIs that have a core count limit (32/64 depending on process bitness).
// So if we get a high core count on full framework, double-check it.
Expand Down
2 changes: 1 addition & 1 deletion src/Tasks/Microsoft.Common.CurrentVersion.targets
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
</PropertyGroup>

<!-- Users familiar with how some other repos work try to use NoWarn with MSBuild in place of MSBuildWarningsAsMessages. -->
<PropertyGroup Condition="$([MSBuild]::AreFeaturesEnabled('16.8'))">
<PropertyGroup>
<MSBuildWarningsAsMessages Condition="'$(MSBuildWarningsAsMessages)'==''">$(NoWarn)</MSBuildWarningsAsMessages>
<MSBuildWarningsAsErrors Condition="'$(MSBuildWarningsAsErrors)'==''">$(WarningsAsErrors)</MSBuildWarningsAsErrors>
</PropertyGroup>
Expand Down