Build one MonoTargetsTasks assembly for Microsoft.NET.Runtime.MonoTargets.Sdk pack by lambdageek · Pull Request #59720 · dotnet/runtime

lambdageek · 2021-09-28T20:48:38Z

Instead of 3 assemblies for some tiny tasks.

Building a single assembly also ensures that everything in the pack depends on and bundles identical versions of dependent assemblies. For example on net472, a single version of System.Text.Json with a single fixed version shared by both the JsonToItemsTaskFactory task factory and the RuntimeConfigParser task.

Related to #59619

Additional fixes:

Update all the READMEs
Build AssemblyStripper.dll that just includes CilStrip. This is primarily to turn off nullability and code style checking just for the old Mono.Cecil assemblies.
Don't include Microsoft.Build.dll with the net472 version of the tasks - there is no need to ship an extra copy of MSBuild.
Update ILStrip to use the same Parallel.ForEach logic as MonoAOTCompiler

ghost · 2021-09-28T20:48:45Z

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

Instead of 3 assemblies for some tiny tasks.

Building a single assembly also ensures that everything in the pack depends on and bundles identical versions of dependent assemblies. For example on net472, a single version of System.Text.Json with a single fixed version shared by both the JsonToItemsTaskFactory task factory and the RuntimeConfigParser task.

Related to #59619

todo:

Fix all the READMEs
Build CilStrip to a separate helper assembly (but keep ILStripTask in MonoTargetsTasks.dll) with nullability disabled, and re-enable nullability for the tasks assembly
Figure out why net472 includes a copy of Microsoft.Build.dll instead of just referencing it.

Author:	lambdageek
Assignees:	-
Labels:	`area-Infrastructure-mono`
Milestone:	-

…Sdk pack Instead of 3 assemblies for some tiny tasks. Building a single assembly also ensures that everything in the pack depends on and bundles identical versions of dependent assemblies. For example on net472, a single version of System.Text.Json with a single fixed version shared by both the JsonToItemsTaskFactory task factory and the RuntimeConfigParser task. Related to dotnet#59619

Build it with nullability checking disabled. Build the main MonoTargetsTasks assembly with nullability enabled

In particular, pick up all the dependent assemblies for the net472 build

lambdageek · 2021-09-29T19:09:30Z

With these changes, this is what the Microsoft.NET.Runtime.MonoTargets.Sdks.nupkg looks like:

Archive: artifacts/packages/Release/Shipping/Microsoft.NET.Runtime.MonoTargets.Sdk.7.0.0-ci.nupkg
Length Date Time Name
--------- ---------- ----- ----
527 09-29-2021 15:07 _rels/.rels
1117 09-29-2021 15:07 Microsoft.NET.Runtime.MonoTargets.Sdk.nuspec
88 07-09-2021 15:00 build/Microsoft.NET.Runtime.MonoTargets.Sdk.props
7006 09-13-2021 17:01 Icon.png
1116 01-21-2020 18:43 LICENSE.TXT
95 09-29-2021 17:42 Sdk/Sdk.props
105 07-09-2021 15:00 Sdk/Sdk.targets
1478 09-29-2021 18:04 Sdk/MonoTargetsTasks.props
10473 07-30-2021 20:59 Sdk/RuntimeComponentManifest.targets
357376 09-29-2021 19:07 tasks/net472/AssemblyStripper.dll
20872 10-19-2020 18:40 tasks/net472/Microsoft.Bcl.AsyncInterfaces.dll
29696 09-29-2021 19:07 tasks/net472/MonoTargetsTasks.dll
20856 02-19-2020 10:05 tasks/net472/System.Buffers.dll
189312 10-19-2020 18:37 tasks/net472/System.Collections.Immutable.dll
141184 02-19-2020 10:05 tasks/net472/System.Memory.dll
115856 05-15-2018 13:29 tasks/net472/System.Numerics.Vectors.dll
462728 10-19-2020 18:45 tasks/net472/System.Reflection.Metadata.dll
16768 10-19-2020 18:46 tasks/net472/System.Runtime.CompilerServices.Unsafe.dll
65928 10-19-2020 18:40 tasks/net472/System.Text.Encodings.Web.dll
355712 10-19-2020 18:42 tasks/net472/System.Text.Json.dll
25984 02-19-2020 10:05 tasks/net472/System.Threading.Tasks.Extensions.dll
25232 05-15-2018 13:29 tasks/net472/System.ValueTuple.dll
357888 09-29-2021 19:07 tasks/net6.0/AssemblyStripper.dll
29184 09-29-2021 19:07 tasks/net6.0/MonoTargetsTasks.dll
54772 09-20-2021 14:01 THIRD-PARTY-NOTICES.TXT
713 09-29-2021 15:07 [Content_Types].xml
677 09-29-2021 15:07 package/services/metadata/core-properties/20a1d10c0d2541d680d1da9fd5359eb4.psmdcp
--------- -------
2292743 27 files

radical

Looks good to me. It would be useful to try this out with VS though, if possible.

Co-authored-by: Ankit Jain <radical@gmail.com>

ViktorHofer · 2021-09-30T08:25:56Z

@@ -0,0 +1,24 @@
+<Project Sdk="Microsoft.NET.Sdk">


Why do we need a separate project for the AssemblyStripper if it's just a library and can't be part of the MonoTargetsTasks.csproj?

@ViktorHofer because it pulls in the CilStripper sources which are not actively maintained, full of warnings and aren't nullability friendly. Putting everything in one assembly means we don't get nice checking for all the other tasks.

OK. AFAIK the old sources will be replaced anyway with a new tooling during NET7.

OK. AFAIK the old sources will be replaced anyway with a new tooling during NET7.
That's my understanding, too. At that point this extra assembly can go away

lambdageek · 2021-09-30T14:03:14Z

Don't merge, for now.

Need to check that xamarin-macios doesn't depend on the ILStrip assembly name.

lambdageek · 2021-09-30T14:44:37Z

XI does depend on the assembly name in dotnet/macios#12563.
So if we move the ILStrip task to another assembly, the XI code will need to change in net7 to use the new name. /cc @chamons

chamons · 2021-09-30T15:09:47Z

Talked to @rolfbjarne and we agree this shouldn't be an issue as we'll support either net6 or net7, and can fix anything up when we bump.

On VS2022 Preview 5, building a blazorwasm project which used sqlite native library crashed: ``` System.BadImageFormatException: Expected signature header for 'Property' or 'Method', but found '9' (0x09). at System.Reflection.Metadata.Ecma335.SignatureDecoder`2.CheckMethodOrPropertyHeader(SignatureHeader header) at System.Reflection.Metadata.Ecma335.SignatureDecoder`2.DecodeMethodSignature(BlobReader& blobReader) at System.Reflection.Metadata.Ecma335.SignatureDecoder`2.DecodeType(BlobReader& blobReader, Boolean allowlypeSpecifications, Int32 typeCode) at System.Reflection.Metadata.Ecma335.SignatureDecoder`2.DecodeMethodSignature(BlobReader& blobReader) at System.Reflection.Metadata.MethodDefinition.DecodeSignature[TType, TGenericContext] (ISignatureTypeProvider`2 provider, TGenericContext genericCo at System.Reflection.TypeLoading.Ecma.EcmaMethodDecoder.SpecializeMethodSigStrings(TypeContext& typeContext) at System.Reflection.TypeLoading.RoDefinitionMethod`1.ComputeMethodSigStrings() at System.Reflection.TypeLoading.RoMethod.ToString() at System.Text.StringBuilder.AppendFormatHelper(IFormatProvider provider, String format, ParamsArray args) at System.String.FormatHelper(IFormatProvider provider, String format, ParamsArray args) at System.String.Format(String format, Object arg0, Object arg1, Object arg2) at PInvokeComparer.GetHashCode(PInvoke pinvoke) at ... at System.Ling.Enumerable.<DistinctIterator>d__64`1.MoveNext() at System.Ling.Buffer`1..ctor(IEnumerable`1 source) at System.Ling.Enumerable.ToArray [TSource] (IEnumerable`1 source) at PInvokeTableGenerator.EmitPInvokeTable(StreamWriter w, Dictionary`2 modules, List`1 pinvokes) at PInvokeTableGenerator.GenPInvokeTable(String[] pinvokeModules, String[] assemblies) at PInvokeTableGenerator.Execute() ``` - This is crashing in a new code path(`PInvokeComparer.GetHashCode`) that invokes `methodInfo.ToString()` for methods with `DllImport` attribute. - It fails for `EnumCalendarInfo`, which has a function pointer parameter, which `System.Reflection.MetadataLoadContext`(SRMLC) does not support, and is explicitly worked around in the code. - It usually fails with on `methodInfo.GetParameters()` with `NotSupportedException`, and `methodInfo.ToString()` works fine The difference seems to be because the version of `SRMLC` being bundled with the task is `1.4.5.0`, which is too old. The fix is to use the same version of SRMLC, and `System.Reflection.Metadata`(SRM) used by other tasks, which is `5.0.0` currently. Also, this mirrors some other reference changes from dotnet#59720 - don't include msbuild assemblies with the task - and explicitly reference `System.Text.Json`, and `System.Threading.Tasks.Extensions` to be sure about the version that we'll include for net472 The following in a table of the assemblies that are bundled with the task, for `net472`, and `net6.0`. The `new` has fewer assemblies. | Path | Old | New | |---------------------------------------------------------------|-------------|-------------| | net472/Microsoft.Bcl.AsyncInterfaces.dll | 1.0.0.0 | 5.0.0.0 | | net472/Microsoft.Build.Framework.dll | 15.1.0.0 | | | net472/Microsoft.Build.Tasks.Core.dll | 15.1.0.0 | | | net472/Microsoft.Build.Utilities.Core.dll | 15.1.0.0 | | | net472/Microsoft.Build.dll | 15.1.0.0 | | | net472/Microsoft.NET.StringTools.dll | 1.0.0.0 | | | net472/Microsoft.VisualStudio.Setup.Configuration.Interop.dll | 1.0.0.0 | | | net472/System.Buffers.dll | 4.0.3.0 | 4.0.3.0 | | net472/System.Collections.Immutable.dll | 5.0.0.0 | 5.0.0.0 | | net472/System.Configuration.ConfigurationManager.dll | 4.0.3.0 | | | net472/System.Memory.dll | 4.0.1.1 | 4.0.1.1 | | net472/System.Numerics.Vectors.dll | 4.1.4.0 | 4.1.4.0 | | net472/System.Reflection.Metadata.dll | 1.4.5.0 | 5.0.0.0 | | net472/System.Reflection.MetadataLoadContext.dll | 4.0.1.1 | 5.0.0.0 | | net472/System.Resources.Extensions.dll | 4.0.0.0 | | | net472/System.Runtime.CompilerServices.Unsafe.dll | 5.0.0.0 | 5.0.0.0 | | net472/System.Security.AccessControl.dll | 4.1.3.0 | | | net472/System.Security.Permissions.dll | 4.0.3.0 | | | net472/System.Security.Principal.Windows.dll | 4.1.3.0 | | | net472/System.Text.Encodings.Web.dll | 4.0.5.0 | 5.0.0.0 | | net472/System.Text.Json.dll | 4.0.1.0 | 5.0.0.0 | | net472/System.Threading.Tasks.Dataflow.dll | 4.6.3.0 | | | net472/System.Threading.Tasks.Extensions.dll | 4.2.0.0 | 4.2.0.1 | | net472/System.ValueTuple.dll | 4.0.3.0 | 4.0.3.0 | | net472/WasmAppBuilder.dll | 6.0.0.0 | 6.0.0.0 | | | | | | net6.0/System.Reflection.MetadataLoadContext.dll | 4.0.1.1 | 5.0.0.0 | | net6.0/WasmAppBuilder.dll | 6.0.0.0 | 6.0.0.0 |

ghost added the area-Infrastructure-mono label Sep 28, 2021

lambdageek changed the title ~~Build one MonoTargetsTasks assembly for Mono.NET.Runtime.MonoTargets.Sdk pack~~Build one MonoTargetsTasks assembly for Microsoft.NET.Runtime.MonoTargets.Sdk packSep 28, 2021

runfoappBot mentioned this pull request Sep 28, 2021
RunContinueWithStressTestsNoState timing out in CI #2271
Closed

lambdageek force-pushed the single-mono-targets-tasks-assembly branch 3 times, most recently from 2c13699 to a6a0798Compare September 29, 2021 04:02

lambdageek added 3 commits September 29, 2021 11:24

Update testing and wasm to use MonoTargetsTasks.dll
9ed921d

FIXME: disable nullability for now
4ae1a32

lambdageek force-pushed the single-mono-targets-tasks-assembly branch from a6a0798 to 4ae1a32Compare September 29, 2021 15:27

lambdageek added 5 commits September 29, 2021 14:37

Move assembly stripper to a separate assembly
c1a5d6c
Build it with nullability checking disabled. Build the main MonoTargetsTasks assembly with nullability enabled

Don't include msbuild assemblies in MonoTargets package
747d88e

re-enable analyzers for the MonoTargetsTasks assembly
290ad79

Include *.dll in the MonoTargetsTasks nupkg
a0b5e8b
In particular, pick up all the dependent assemblies for the net472 build

Update various READMEs
9953d77

lambdageek marked this pull request as ready for review September 29, 2021 19:03

lambdageek requested a review from marek-safar as a code owner September 29, 2021 19:03

lambdageek requested review from ViktorHofer, akoeplinger and steveisok September 29, 2021 19:03

remove unneeded target
2e1d08e

lambdageek requested a review from radical September 29, 2021 19:30

radical reviewed Sep 29, 2021
View reviewed changes

Comment threadsrc/tasks/MonoTargetsTasks/MonoTargetsTasks.csproj

radical reviewed Sep 29, 2021
View reviewed changes

Comment threadsrc/tasks/MonoTargetsTasks/MonoTargetsTasks.csproj Outdated

Switch ILStrip to use the same Parallel.ForEach logic as MonoAOTCompiler
eb70042

radical approved these changes Sep 29, 2021
View reviewed changes

lambdageek added 2 commits September 29, 2021 20:06

Remove unneeded NoWarn warnings
979e9ac

rearrange package references in MonoTargetsTask.csproj for readabilty
8482287

radical reviewed Sep 30, 2021
View reviewed changes

Comment threadsrc/tasks/MonoTargetsTasks/MonoTargetsTasks.csproj Outdated

More ItemGroup reformatting
9eafb87
Co-authored-by: Ankit Jain <radical@gmail.com>

radical approved these changes Sep 30, 2021
View reviewed changes

ViktorHofer reviewed Sep 30, 2021
View reviewed changes

Comment threadsrc/tasks/MonoTargetsTasks/ILStrip/AssemblyStripper/AssemblyStripper.csproj Outdated

ViktorHofer reviewed Sep 30, 2021
View reviewed changes

Comment threadsrc/tasks/MonoTargetsTasks/ILStrip/AssemblyStripper/AssemblyStripper.csproj Outdated

ViktorHofer reviewed Sep 30, 2021
View reviewed changes

Comment threadsrc/tasks/MonoTargetsTasks/MonoTargetsTasks.csproj Outdated

ViktorHofer reviewed Sep 30, 2021
View reviewed changes

lambdageek added 2 commits September 30, 2021 08:28

fix nits
3d134c8

Use SystemThreadingTasksExtensionsVersion instead of hardcoding
b70035f

ViktorHofer approved these changes Sep 30, 2021
View reviewed changes

lambdageek added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 30, 2021

lambdageek mentioned this pull request Sep 30, 2021
[msbuild] Add ILStrip'ing for net6 applications. Fixes #11445. dotnet/macios#12563
Merged

lambdageek removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 30, 2021

lambdageek merged commit acb0050 into dotnet:mainSep 30, 2021

ghost locked as resolved and limited conversation to collaborators Nov 3, 2021

Navigation Menu

Uh oh!

Build one MonoTargetsTasks assembly for Microsoft.NET.Runtime.MonoTargets.Sdk pack - #59720

Build one MonoTargetsTasks assembly for Microsoft.NET.Runtime.MonoTargets.Sdk pack#59720
lambdageek merged 15 commits into
dotnet:mainfrom
lambdageek:single-mono-targets-tasks-assembly

lambdageek commented Sep 28, 2021•
edited
Loading

Uh oh!

ghost commented Sep 28, 2021

Uh oh!

lambdageek commented Sep 29, 2021

Uh oh!

Uh oh!

Uh oh!

radical left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ViktorHoferSep 30, 2021

Uh oh!

lambdageekSep 30, 2021

Uh oh!

ViktorHoferSep 30, 2021

Uh oh!

lambdageekSep 30, 2021

Uh oh!

lambdageek commented Sep 30, 2021

Uh oh!

lambdageek commented Sep 30, 2021•
edited
Loading

Uh oh!

chamons commented Sep 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

lambdageek commented Sep 28, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Sep 28, 2021

Uh oh!

lambdageek commented Sep 29, 2021

Uh oh!

Uh oh!

Uh oh!

radical left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ViktorHoferSep 30, 2021

Choose a reason for hiding this comment

Uh oh!

lambdageekSep 30, 2021

Choose a reason for hiding this comment

Uh oh!

ViktorHoferSep 30, 2021

Choose a reason for hiding this comment

Uh oh!

lambdageekSep 30, 2021

Choose a reason for hiding this comment

Uh oh!

lambdageek commented Sep 30, 2021

Uh oh!

lambdageek commented Sep 30, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chamons commented Sep 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lambdageek commented Sep 28, 2021•
edited
Loading

lambdageek commented Sep 30, 2021•
edited
Loading