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

Add Android workload manifests to SB .NET SDK #42125

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Jul 12, 2024

Context: dotnet/source-build#3242

If using a source-built .NET SDK, the Android workload manifests are
not included in the SDK. So, you won't see android workload when you
run dotnet workload search.

The Android workload, in general, contains private sources, but the
manifest files are completely public.

In the long run, we could consider onboarding dotnet/android to:

But the prerequisites would be to first have the Android workload
onboard to arcade:

For now, we could include the Android workload in source-build by:

  • Add a step that copies the Android workload manifests source code
    in-tree (to be used when updating manifests).

  • Commit the manifest sources.

  • Source-build will then include the Android workload manifests in
    the SDK by using the source code, in-tree.

Context: dotnet/source-build#3242

If using a source-built .NET SDK, the Android workload manifests are
not included in the SDK. So, you won't see `android` workload when you
run `dotnet workload search`.

The Android workload, in general, contains private sources, but the
manifest files are completely public.

In the long run, we could consider onboarding dotnet/android to:

* https://github.com/dotnet/source-build/blob/main/Documentation/sourcebuild-in-repos/new-repo.md

But the prerequisites would be to first have the Android workload
onboard to arcade:

* https://github.com/dotnet/arcade/blob/main/Documentation/Onboarding.md

*For now*, we could include the Android workload in source-build by:

* Add a step that copies the Android workload manifests source code
  in-tree (to be used when updating manifests).

* Commit the manifest sources.

* Source-build will then include the Android workload manifests in
  the SDK by using the source code, in-tree.
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Workloads untriaged Request triage from a team member labels Jul 12, 2024
Comment on lines +108 to +112
<!-- Do not copy for source build, should already be in-tree -->
<Copy Condition="'$(DotNetBuildSourceOnly)' != 'true'"
SourceFiles="@(SourceManifestContent)"
DestinationFolder="sdk-manifests/%(DestinationPath)"
SkipUnchangedFiles="true" />
Copy link
Member Author

Choose a reason for hiding this comment

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

This is for convenience of someone updating the manifests (likely me). We could put it behind a property like -p:UpdateManifestSources=true, if we are concerned about this running.

@baronfel
Copy link
Member

cc @dotnet/source-build-internal this change updates the source-built SDK - does running the build script with -sb create an artifact that @jonathanpeppers can use to test the changes? That should all work from an e.g. Ubuntu WSL, right?

@MichaelSimons
Copy link
Member

cc @dotnet/source-build-internal this change updates the source-built SDK - does running the build script with -sb create an artifact that @jonathanpeppers can use to test the changes? That should all work from an e.g. Ubuntu WSL, right?

No it won't. You need the sdk tarball that is produced from the sdk-source-build leg.

@jonathanpeppers
Copy link
Member Author

jonathanpeppers commented Jul 22, 2024

Ok, it seems to be working, I tested in a GitHub codespace:

  1. Get dotnet-sdk-9.0.100-preview.7.24366.1-centos.9-x64.tar.gz from build artifacts
  2. mkdir test-sdk && cd test-sdk
  3. tar -xvzf ../dotnet-sdk-9.0.100-preview.7.24366.1-centos.9-x64.tar.gz
  4. $ ./dotnet workload search (this used to not even work)
Workload ID                 Description                                         
--------------------------------------------------------------------------------
android                     .NET SDK Workload for building Android applications.
  1. $ ./dotnet workload install android
Skipping NuGet package signature verification.
...
Installing pack Microsoft.Android.Sdk.Linux version 34.99.0-preview.6.340...
...
Successfully installed workload(s) android.
  1. Make a helloandroid folder somewhere and $ ../sdk/test-sdk/dotnet new android
The template "Android Application" was created successfully.
  1. Now we need some Android dependencies:
$ ../sdk/test-sdk/dotnet build -t:InstallAndroidDependencies -p:AndroidSdkDirectory=/home/vscode/android-sdk/ -p:JavaSdkDirectory=/home/vscode/jdk/ -p:AcceptAndroidSDKLicenses=true
  1. Build the app:
$ ../sdk/test-sdk/dotnet build -bl -p:AndroidSdkDirectory=/home/vscode/android-sdk/ -p:JavaSdkDirectory=/home/vscode/jdk/
Restore complete (0.5s)
You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  helloandroid succeeded (17.4s) → bin/Debug/net9.0-android/helloandroid.dll

Build succeeded in 19.5s

$ ls -la bin/Debug/net9.0-android/*.apk
-rw-rw-rw- 1 vscode vscode 10224521 Jul 22 20:43 bin/Debug/net9.0-android/com.companyname.helloandroid.apk
-rw-r--rw- 1 vscode vscode 10237801 Jul 22 20:43 bin/Debug/net9.0-android/com.companyname.helloandroid-Signed.apk

@jonathanpeppers jonathanpeppers marked this pull request as ready for review July 22, 2024 20:45
@jonathanpeppers jonathanpeppers requested a review from a team as a code owner July 22, 2024 20:45
@jonathanpeppers
Copy link
Member Author

@MichaelSimons there is an error like:

/vmr/repo-projects/Directory.Build.targets(765,5): warning : 1 new packages used not in baseline! See report at /vmr/artifacts/prebuilt-report/baseline-comparison.xml for more information. Package IDs are: [/vmr/repo-projects/dotnet.proj]
/vmr/repo-projects/Directory.Build.targets(765,5): warning : Microsoft.NET.Sdk.Android.Manifest-9.0.100-preview.6.34.99.0-preview.6.340 [/vmr/repo-projects/dotnet.proj]
/vmr/repo-projects/Directory.Build.targets(765,5): warning : Prebuilt usages are different from the baseline. If detected changes are acceptable, update baseline with: [/vmr/repo-projects/dotnet.proj]
/vmr/repo-projects/Directory.Build.targets(765,5): warning : cp '/vmr/artifacts/prebuilt-report/generated-new-baseline.xml' '/vmr/eng/tools/prebuilt-baseline.xml' [/vmr/repo-projects/dotnet.proj]
  Packaged all symbols in '/vmr/artifacts/assets/Release/dotnet-symbols-all-9.0.100-preview.7.24366.1-centos.9-x64.tar.gz'
  Packaged sdk symbols in '/vmr/artifacts/assets/Release/dotnet-symbols-sdk-9.0.100-preview.7.24366.1-centos.9-x64.tar.gz'
  Found 1 files in prebuilt packages dir.
  Tarball '/vmr/artifacts/assets/Release/Private.SourceBuilt.Prebuilts.9.0.100-preview.7.24366.1.centos.9-x64.tar.gz' was successfully created from '/vmr/artifacts/prebuilt-report/prebuilt-packages/'
/vmr/eng/finish-source-only.proj(171,5): error : 1 Prebuilts Exist

Can that be fixed in this repo?

@@ -7,6 +7,9 @@
<UsagePattern IdentityGlob="Nuget.*/*" />
<UsagePattern IdentityGlob="Microsoft.Build.NuGetSdkResolver/*" />

<!-- Sources for this package are included in-repo -->
<UsagePattern IdentityGlob="Microsoft.NET.Sdk.Android.Manifest-*/*" />
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this should be added here. If it is in this repo, then it shouldn't be detected as a prebuilt. I suspect something isn't working as expected.

Copy link
Member Author

Choose a reason for hiding this comment

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

How does it "find" prebuilts? Would <PackageDownload Include="Foo" Version="[1.0.0]"/> trigger this?

Copy link
Member

Choose a reason for hiding this comment

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

The package is being included as a download dependency:
image

So in this case, yes, <PackageDownload Include="Microsoft.NET.Sdk.Android.Manifest" .../> is triggering the prebuilt.

Copy link
Member

@ellahathaway ellahathaway Jul 22, 2024

Choose a reason for hiding this comment

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

More often than not, prebuilts are caused by package references & transitive dependencies. Technically, any package used in the build that is not built from source is a prebuilt, so it makes sense that a download dependency would cause a prebuilt, but this is the first time I've seen a package download cause a prebuilt.

Co-authored-by: Michael Simons <msimons@microsoft.com>
@@ -31,12 +31,14 @@
<RestoredNupkgContentPath>$(NuGetPackageRoot)$([MSBuild]::ValueOrDefault('%(NupkgId)', '').ToLower())/$([MSBuild]::ValueOrDefault('%(Version)', '').ToLower())</RestoredNupkgContentPath>
<MsiNupkgId>%(Identity).Manifest-%(FeatureBand).Msi.$(MsiArchitectureForWorkloadManifests)</MsiNupkgId>
<RestoredMsiNupkgContentPath>$(NuGetPackageRoot)$([MSBuild]::ValueOrDefault('%(MsiNupkgId)', '').ToLower())/$([MSBuild]::ValueOrDefault('%(Version)', '').ToLower())</RestoredMsiNupkgContentPath>
<IncludedInSource Condition="'%(IncludedInSource)' == ''">false</IncludedInSource>
<PackageDownload Condition="'$(DotNetBuildSourceOnly)' != 'true' or '%(IncludedInSource)' != 'true'">true</PackageDownload>
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to not specialize this for source-build? e.g. the MSFT build would also use the version from the SDK repo. It is best to avoid unnecessary differences between the two builds. Keeping them the same helps flush out issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can rework this to use a property like -p:UpdateManifestSources=true, and then it wouldn't need to ever check $(DotNetBuildSourceOnly). Someone would only set this property when updating the manifest sources.

Is the build green (or need a rerun)? I don't think I've seen it fully green yet, so just wondering if it works before I refactor it. 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I think this got past the pre-built errors now, but some of the smoke tests run into:

System.InvalidOperationException : Failed to execute /vmr/artifacts/bin/Microsoft.DotNet.SourceBuild.SmokeTests/Release/extracted-sdk/dotnet test /bl:/vmr/artifacts/TestResults/Release/BasicScenarioTests_MSTest_CSharp-test.binlog
Exit code: 1
/vmr/artifacts/bin/Microsoft.DotNet.SourceBuild.SmokeTests/Release/projects-202407231701323151/BasicScenarioTests_MSTest_CSharp/BasicScenarioTests_MSTest_CSharp.csproj : error MSB4242: SDK Resolver Failure: "The SDK resolver "Microsoft.DotNet.MSBuildWorkloadSdkResolver" failed while attempting to resolve the SDK "MSTest.Sdk/3.4.3". Exception: "System.IO.FileNotFoundException: Workload manifest microsoft.net.sdk.android: 34.99.0-preview.6.340/9.0.100-preview.6 from workload version 9.0.100-preview.7.24373.1 was not installed. Running "dotnet workload repair" may resolve this.
/vmr/artifacts/bin/Microsoft.DotNet.SourceBuild.SmokeTests/Release/projects-202407231701323151/BasicScenarioTests_MSTest_CSharp/BasicScenarioTests_MSTest_CSharp.csproj : error MSB4242:    at Microsoft.NET.Sdk.WorkloadManifestReader.SdkDirectoryWorkloadManifestProvider.GetManifests()
/vmr/artifacts/bin/Microsoft.DotNet.SourceBuild.SmokeTests/Release/projects-202407231701323151/BasicScenarioTests_MSTest_CSharp/BasicScenarioTests_MSTest_CSharp.csproj : error MSB4242:    at Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadResolver.LoadManifestsFromProvider(IWorkloadManifestProvider manifestProvider)
/vmr/artifacts/bin/Microsoft.DotNet.SourceBuild.SmokeTests/Release/projects-202407231701323151/BasicScenarioTests_MSTest_CSharp/BasicScenarioTests_MSTest_CSharp.csproj : error MSB4242:    at Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadResolver.InitializeManifests()
/vmr/artifacts/bin/Microsoft.DotNet.SourceBuild.SmokeTests/Release/projects-202407231701323151/BasicScenarioTests_MSTest_CSharp/BasicScenarioTests_MSTest_CSharp.csproj : error MSB4242:    at Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadResolver.TryGetPackInfo(WorkloadPackId packId)
/vmr/artifacts/bin/Microsoft.DotNet.SourceBuild.SmokeTests/Release/projects-202407231701323151/BasicScenarioTests_MSTest_CSharp/BasicScenarioTests_MSTest_CSharp.csproj : error MSB4242:    at Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.CachingWorkloadResolver.Resolve(String sdkReferenceName, IWorkloadManifestProvider manifestProvider, IWorkloadResolver workloadResolver)
/vmr/artifacts/bin/Microsoft.DotNet.SourceBuild.SmokeTests/Release/projects-202407231701323151/BasicScenarioTests_MSTest_CSharp/BasicScenarioTests_MSTest_CSharp.csproj : error MSB4242:    at Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.CachingWorkloadResolver.Resolve(String sdkReferenceName, String dotnetRootPath, String sdkVersion, String userProfileDir, String globalJsonPath)
/vmr/artifacts/bin/Microsoft.DotNet.SourceBuild.SmokeTests/Release/projects-202407231701323151/BasicScenarioTests_MSTest_CSharp/BasicScenarioTests_MSTest_CSharp.csproj : error MSB4242:    at Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.WorkloadSdkResolver.Resolve(SdkReference sdkReference, SdkResolverContext resolverContext, SdkResultFactory factory)
/vmr/artifacts/bin/Microsoft.DotNet.SourceBuild.SmokeTests/Release/projects-202407231701323151/BasicScenarioTests_MSTest_CSharp/BasicScenarioTests_MSTest_CSharp.csproj : error MSB4242:    at Microsoft.Build.BackEnd.SdkResolution.SdkResolverService.TryResolveSdkUsingSpecifiedResolvers(IReadOnlyList`1 resolvers, Int32 submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, String solutionPath, String projectPath, Boolean interactive, Boolean isRunningInVisualStudio, SdkResult& sdkResult, IEnumerable`1& errors, IEnumerable`1& warnings)""

I'll see if I can reproduce this on GitHub codespaces.

@MichaelSimons
Copy link
Member

@jonathanpeppers, is this something that is still in scope for 9.0?

@jonathanpeppers
Copy link
Member Author

@MichaelSimons yes, unfortunately I've had to focus on other things for a bit.

Linux support is just lower on the list than anything else, but we'd still like to complete this at some point.

Maybe if this doesn't make RC 2, we do it for .NET 10?

@MichaelSimons
Copy link
Member

@MichaelSimons yes, unfortunately I've had to focus on other things for a bit.

Linux support is just lower on the list than anything else, but we'd still like to complete this at some point.

Maybe if this doesn't make RC 2, we do it for .NET 10?

Thanks, if you need help diagnosing source-build related failures, let me know and I can help.

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

Successfully merging this pull request may close these issues.

None yet

4 participants