[ci] Move .NET preview installation to xaprepare #5191

pjcollins · 2020-10-07T16:21:28Z

Context: https://github.com/dotnet/designs/blob/main/accepted/2020/workloads/workloads.md

In One .NET, our legacy installer payload content will be distributed
through a new "Optional Workload" concept. Early support for workloads
has now landed in VS and the .NET SDK, and we'd like to begin to convert
our monolithic MSBuild SDK NuGet into various workload packs. In an
effort to support dev and test efforts to this end, it will be helpful
to introduce a non-system installation of .NET that we can modify freely
without admin access or fear of leaving machines in a broken state.

pjcollins · 2020-10-07T21:32:32Z

Full run for commit f6dc8fb: https://dev.azure.com/devdiv/DevDiv/_build/results?buildId=4128812&view=results

jpobst · 2020-10-08T15:40:06Z

Shouldn't we be moving towards a future where xaprepare is built and run by .NET Core instead of .NET Framework/Mono? How is this going to affect that?

pjcollins · 2020-10-08T16:37:05Z

@jpobst The goal of these changes is to provide a non-system based installation of the .NET SDK that we can use for One .NET work, and modify freely (without requiring admin) for local dev and local + CI test needs without worrying about breaking system installations. It also allows us to consolidate our .NET SDK version dependency information into a single location.

Building xamarin-android still requires a system installation of .NET even with these changes (unless you update your $PATH to prefer this installation). In the future if we do move xaprepare to target net5 or something newer, we'll likely want to consider introducing some system installation bootstrap for .NET. I don't think we'll want or need to do this in the short or even mid term future though.

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Android/AndroidSdkResolver.cs

jpobst · 2020-10-08T17:40:15Z

It feels like we should keep the .NET Core 3.1 install the way it is today, and then install any additional needed .NETs with the new stuff. I think there's no reason we couldn't be running xaprepare with .NET Core today, other than no one has made the change.

build-tools/xaprepare/xaprepare/ConfigAndData/Configurables.MacOS.cs

build-tools/xaprepare/xaprepare/Steps/Step_InstallDotNetSDKs.cs

pjcollins · 2020-10-08T19:23:47Z

It feels like we should keep the .NET Core 3.1 install the way it is today, and then install any additional needed .NETs with the new stuff.

I see some downside with this approach. We'd be increasing the number of places where we are tracking .NET SDK dependency information, and we'd have to keep the 3.1 version in sync across multiple YAML files (including monodroid which is already out of sync, but we don't do a great job of syncing there in general). I'd also prefer to keep the .NET SDK versions we depend on for CI/testing in the same installation directory for my own sanity. Maybe there are benefits that outweigh these costs that I am overlooking?

I know @grendello was opposed to making this a .NET Core app when this was being written, I assume some of that reasoning still stands today? This tool seems like one of the lower priority things we'd want/need to upgrade to .NET Core when it comes to our build system,

jpobst · 2020-10-08T19:43:24Z

I think there are some benefits:

Mono and .NET Framework are deprecated, we should be working towards using them less. It'll be a very long process, but we shouldn't be going backwards.
The same command runs on all platforms, so we don't need the extra complexity of xaprepare.exe versus mono xaprepare.exe.
We could avoid the hacks in place so that xaprepare can update the system Mono while it is running on Mono. (https://github.com/xamarin/xamarin-android/blob/master/build-tools/automation/azure-pipelines.yaml#L164-L167)

grendello · 2020-10-08T19:47:38Z

I think there are some benefits:

* Mono and .NET Framework are deprecated, we should be working towards using them less.  It'll be a very long process, but we shouldn't be going backwards.

* The same command runs on all platforms, so we don't need the extra complexity of `xaprepare.exe` versus `mono xaprepare.exe`.

* We could avoid the hacks in place so that `xaprepare` can update the system Mono while it is running on Mono.  (https://github.com/xamarin/xamarin-android/blob/master/build-tools/automation/azure-pipelines.yaml#L164-L167)

One big disadvantage of this is that we would require both Mono and DotNet to be installed on the system with no, currently, clear advantage to using dotnet. xaprepare.exe is invoked via make on *nix so there's no benefit here either (on Windows it's invoked via MSBuild with /t:Prepare)

grendello · 2020-10-08T19:48:36Z

It feels like we should keep the .NET Core 3.1 install the way it is today, and then install any additional needed .NETs with the new stuff.

I see some downside with this approach. We'd be increasing the number of places where we are tracking .NET SDK dependency information, and we'd have to keep the 3.1 version in sync across multiple YAML files (including monodroid which is already out of sync, but we don't do a great job of syncing there in general). I'd also prefer to keep the .NET SDK versions we depend on for CI/testing in the same installation directory for my own sanity. Maybe there are benefits that outweigh these costs that I am overlooking?

I know @grendello was opposed to making this a .NET Core app when this was being written, I assume some of that reasoning still stands today? This tool seems like one of the lower priority things we'd want/need to upgrade to .NET Core when it comes to our build system,

Indeed. I'd much prefer to convert xaprepare to .net core only after we absolutely require dotnet to be installed.

jpobst · 2020-10-08T19:50:34Z

There will never be a "big bang" change where we convert 100% to dotnet in a single commit. The only way to do it is slowly when the opportunities arise. Putting it off is simply fighting against the inevitable future.

jpobst · 2020-10-08T19:52:42Z

One big disadvantage of this is that we would require both Mono and DotNet to be installed on the system with no, currently, clear advantage to using dotnet. xaprepare.exe is invoked via make on *nix so there's no benefit here either (on Windows it's invoked via MSBuild with /t:Prepare)

We do require both Mono and DotNet to be installed on the system. This is simply about which is used to bootstrap setting them up.

grendello · 2020-10-08T20:16:00Z

There will never be a "big bang" change where we convert 100% to dotnet in a single commit. The only way to do it is slowly when the opportunities arise. Putting it off is simply fighting against the inevitable future.

I just don't see any compelling advantage from this change with regards to xaprepare as of yet

The '-- RunSettings' test option is not available in 3.1

jonpryor · 2020-10-09T14:48:50Z

Context: https://github.com/dotnet/designs/blob/main/accepted/2020/workloads/workloads.md

For .NET 6, our .NET 6 installer payload content will be distributed
through a new "Optional Workload" concept.

Early support for workloads has now landed in VS and the .NET SDK,
and we'd like to begin to convert our monolithic MSBuild SDK NuGet into
various workload packs.  In an effort to support dev and test efforts
to this end, it will be helpful to introduce a non-system installation
of .NET that we can modify freely without admin access or fear of
leaving machines in a broken state.

Update the `xaprepare -s:Standard` scenario to install multiple
`dotnet` versinos into `$(DotNetPreviewPath)`, which defaults to
`$(AndroidToolchainDirectory)\dotnet`.

The currently installed `dotnet` versions are:

  * 3.1.3: Stable version, for some tests.
  * 5.0.100-rc.2.20480.7: Preview .NET 5 RC which contains support
    for the Optional Workload concept.

Context: dotnet#5293 PR dotnet#5293 introduced an incompatibility with xabuild, and it appears to be causing issues with other stable builds. The [UseDotNet][1] task has a caching feature to improve "installation" times for certain versions, however this can cause issues for our non-hosted build machines. We should continue to use the custom `use-dot-net` task that Jonathan Peppers added for us during earlier .NET 5 preview work. This template was removed in PR dotnet#5191 but in hindsight it should not have been. [1]: https://docs.microsoft.com/en-us/azure/devops/pipelines/tasks/tool/dotnet-core-tool-installer?view=azure-devops

Context: dotnet#5293 PR dotnet#5293 introduced an incompatibility with xabuild, and it appears to be causing issues with other stable builds. The [UseDotNet][1] task has a caching feature to improve "installation" times for certain versions, however this can cause issues for our non-hosted build machines. /Users/builder/azdo/_work/_tool/dotnet/sdk/5.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.TargetFrameworkInference.targets(54,5): error MSB4186: Invalid static method invocation syntax: "[MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')". Method '[MSBuild]::GetTargetFrameworkIdentifier' not found. Static method invocation should be of the form: $([FullTypeName]::Method()), e.g. $([System.IO.Path]::Combine(`a`, `b`)). Check that all parameters are defined, are of the correct type, and are specified in the right order. [/Users/builder/azdo/_work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop.GenericMarshaler/Java.Interop.GenericMarshaler.csproj] We should continue to use the custom `use-dot-net` task that Jonathan Peppers added for us during earlier .NET 5 preview work. This template was removed in PR dotnet#5191 but in hindsight it should not have been. [1]: https://docs.microsoft.com/en-us/azure/devops/pipelines/tasks/tool/dotnet-core-tool-installer?view=azure-devops

@jonathanpeppers

Context: #5293 PR #5293 introduced an incompatibility with `xabuild`, and it appears to be causing issues with other stable builds. The [UseDotNet][1] task has a caching feature to improve "installation" times for certain versions, however this can cause issues for our non-hosted build machines. /Users/builder/azdo/_work/_tool/dotnet/sdk/5.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.TargetFrameworkInference.targets(54,5): error MSB4186: Invalid static method invocation syntax: "[MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')". Method '[MSBuild]::GetTargetFrameworkIdentifier' not found. Static method invocation should be of the form: $([FullTypeName]::Method()), e.g. $([System.IO.Path]::Combine(`a`, `b`)). Check that all parameters are defined, are of the correct type, and are specified in the right order. We should continue to use the custom `use-dot-net` task that @jonathanpeppers added for us during earlier .NET 5 preview work. This template was removed in PR #5191 but in hindsight it should not have been. [1]: https://docs.microsoft.com/azure/devops/pipelines/tasks/tool/dotnet-core-tool-installer

pjcollins force-pushed the local-dotnet branch 3 times, most recently from 9b75680 to 8717b68 Compare October 7, 2020 18:19

[ci] Move .NET SDK installation to xaprepare

f6dc8fb

pjcollins force-pushed the local-dotnet branch from a8d1f08 to f6dc8fb Compare October 7, 2020 21:30

pjcollins force-pushed the local-dotnet branch 2 times, most recently from cb2e582 to ee085ca Compare October 7, 2020 23:46

Apply @jonathanpeppers feedback for local test execution

3aa4c9e

pjcollins force-pushed the local-dotnet branch from ee085ca to 3aa4c9e Compare October 8, 2020 13:51

pjcollins marked this pull request as ready for review October 8, 2020 16:41

pjcollins requested review from dellis1972, grendello, jonathanpeppers and jonpryor as code owners October 8, 2020 16:41

jonathanpeppers reviewed Oct 8, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Android/AndroidSdkResolver.cs Outdated Show resolved Hide resolved

grendello requested changes Oct 8, 2020

View reviewed changes

pjcollins changed the title ~~[ci] Move .NET SDK installation to xaprepare~~ [ci] Move .NET preview installation to xaprepare Oct 8, 2020

Apply feedback

7613628

pjcollins requested a review from grendello October 8, 2020 22:01

grendello approved these changes Oct 8, 2020

View reviewed changes

jonathanpeppers approved these changes Oct 8, 2020

View reviewed changes

pjcollins and others added 2 commits October 8, 2020 20:28

Use dotnet preview to run msbuild tests

980bfe6

The '-- RunSettings' test option is not available in 3.1

Install the required runtime for netcoreapp3.1 tests

b0e810c

jpobst approved these changes Oct 9, 2020

View reviewed changes

jonpryor merged commit cdc35b6 into dotnet:master Oct 9, 2020

pjcollins deleted the local-dotnet branch October 9, 2020 14:52

brendanzagaeski added this to the In Preview: 11.2 (d16-9) milestone Nov 10, 2020

pjcollins mentioned this pull request Nov 16, 2020

[ci] Restore custom use-dot-net task #5297

Merged

github-actions bot locked and limited conversation to collaborators Jan 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ci] Move .NET preview installation to xaprepare #5191

[ci] Move .NET preview installation to xaprepare #5191

Uh oh!

pjcollins commented Oct 7, 2020 •

edited

Loading

Uh oh!

pjcollins commented Oct 7, 2020

Uh oh!

jpobst commented Oct 8, 2020

Uh oh!

pjcollins commented Oct 8, 2020 •

edited

Loading

Uh oh!

Uh oh!

jpobst commented Oct 8, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pjcollins commented Oct 8, 2020

Uh oh!

jpobst commented Oct 8, 2020

Uh oh!

grendello commented Oct 8, 2020

Uh oh!

grendello commented Oct 8, 2020

Uh oh!

jpobst commented Oct 8, 2020

Uh oh!

jpobst commented Oct 8, 2020

Uh oh!

grendello commented Oct 8, 2020

Uh oh!

jonpryor commented Oct 9, 2020

Uh oh!

Uh oh!

[ci] Move .NET preview installation to xaprepare #5191

[ci] Move .NET preview installation to xaprepare #5191

Uh oh!

Conversation

pjcollins commented Oct 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pjcollins commented Oct 7, 2020

Uh oh!

jpobst commented Oct 8, 2020

Uh oh!

pjcollins commented Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jpobst commented Oct 8, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pjcollins commented Oct 8, 2020

Uh oh!

jpobst commented Oct 8, 2020

Uh oh!

grendello commented Oct 8, 2020

Uh oh!

grendello commented Oct 8, 2020

Uh oh!

jpobst commented Oct 8, 2020

Uh oh!

jpobst commented Oct 8, 2020

Uh oh!

grendello commented Oct 8, 2020

Uh oh!

jonpryor commented Oct 9, 2020

Uh oh!

Uh oh!

pjcollins commented Oct 7, 2020 •

edited

Loading

pjcollins commented Oct 8, 2020 •

edited

Loading