-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Description
The Android (32) workload currently ships four runtime packs, one for each Android architecture:
Microsoft.Android.Runtime.32.android-arm
Microsoft.Android.Runtime.32.android-arm64
Microsoft.Android.Runtime.32.android-x86
Microsoft.Android.Runtime.32.android-x64
In order to reduce our installation footprint we have a desire to introduce a new runtime pack for managed assemblies that are not architecture specific, but are still only intended to be used with the parent android
RID. Right now these managed assemblies are duplicated in all four packs mentioned above. Ideally, we would move the non-architecture specific assemblies into a new pack:
Microsoft.Android.Runtime.32.android
I tried to get this to work by adding a second RuntimePackNamePatterns
value to the KnownFrameworkReference
we declare:
<KnownFrameworkReference
Include="Microsoft.Android"
TargetFramework="net6.0"
RuntimeFrameworkName="Microsoft.Android"
LatestRuntimeFrameworkVersion="**FromWorkload**"
TargetingPackName="Microsoft.Android.Ref.32"
TargetingPackVersion="**FromWorkload**"
- RuntimePackNamePatterns="Microsoft.Android.Runtime.32.**RID**"
+ RuntimePackNamePatterns="Microsoft.Android.Runtime.32.android;Microsoft.Android.Runtime.32.**RID**"
RuntimePackRuntimeIdentifiers="android-arm;android-arm64;android-x86;android-x64"
Profile="Android"
/>
The ProcessFrameworkReferences
task handles this approach, but the ResolveFrameworkReferences task breaks as it only allows for a 1:1 relationship between runtime pack and framework reference:
Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(326,5): error MSB4018: System.ArgumentException: An item with the same key has already been added. Key: Microsoft.Android
I'm wondering how risky or big of a breaking change it would be to change the RuntimePack*
metadata on ResolvedFrameworkReferences
, and do something like this instead:
--- a/src/Tasks/Microsoft.NET.Build.Tasks/ResolveFrameworkReferences.cs
+++ b/src/Tasks/Microsoft.NET.Build.Tasks/ResolveFrameworkReferences.cs
@@ -26,7 +26,6 @@ protected override void ExecuteCore()
}
var resolvedTargetingPacks = ResolvedTargetingPacks.ToDictionary(tp => tp.ItemSpec, StringComparer.OrdinalIgnoreCase);
- var resolvedRuntimePacks = ResolvedRuntimePacks.ToDictionary(rp => rp.GetMetadata(MetadataKeys.FrameworkName), StringComparer.OrdinalIgnoreCase);
var resolvedFrameworkReferences = new List<TaskItem>(FrameworkReferences.Length);
@@ -48,12 +47,11 @@ protected override void ExecuteCore()
resolvedFrameworkReference.SetMetadata("TargetingPackVersion", targetingPack.GetMetadata(MetadataKeys.NuGetPackageVersion));
resolvedFrameworkReference.SetMetadata("Profile", targetingPack.GetMetadata("Profile"));
- ITaskItem runtimePack;
- if (resolvedRuntimePacks.TryGetValue(frameworkReference.ItemSpec, out runtimePack))
- {
- resolvedFrameworkReference.SetMetadata("RuntimePackPath", runtimePack.GetMetadata(MetadataKeys.PackageDirectory));
- resolvedFrameworkReference.SetMetadata("RuntimePackName", runtimePack.GetMetadata(MetadataKeys.NuGetPackageId));
- resolvedFrameworkReference.SetMetadata("RuntimePackVersion", runtimePack.GetMetadata(MetadataKeys.NuGetPackageVersion));
+ var matchingPacks = ResolvedRuntimePacks.Where(rp => rp.GetMetadata(MetadataKeys.FrameworkName).Equals(frameworkReference.ItemSpec, StringComparison.OrdinalIgnoreCase));
+ if (matchingPacks.Any()) {
+ resolvedFrameworkReference.SetMetadata("RuntimePackPath", string.Join (";", matchingPacks.Select (mp => mp.GetMetadata(MetadataKeys.PackageDirectory))));
+ resolvedFrameworkReference.SetMetadata("RuntimePackName", string.Join (";", matchingPacks.Select (mp => mp.GetMetadata(MetadataKeys.NuGetPackageId))));
+ resolvedFrameworkReference.SetMetadata("RuntimePackVersion", string.Join (";", matchingPacks.Select (mp => mp.GetMetadata(MetadataKeys.NuGetPackageVersion))));
}
resolvedFrameworkReferences.Add(resolvedFrameworkReference);
We may also want to update the metadata names, but that might cause more breakage. These changes would produce ;
separated RuntimePack*
metadata values, for example:
RuntimePackName = Microsoft.Android.Runtime.32.android;Microsoft.Android.Runtime.32.android-x64
These changes to ResolveFrameworkReferences
appear to work as needed for the Android use case. I've also got something working on the workload side by introducing a new FrameworkReference
for Android, but I am not convinced that this FrameworkReference
addition is the most sane or desirable approach.
Maybe there are other approaches that would support a "generic RID" runtime pack that I am not considering?
Activity
ghost commentedon Feb 23, 2022
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.
pjcollins commentedon Feb 23, 2022
Adding @dsplaisted and @jonathanpeppers to help weigh in on this, I think it would be something we may want to do for .NET 7.
dsplaisted commentedon Feb 23, 2022
This makes sense to me, and I think the multiple runtime pack approach per FrameworkReference approach is preferrable to the multiple FrameworkReference approach.
baronfel commentedon Feb 25, 2022
We'd love to see this for 7.0! Of course we'd accept a PR, but otherwise the team will work it in as time/scheduling allows.
jonathanpeppers commentedon Feb 25, 2022
@pjcollins it might work without any code changes if we did:
Then put native libraries for every RID inside? Release apps use all 4 architectures by default anyway -- don't need them split into separate packs?
This is similar to what Maui does as there are no native libraries:
https://github.com/dotnet/maui/blob/4a0f92458da62006b347f5eb7e29e06e8d7b5f97/src/Workload/Microsoft.Maui.Sdk/Sdk/BundledVersions.in.targets#L18-L31
%(RuntimePackRuntimeIdentifiers)
is justandroid
for this one.pjcollins commentedon Feb 25, 2022
Thanks that is a good point given how small our architecture specific libraries are, I will try to play around with that option as well. I can probably open a PR against
ResolveFrameworkReferences
at some point next week if it ends up still being needed.pjcollins commentedon Mar 1, 2022
I spent some time seeing if we could fit everything into a single runtime pack:
With this approach I've ran into an issue with the ResolveRuntimePackAssets task, which generally doesn't support nested files with the same name:
To move forward with the single runtime pack suggestion we could try to update this task to handle this case, or alternatively rename the architecture specific components so that they are unique and add custom processing tasks/targets to our Android SDK.
I think I am still leaning towards the approach of adding a new runtime pack for managed assemblies and adding support for multiple runtime packs per framework reference.
8 remaining items
jeromelaban commentedon Sep 19, 2023
As there's a bit of discussion around workload sizes, though I'm not sure if this is the appropriate issue to discuss this, I've been wondering if having library-only (reference maybe?) workloads would be possible?
It would save a lot build time for package authors, as we're still around 3 minutes of install time during each build.
jonathanpeppers commentedon Sep 19, 2023
I think there is a tradeoff here between what the average user wants installed (everything) and what a CI build might want. We could create lots of tiny workloads, but then
dotnet workload search
starts to be a long list.With the
android
workload as an example, you could remove these lines and probably still build class libraries?https://github.com/xamarin/xamarin-android/blob/554eb9e6dd9944c68707fbe216aa91f2df14fc8a/src/Xamarin.Android.Build.Tasks/Microsoft.NET.Sdk.Android/WorkloadManifest.in.json#L17-L22
Maybe you could give that a test during your build, remove this
extends
json array? If everything still works, that might be 50% of the install time.jeromelaban commentedon Sep 19, 2023
Thanks for the hints, I will try that!
[workload] Add Microsoft.Android.Runtimes framework (#10038)
Context: dotnet/sdk#24077
[net10.0] [workload] Add Microsoft.$(platform).Runtimes frameworks (#…