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

Netstandard Support #846

Merged
merged 5 commits into from
Nov 27, 2017
Merged

Netstandard Support #846

merged 5 commits into from
Nov 27, 2017

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Sep 12, 2017

This adds Netstandard support to our msbuild unit test system. You can now create a new DotNetStandard project and add that as a reference. The builder also has a new property RequiresMSBuild which is used to force the system to use msbuild for its build process.

@dellis1972
Copy link
Contributor Author

Requires #850 in order for the test to pass

@dellis1972 dellis1972 changed the title Netstandard Support [WIP] Netstandard Support Sep 14, 2017
@jonpryor
Copy link
Member

Xamarin.Android.Build.Tests.PackagingTest.NetStandardReferenceTest fails. :-(

@dellis1972
Copy link
Contributor Author

@jonpryor seem my comment "Requires #850 in order for the test to pass" :) I was 100% expecting the test to fail.

@jonpryor
Copy link
Member

PR #850 has been merged. Please rebase. :-)

@dellis1972
Copy link
Contributor Author

Rebased :)

@jonpryor
Copy link
Member

@dellis1972: and...it still fails.

:-(

@dellis1972
Copy link
Contributor Author

sigh.. ok updated

@dellis1972
Copy link
Contributor Author

build

@dellis1972
Copy link
Contributor Author

build

@dellis1972
Copy link
Contributor Author

Finally!

@jonpryor
Copy link
Member

PR #873 has been merged. Let's rebase this PR and see if it works. :-)

@dellis1972 dellis1972 force-pushed the netstandard2 branch 2 times, most recently from bfe36a5 to 79fcaf5 Compare September 29, 2017 13:33
@dellis1972
Copy link
Contributor Author

build

2 similar comments
@dellis1972
Copy link
Contributor Author

build

@jonpryor
Copy link
Member

build

@dellis1972 dellis1972 force-pushed the netstandard2 branch 3 times, most recently from eea1370 to 6610391 Compare November 21, 2017 14:09
@dellis1972 dellis1972 removed the do-not-merge PR should not be merged. label Nov 21, 2017
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

The only issue I'm seeing is this new test doesn't work on Windows, it fails with:

"C:\Users\jopepper\Desktop\Git\xamarin-android\bin\TestDebug\temp\NetStandardReferenceTest\XamFormsSample\XamFormsSample.csproj" (Restore target) (1) ->
  C:\Users\jopepper\Desktop\Git\xamarin-android\bin\TestDebug\temp\NetStandardReferenceTest\XamFormsSample\XamFormsSample.csproj : error MSB4057: The target "Restore" does not exist in the project.

    0 Warning(s)
    1 Error(s)

I think it may be something to do with using the full framework version of MSBuild as they mention here.

So that means there is yet another msbuild? The .NET Core one?

@dellis1972
Copy link
Contributor Author

@jonathanpeppers was that issue on windows when you build from a VS2017 Command Prompt?
We normally use nuget restore but that does not seem to work for net standard properly.

This adds Netstandard support to our msbuild unit test system.
You can now create a new `DotNetStandard` project and add that
as a reference. The builder also has a new property `RequiresMSBuild`
which is used to force the system to use `msbuild` for its build
process.
@jonathanpeppers
Copy link
Member

@dellis1972 you could also try dotnet restore, but I need to try to get xabuild.exe /t:Restore to work next week. Maybe I can figure something out.

@@ -317,6 +341,8 @@ protected bool BuildInternal (string projectOrSolution, string target, string []

if (nativeCrashDetected) {
Console.WriteLine ($"Native crash detected! Running the build for {projectOrSolution} again.");
File.Move(processLog, processLog + ".bak");
File.Delete(processLog);
Copy link
Member

Choose a reason for hiding this comment

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

How will this actually delete anything? The previous line ensures that processLog will no longer exist, unless the directory that processLog is in doesn't allow renaming...in which case, what are the odds that the File.Delete() will work either?

@jonpryor jonpryor merged commit c80472e into dotnet:master Nov 27, 2017
jonpryor pushed a commit that referenced this pull request Jul 21, 2021
Fixes: dotnet/java-interop#854

Changes: dotnet/installer@e8b3b6b...9c46371

    % git diff --shortstat e8b3b6be...9c463710
     103 files changed, 2301 insertions(+), 2757 deletions(-)

Changes: dotnet/linker@a07cab7...460dd6d

    % git diff --shortstat a07cab7b...460dd6dd
     84 files changed, 2403 insertions(+), 258 deletions(-)

Changes: dotnet/runtime@02f70d0...96ce6b3

    % git diff --shortstat 02f70d0b90...96ce6b3535
     2586 files changed, 123677 insertions(+), 34433 deletions(-)

Changes: dotnet/java-interop@a5ed891...4fb7c14

  * dotnet/java-interop@4fb7c147: [build] set $(DisableImplicitNamespaceImports) by default (#859)
  * dotnet/java-interop@855ecfa3: [generator] Don't generate unexpected NRT types like `void?` (#856)
  * dotnet/java-interop@4a02bc32: Revert "[Xamarin.Android.Tools.Bytecode] hide nested types (#827)" (#855)
  * dotnet/java-interop@95c9b79d: [generator] Avoid 'error (…):' construct in diagnostic messages (#851)
  * dotnet/java-interop@7c4f7db0: [build] Bump to Mono with MSBuild 16.10 (#848)
  * dotnet/java-interop@0227cdae: [generator] Gracefully handle BindingGeneratorException. (#845)
  * dotnet/java-interop@ce1750fd: Add SECURITY.md (#846)

Context: dotnet/runtime#55384
Context: dotnet/sdk#18639

Updates:

  * Microsoft.Dotnet.Sdk.Internal: from 6.0.100-preview.7.21327.2 to 6.0.100-rc.1.21369.3
  * Microsoft.NET.ILLink.Tasks: from 6.0.100-preview.6.21322.1 to 6.0.100-preview.6.21366.2
  * Microsoft.NETCore.App.Ref: from 6.0.0-preview.7.21326.8 to 6.0.0-rc.1.21368.1

dotnet/runtime#55384 broke how .NET 6 interacts with
`AndroidClientHandler`.  Fix this by introducing a new
`Xamarin.Android.Net.AndroidMessageHandler` type for use on .NET 6,
and update the .NET 6 `AndroidClientHandler` to delegate to
`AndroidMessageHandler`.

`AndroidMessageHandler` doesn't exist on Legacy.

Update `.apkdesc` files:

  * `BuildReleaseArm64SimpleDotNet` is ~37KB smaller
  * `BuildReleaseArm64XFormsDotNet` is ~62KB larger.

Update `tests/api-compatibility/api-compat-exclude-attributes.txt`
so that `T:System.Runtime.CompilerServices.CompilerGeneratedAttribute`
is ignored.  `[CompilerGeneratedAttribute]` is emitted as part of
C# 3 "auto-props":

	public T Property { get; set; }

Converting this into a "real" property:

	T value;
	public T Property {
	    get => value;
	    set => this.value = value;
	}

results in the `_CheckApiCompatibility` target complaining about an
API break.  We don't care; ignore `[CompilerGeneratedAttribute]`.

Remove `$(SelfContained)` property: early on in .NET 5 (yes 5)
development, the Xamarin.Android SDK needed to specify
`$(SelfContained)` by default in order to produce `.apk` files.

After we became a proper workload, setting the value became
unnecessary.  It also didn't actually do anything because
dotnet/sdk overwrote the value.

Starting in dotnet/sdk#18639,
`Microsoft.NET.RuntimeIdentifierInference.targets` is now being
imported *after* a workload, which meant that dotnet/sdk no longer
overwrote our `$(SelfContained)` value, which broke things:

	error NETSDK1031: It is not supported to build or publish a self-contained application without specifying a RuntimeIdentifier. You must either specify a RuntimeIdentifier or set SelfContained to false.

We can simply remove `$(SelfContained)` now, and rely on
dotnet/sdk to set this value.

Co-authored-by: Jonathan Peppers <[email protected]>
Co-authored-by: Steve Pfister <[email protected]>
jonpryor pushed a commit that referenced this pull request Jul 21, 2021
Fixes: dotnet/java-interop#854

Changes: dotnet/installer@e8b3b6b...808795c

    % git diff --shortstat e8b3b6be...808795cc
     102 files changed, 2218 insertions(+), 2674 deletions(-)

Changes: dotnet/linker@a07cab7...9ecf5bd

    % git diff --shortstat a07cab7b...9ecf5bd2
     81 files changed, 2122 insertions(+), 246 deletions(-)


Changes: dotnet/runtime@02f70d0...8d3afa3

    % git diff --shortstat 02f70d0b...8d3afa3a
     2518 files changed, 122843 insertions(+), 33676 deletions(-)

Changes: dotnet/java-interop@a5ed891...4fb7c14

  * dotnet/java-interop@4fb7c147: [build] set $(DisableImplicitNamespaceImports) by default (#859)
  * dotnet/java-interop@855ecfa3: [generator] Don't generate unexpected NRT types like `void?` (#856)
  * dotnet/java-interop@4a02bc32: Revert "[Xamarin.Android.Tools.Bytecode] hide nested types (#827)" (#855)
  * dotnet/java-interop@95c9b79d: [generator] Avoid 'error (…):' construct in diagnostic messages (#851)
  * dotnet/java-interop@7c4f7db0: [build] Bump to Mono with MSBuild 16.10 (#848)
  * dotnet/java-interop@0227cdae: [generator] Gracefully handle BindingGeneratorException. (#845)
  * dotnet/java-interop@ce1750fd: Add SECURITY.md (#846)

Context: dotnet/runtime#55384
Context: dotnet/sdk#18639

Updates:

  * Microsoft.Dotnet.Sdk.Internal: from 6.0.100-preview.7.21327.2 to 6.0.100-preview.7.21369.5
  * Microsoft.NET.ILLink.Tasks: from 6.0.100-preview.6.21322.1 to 6.0.100-preview.6.21365.1
  * Microsoft.NETCore.App.Ref: from 6.0.0-preview.7.21326.8 to 6.0.0-preview.7.21368.2

dotnet/runtime#55384 broke how .NET 6 interacts with
`AndroidClientHandler`.  Fix this by introducing a new
`Xamarin.Android.Net.AndroidMessageHandler` type for use on .NET 6,
and update the .NET 6 `AndroidClientHandler` to delegate to
`AndroidMessageHandler`.

`AndroidMessageHandler` doesn't exist on Legacy.

Update `.apkdesc` files:

  * `BuildReleaseArm64SimpleDotNet` is ~37KB smaller
  * `BuildReleaseArm64XFormsDotNet` is ~62KB larger.

Update `tests/api-compatibility/api-compat-exclude-attributes.txt`
so that `T:System.Runtime.CompilerServices.CompilerGeneratedAttribute`
is ignored.  `[CompilerGeneratedAttribute]` is emitted as part of
C# 3 "auto-props":

	public T Property { get; set; }

Converting this into a "real" property:

	T value;
	public T Property {
	    get => value;
	    set => this.value = value;
	}

results in the `_CheckApiCompatibility` target complaining about an
API break.  We don't care; ignore `[CompilerGeneratedAttribute]`.

Remove `$(SelfContained)` property: early on in .NET 5 (yes 5)
development, the Xamarin.Android SDK needed to specify
`$(SelfContained)` by default in order to produce `.apk` files.

After we became a proper workload, setting the value became
unnecessary.  It also didn't actually do anything because
dotnet/sdk overwrote the value.

Starting in dotnet/sdk#18639,
`Microsoft.NET.RuntimeIdentifierInference.targets` is now being
imported *after* a workload, which meant that dotnet/sdk no longer
overwrote our `$(SelfContained)` value, which broke things:

	error NETSDK1031: It is not supported to build or publish a self-contained application without specifying a RuntimeIdentifier. You must either specify a RuntimeIdentifier or set SelfContained to false.

We can simply remove `$(SelfContained)` now, and rely on
dotnet/sdk to set this value.

Co-authored-by: Jonathan Peppers <[email protected]>
Co-authored-by: Steve Pfister <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants