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.

Sign up for GitHub

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

[main] Update dependencies from dotnet/arcade #6711

Merged
merged 10 commits into from
Aug 2, 2021
Merged

[main] Update dependencies from dotnet/arcade #6711

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
27cfef9
Update dependencies from https://github.com/dotnet/arcade build 20210…
dotnet-maestro[bot] Jul 26, 2021
a81f429
Update dependencies from https://github.com/dotnet/arcade build 20210…
dotnet-maestro[bot] Jul 26, 2021
3e806d8
Update dependencies from https://github.com/dotnet/arcade build 20210…
dotnet-maestro[bot] Jul 27, 2021
0dd95e8
Remove SNKP and disable CA1416
Forgind Jul 27, 2021
1ed51c3
Merge https://github.com/dotnet/msbuild into darc-main-d484a0e6-ddaa-…
Forgind Jul 27, 2021
f43788e
Update dependencies from https://github.com/dotnet/arcade build 20210…
dotnet-maestro[bot] Jul 28, 2021
b612daf
Update dependencies from https://github.com/dotnet/arcade build 20210…
dotnet-maestro[bot] Jul 29, 2021
fccce76
Update dependencies from https://github.com/dotnet/arcade build 20210…
dotnet-maestro[bot] Jul 30, 2021
b6e19f1
Add comment
Forgind Jul 30, 2021
fe5c14c
Merge branch 'main' into darc-main-d484a0e6-ddaa-44fe-8415-92626fffb0b0
rokonec Aug 2, 2021
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
12 changes: 6 additions & 6 deletions eng/Version.Details.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
<Dependencies>
<ToolsetDependencies>
<Dependency Name="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="3.3.2">
<Uri>https://github.com/dotnet/roslyn-analyzers</Uri>
<Sha />
<Uri>https://github.com/dotnet/roslyn-analyzers</Uri>
<Sha />
</Dependency>
<Dependency Name="Microsoft.DotNet.Arcade.Sdk" Version="6.0.0-beta.21369.3">
<Dependency Name="Microsoft.DotNet.Arcade.Sdk" Version="6.0.0-beta.21379.2">
<Uri>https://github.com/dotnet/arcade</Uri>
<Sha>e97027cf100d2b532adce387e5cb93a373de93c9</Sha>
<Sha>62a8aafffd4c68ef887680f6837abdff906a662c</Sha>
<SourceBuild RepoName="arcade" ManagedOnly="true" />
</Dependency>
<Dependency Name="NuGet.Build.Tasks" Version="6.0.0-preview.1.107">
Expand All @@ -18,9 +18,9 @@
<Uri>https://github.com/dotnet/roslyn</Uri>
<Sha>ea623578b108856d3416af28af61060ed3d695e8</Sha>
</Dependency>
<Dependency Name="Microsoft.DotNet.XUnitExtensions" Version="6.0.0-beta.21369.3">
<Dependency Name="Microsoft.DotNet.XUnitExtensions" Version="6.0.0-beta.21379.2">
<Uri>https://github.com/dotnet/arcade</Uri>
<Sha>e97027cf100d2b532adce387e5cb93a373de93c9</Sha>
<Sha>62a8aafffd4c68ef887680f6837abdff906a662c</Sha>
</Dependency>
</ToolsetDependencies>
</Dependencies>
2 changes: 1 addition & 1 deletion eng/Versions.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<!-- DotNetCliVersion MUST match the dotnet version in global.json.
Otherwise, this version of dotnet will not be installed and the build will error out. -->
<DotNetCliVersion>$([System.Text.RegularExpressions.Regex]::Match($([System.IO.File]::ReadAllText('$(MSBuildThisFileDirectory)..\global.json')), '"dotnet": "([^"]*)"').Groups.get_Item(1))</DotNetCliVersion>
<MicrosoftDotNetXUnitExtensionsVersion>6.0.0-beta.21369.3</MicrosoftDotNetXUnitExtensionsVersion>
<MicrosoftDotNetXUnitExtensionsVersion>6.0.0-beta.21379.2</MicrosoftDotNetXUnitExtensionsVersion>
<MicrosoftCodeAnalysisBannedApiAnalyzersVersion>3.3.2</MicrosoftCodeAnalysisBannedApiAnalyzersVersion>
<MicrosoftExtensionsDependencyModelVersion>6.0.0-preview.2.21154.6</MicrosoftExtensionsDependencyModelVersion>
<MicrosoftNetCompilersToolsetVersion>4.0.0-3.21369.1</MicrosoftNetCompilersToolsetVersion>
Expand Down
2 changes: 1 addition & 1 deletion eng/common/internal/Tools.csproj
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions eng/common/templates/steps/source-build.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"allowPrerelease": true
},
"tools": {
"dotnet": "6.0.100-preview.4.21255.9",
"dotnet": "6.0.100-preview.6.21355.2",
rainersigwald marked this conversation as resolved.
Show resolved Hide resolved
"runtimes": {
"dotnet/x64": [
"2.1.7"
Expand All @@ -15,6 +15,6 @@
},
"msbuild-sdks": {
"Microsoft.Build.CentralPackageVersions": "2.0.1",
"Microsoft.DotNet.Arcade.Sdk": "6.0.0-beta.21369.3"
"Microsoft.DotNet.Arcade.Sdk": "6.0.0-beta.21379.2"
}
}
1 change: 0 additions & 1 deletion src/Build.UnitTests/BackEnd/BinaryTranslator_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,6 @@ public void AssemblyNameWithAllFields()
HashAlgorithm = System.Configuration.Assemblies.AssemblyHashAlgorithm.SHA256,
VersionCompatibility = AssemblyVersionCompatibility.SameMachine,
CodeBase = "C:\\src",
KeyPair = new StrongNameKeyPair(new byte[] { 4, 3, 2, 1 }),
ContentType = AssemblyContentType.WindowsRuntime,
CultureName = "zh-HK",
};
Expand Down
5 changes: 1 addition & 4 deletions src/Shared/AssemblyNameExtension.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ private AssemblyNameExtension(SerializationInfo info, StreamingContext context)
var hashAlgorithm = (System.Configuration.Assemblies.AssemblyHashAlgorithm) info.GetInt32("hashAlg");
var versionCompatibility = (AssemblyVersionCompatibility) info.GetInt32("verCompat");
var codeBase = info.GetString("codebase");
var keyPair = (StrongNameKeyPair) info.GetValue("keypair", typeof(StrongNameKeyPair));

asAssemblyName = new AssemblyName
{
Expand All @@ -160,7 +159,6 @@ private AssemblyNameExtension(SerializationInfo info, StreamingContext context)
HashAlgorithm = hashAlgorithm,
VersionCompatibility = versionCompatibility,
CodeBase = codeBase,
KeyPair = keyPair
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still unclear on whether it's ok to make this change in the .NET Framework version of MSBuild where this API is available and works. Can you elaborate on your reasoning there?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AssemblyNameExtension is internal, so customers can't use it without making a custom MSBuild. We don't rely on the KeyPair part because otherwise, the .net 6 code wouldn't run. So we don't need KeyPair, and customers can't need KeyPair, so we should be able to remove it and stop serializing/deserializing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't rely on the KeyPair part because otherwise, the .net 6 code wouldn't run. So we don't need KeyPair

This doesn't obviously follow for me. I get that we don't need it on .NET 6+. But it's never used/compared in .NET Framework scenarios?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't say I took a deep look at every use, but I tried to briefly look at all our uses (outside tests) to see if they were #ifdef'd on platform, and the ones that were didn't seem to use StrongNameKeyPair. I was a little thrown by "GetPublicKeyToken" and wondered if that was a use, but it seems to still exist on .net 6 without throwing deprecated warnings, so I think it's something else. If you want me to, I can put the removal under an ifdef'd change wave or, if it makes you really uncomfortable, just ifdef it out, but then we may be serializing and deserializing something on .net framework for no reason.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think this is what I was looking for (from https://docs.microsoft.com/en-us/dotnet/api/system.reflection.assemblyname.keypair?view=net-5.0#remarks):

When the runtime loads an assembly, it does not set the KeyPair property. The getter for the property is only useful if the user set the property before using the AssemblyName object to create a dynamic assembly, and subsequently wants to retrieve the key pair.

Since we use this in RAR, which is not interested in dynamic assemblies, it will always be null, so dropping it is ok.

};

asAssemblyName.SetPublicKey(publicKey);
Expand Down Expand Up @@ -635,7 +633,7 @@ private static int CompareBaseNamesStringWise(string asString1, string asString2
/// </summary>
internal AssemblyNameExtension Clone()
{
AssemblyNameExtension newExtension = new AssemblyNameExtension();
AssemblyNameExtension newExtension = new();

if (asAssemblyName != null)
{
Expand Down Expand Up @@ -1001,7 +999,6 @@ public void GetObjectData(SerializationInfo info, StreamingContext context)
info.AddValue("hashAlg", asAssemblyName.HashAlgorithm);
info.AddValue("verCompat", asAssemblyName.VersionCompatibility);
info.AddValue("codebase", asAssemblyName.CodeBase);
info.AddValue("keypair", asAssemblyName.KeyPair);
}

info.AddValue("asStr", asString);
Expand Down
4 changes: 4 additions & 0 deletions src/Shared/NativeMethodsShared.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,9 @@ internal static bool IsMaxPathLegacyWindows()
}
}

// CA1416 warns about code that can only run on Windows, but we verified we're running on Windows before this.
// This is the most reasonable way to resolve this part because other ways would require ifdef'ing on NET472.
#pragma warning disable CA1416
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing this, can you follow the guidance on this warning? https://docs.microsoft.com/en-us/dotnet/core/compatibility/code-analysis/5.0/ca1416-platform-compatibility-analyzer#recommended-action

Specifically, I'd expect to see this call annotated with [SupportedOSPlatform("windows")], which will require annotating or conditionalizing up the chain.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guidance I followed is right below the guidance you suggested.

Also, [SupportedOSPlatform("windows")] was introduced in .net 6, as was OperatingSystem.IsWindows(), so I would have to ifdef around the attribute or IsWindows check. That's pretty terrible, so I don't think that's a good idea.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the very least this needs a comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add one. It doesn't change anything, but I should correct myself: it was introduced in .net 5, not .net 6.

private static bool IsLongPathsEnabledRegistry()
{
using (RegistryKey fileSystemKey = Registry.LocalMachine.OpenSubKey(WINDOWS_FILE_SYSTEM_REGISTRY_KEY))
Expand All @@ -640,6 +643,7 @@ private static bool IsLongPathsEnabledRegistry()
return fileSystemKey != null && Convert.ToInt32(longPathsEnabledValue) == 1;
}
}
#pragma warning restore CA1416

/// <summary>
/// Cached value for IsUnixLike (this method is called frequently during evaluation).
Expand Down
2 changes: 2 additions & 0 deletions src/Tasks.UnitTests/AssemblyDependency/ResolveAssemblyReferenceTestFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2493,6 +2493,7 @@ internal static AssemblyNameExtension[] GetDependencies(string path)
};
}

#pragma warning disable CA1416
/// <summary>
/// Registry access delegate. Given a hive and a view, return the registry base key.
/// </summary>
Expand Down Expand Up @@ -2901,6 +2902,7 @@ private static string GetRegistrySubKeyDefaultValue(RegistryKey baseKey, string
Assert.True(false, $"New GetRegistrySubKeyDefaultValue parameters encountered, need to add unittesting support for subKey={subKey}");
return null;
}
#pragma warning restore CA1416

/// <summary>
/// Delegate for System.IO.File.GetLastWriteTime
Expand Down