-
Notifications
You must be signed in to change notification settings - Fork 534
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
[tests, build] multi-target Xamarin.Android.Build.Tests #4980
Merged
jonathanpeppers
merged 1 commit into
dotnet:master
from
jonathanpeppers:dotnet-multi-target-msbuild-tests
Aug 18, 2020
Merged
[tests, build] multi-target Xamarin.Android.Build.Tests #4980
jonathanpeppers
merged 1 commit into
dotnet:master
from
jonathanpeppers:dotnet-multi-target-msbuild-tests
Aug 18, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonathanpeppers
force-pushed
the
dotnet-multi-target-msbuild-tests
branch
16 times, most recently
from
August 12, 2020 20:41
05f5ff1
to
2365849
Compare
jonathanpeppers
force-pushed
the
dotnet-multi-target-msbuild-tests
branch
2 times, most recently
from
August 14, 2020 16:13
a3979d5
to
f1481ab
Compare
Here is a build with a full run of the |
pjcollins
reviewed
Aug 17, 2020
There are various tests in `Xamarin.Android.Build.Tests` that execute MSBuild tasks directly. This means the changes in dotnet#4820 are not quite complete. We need to actually *run* the tests under .NET Core to see if these tests would pass under a One .NET context. The solution (you would think), would be to make the test assembly `netstandard2.0` so you could run either: > nunit3-console Xamarin.Android.Build.Tests.dll > dotnet test Xamarin.Android.Build.Tests.dll `nunit3-console` would execute the tests under .NET framework/Mono and `dotnet test` would run under .NET Core or .NET 5+. Unfortunately, `nunit3-console` can't run tests from a `netstandard2.0` library! So we multi-target: <TargetFrameworks>net472;net5.0</TargetFrameworks> Unfortunately, the `net5.0` causes MSBuild to fail with: > msbuild Xamarin.Android.sln ... (GetReferenceAssemblyPaths target) -> C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Current\Bin\Microsoft.Common.CurrentVersion.targets(1177,5): error MSB3644: The reference assemblies for .NETFramework,Version=v5.0 were not found. To resolve this, install the Developer Pack (SDK/Targeting Pack) for this framework version or retarget your application. You can download .NET Framework Developer Packs at https://aka.ms/msbuild/developerpacks I think there is some expectation to use `dotnet build` for .NET 5 instead. We don't want to move our *entire* build over to `dotnet build` yet (we probably can't!), so instead we can do: <TargetFrameworks>net472;netcoreapp3.1</TargetFrameworks> This allows `msbuild Xamarin.Android.sln` to continue to work. This might also be something that gets fixed in a future MSBuild version. I updated various `.yaml` definitions so we can run NUnit tests via `nunit3-console` or `dotnet test` as needed. Other things that broke here: * We need `Microsoft.NET.Test.Sdk` for `dotnet test` to work. * `Builder.UseDotNet` can now be set based on a `#if` instead of an NUnit parameter. * `Xamarin.Android.Tools.Aidl` needs to be `netstandard2.0`. This required including a `System.CodeDom` package. * `Xamarin.Android.Build.Tasks` can't use `FSharp.Core` in `netcoreapp3.1`. I just removed it, since it is not used. * Updated various paths to `Xamarin.Android.Build.Tests.dll`. * Usage of System.Drawing crashed on macOS. I replaced this with ImageSharp: https://www.nuget.org/packages/SixLabors.ImageSharp/
jonathanpeppers
force-pushed
the
dotnet-multi-target-msbuild-tests
branch
from
August 17, 2020 19:05
f1481ab
to
5e4eb0e
Compare
jonathanpeppers
commented
Aug 17, 2020
@@ -270,9 +270,6 @@ | |||
BeforeTargets="CoreCompile" | |||
Inputs="@(_SharedRuntimeAssemblies)" | |||
Outputs="$(_GeneratedProfileClass)"> | |||
<ItemGroup> | |||
<_SharedRuntimeAssemblies Remove="$(_SharedRuntimeBuildPath)v1.0\FSharp.Core.dll" /> | |||
</ItemGroup> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked src\Xamarin.Android.Build.Tasks\obj\Debug\Profile.g.cs
and FSharp.Core.dll
isn't being written to it at all, after this was removed:
using System.Collections.Generic;
namespace Xamarin.Android.Tasks {
public partial class Profile {
// KEEP THIS SORTED ALPHABETICALLY, CASE-INSENSITIVE
public static readonly string [] SharedRuntimeAssemblies = new []{
"I18N.CJK.dll",
"I18N.dll",
"I18N.MidEast.dll",
"I18N.Other.dll",
"I18N.Rare.dll",
"I18N.West.dll",
"Java.Interop.dll",
"Microsoft.CSharp.dll",
"Mono.Android.dll",
"Mono.Android.Export.dll",
"Mono.Btls.Interface.dll",
"Mono.CompilerServices.SymbolWriter.dll",
"Mono.CSharp.dll",
"Mono.Data.Sqlite.dll",
"Mono.Data.Tds.dll",
"Mono.Posix.dll",
"Mono.Security.dll",
"mscorlib.dll",
"OpenTK.dll",
"OpenTK-1.0.dll",
"System.ComponentModel.Composition.dll",
"System.ComponentModel.DataAnnotations.dll",
"System.Core.dll",
"System.Data.DataSetExtensions.dll",
"System.Data.dll",
"System.Data.Services.Client.dll",
"System.dll",
"System.EnterpriseServices.dll",
"System.IdentityModel.dll",
"System.IO.Compression.dll",
"System.IO.Compression.FileSystem.dll",
"System.Json.dll",
"System.Net.dll",
"System.Net.Http.dll",
"System.Net.Http.WinHttpHandler.dll",
"System.Numerics.dll",
"System.Numerics.Vectors.dll",
"System.Reflection.Context.dll",
"System.Runtime.CompilerServices.Unsafe.dll",
"System.Runtime.Serialization.dll",
"System.Security.dll",
"System.ServiceModel.dll",
"System.ServiceModel.Internals.dll",
"System.ServiceModel.Web.dll",
"System.Transactions.dll",
"System.Web.Services.dll",
"System.Windows.dll",
"System.Xml.dll",
"System.Xml.Linq.dll",
"System.Xml.Serialization.dll",
};
}
}
pjcollins
approved these changes
Aug 17, 2020
dellis1972
approved these changes
Aug 18, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are various tests in
Xamarin.Android.Build.Tests
that executeMSBuild tasks directly. This means the changes in #4820 are not quite
complete. We need to actually run the tests under .NET Core to see
if these tests would pass under a One .NET context.
The solution (you would think), would be to make the test assembly
netstandard2.0
so you could run either:nunit3-console
would execute the tests under .NET framework/Mono anddotnet test
would run under .NET Core or .NET 5+.Unfortunately,
nunit3-console
can't run tests from anetstandard2.0
library!So we multi-target:
Unfortunately, the
net5.0
causes MSBuild to fail with:I think there is some expectation to use
dotnet build
for .NET 5instead. We don't want to move our entire build over to
dotnet build
yet (we probably can't!), so instead we can do:This allows
msbuild Xamarin.Android.sln
to continue to work. Thismight also be something that gets fixed in a future MSBuild version.
I updated various
.yaml
definitions so we can run NUnit tests vianunit3-console
ordotnet test
as needed.Other things that broke here:
Microsoft.NET.Test.Sdk
fordotnet test
to work.Builder.UseDotNet
can now be set based on a#if
instead of anNUnit parameter.
Xamarin.Android.Tools.Aidl
needs to benetstandard2.0
. Thisrequired including a
System.CodeDom
package.Xamarin.Android.Build.Tasks
can't useFSharp.Core
innetcoreapp3.1
. I just removed it, since it is not used.Xamarin.Android.Build.Tests.dll
.