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

Investigate failures in WorkloadTests when using net10.0 workload manifests #45516

Open
akoeplinger opened this issue Dec 17, 2024 · 10 comments
Open
Assignees
Labels
Area-Workloads untriaged Request triage from a team member
Milestone

Comments

@akoeplinger
Copy link
Member

When bringing in the new workload manifests and bumping to net10.0 from #45500 we see failures in these workloads tests, these need to be investigated.

  • WorkloadTests.It_should_get_suggested_workload_by_GetRequiredWorkloads_target
  • WorkloadTests.Given_multi_target_It_should_get_suggested_workload_by_GetRequiredWorkloads_target
@marcpopMSFT
Copy link
Member

I think the test failures were because our restore logic heuristic is not always right. Can you review that logic and see if there is something obvious that should be fixed? #45500 (comment)

Basically, we try to report the fewest number of workloads to install but sometimes end up recommending wasi-experimental which should probably never be in that least for a android/ios app so there's something off.

@dsplaisted
Copy link
Member

The test was only failing on Linux, right? Are the maui workloads at all available for Linux? If they're not, then I can imagine the logic falling back to the a workload like wasi-experimental.

Maybe the test should be disabled just for Linux.

@akoeplinger
Copy link
Member Author

akoeplinger commented Dec 20, 2024

It was failing on macOS for me as well.

The full MAUI isn't available on Linux but the android workload is.

@Forgind
Copy link
Member

Forgind commented Jan 6, 2025

Do we have other tests that request 10.0 android, i.e., targeting net10.0-android? I looked on nuget.org and didn't see any of the manifest packages I'd expect:
Image

I did see quite a few preview 9.0.100 manifests.

@akoeplinger
Copy link
Member Author

you won't see them on nuget.org, I'm not even sure we have android manifests for 10.0.100 in the dnceng feeds already.

@Forgind
Copy link
Member

Forgind commented Jan 7, 2025

I spent a good bit of time looking into this between yesterday and today. The proximal cause for selecting mobile-librarybuilder is that it covers all the missing packs and has fewer packs that need to be added. By itself, that seems like a good reason to pick mobile-librarybuilder over android, but we didn't make the same choice in the past. So what changed between 9.0 and 10.0?

The android and mobile-librarybuilder workloads seem equivalent between 9.0 and 10.0. In both cases, android has 1 more pack that would have to be installed. In both cases it has a Microsoft.Android.Sdk.net9 pack. But in 10.0, we don't request that pack, whereas in 9.0 we do. How do we decide that we do or don't need that pack?

In 9.0, we resolve an SDK that seems relevant:
Image

In 10.0, we resolve several android runtime-relevant SDKs but not that SDK. It seems that having 9.0-android as a TargetFramework means that you have an ImportGroup with that SDK that pushes MSBuild to look for a way to resolve it, whereas that isn't the case for 10.0-android.

Why is that? I don't know how we go from a TF to a set of SDKs we decide we need to import. @dsplaisted or @rainersigwald, this seems like something one of you might know?

@dsplaisted
Copy link
Member

Why is that? I don't know how we go from a TF to a set of SDKs we decide we need to import.

This is defined in the various WorkloadManifest.targets files (which are part of the workload manifests), and can use arbitrarily complex conditions.

For example, the Android imports look something like this:

  <ImportGroup Condition=" '$(TargetPlatformIdentifier)' == 'android' ">
    <Import Project="Sdk.targets" Sdk="Microsoft.Android.Sdk.net9"
        Condition=" $([MSBuild]::VersionEquals($(TargetFrameworkVersion), '9.0')) " />
    <Import Project="Sdk.targets" Sdk="Microsoft.Android.Sdk.net8"
        Condition=" $([MSBuild]::VersionEquals($(TargetFrameworkVersion), '8.0')) " />
    <Import Project="Eol.targets" Sdk="Microsoft.Android.Sdk.net9"
        Condition=" $([MSBuild]::VersionEquals($(TargetFrameworkVersion), '7.0')) or $([MSBuild]::VersionEquals($(TargetFrameworkVersion), '6.0')) " />
  </ImportGroup>

@Forgind
Copy link
Member

Forgind commented Jan 8, 2025

And that's the problem! There's nothing for TF == 10.0 🙂 Turns out it's a very easy fix. Thanks dsplaisted!

@akoeplinger
Copy link
Member Author

Ah interesting. That condition will get added with dotnet/android#9395 which is currently blocked on dotnet/msbuild#11237

This is another chicken and egg situation when bumping to 10.0, maybe we can make the test more independent of those cases.

@Forgind
Copy link
Member

Forgind commented Jan 9, 2025

I think that would just mean choosing workloads that don't depend on .NET version like mobile-librarybuilder (I think). I think that's a good direction, though I'm not sure at the moment how to make a project that indicates that it needs that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Workloads untriaged Request triage from a team member
Projects
None yet
Development

No branches or pull requests

4 participants