[15.2] Build netstandard.dll and System.Drawing.Primitives.dll facades #569

akoeplinger · 2017-04-21T14:26:59Z

The old project for System.Drawing.Primitives was removed in favor of building the assembly with the existing logic in Mono.

This is a backport of the change I did in the pending mono-2017-04 PR since I was told we want netstandard.dll in 15.2.

…e using (dotnet#567) The updated reference assemblies in Mono 5.0 don't depend on `Microsoft.Build.Engine.dll` anymore, and that assembly contained the `Microsoft.Build.BuildEngine` namespace. Consequently, `ProjectBuilder.cs` doesn't build when using the updated Mono 5.0 reference assemblies. Remove the `using Microsoft.Build.BuildEngine` declaration, as it is unnecessary. (cherry-picked from 634ba1d)

The old project for System.Drawing.Primitives was removed in favor of building the assembly with the existing logic in Mono.

jonpryor · 2017-04-21T14:38:42Z

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/ProjectBuilder.cs

 using System.IO;
 using System.Diagnostics;
 using Microsoft.Build.Framework;
-using Microsoft.Build.BuildEngine;


This change shouldn't be necessary; it's in PR #567.

@jonpryor that was against master, this one is against d15-2.

jonpryor · 2017-04-21T14:46:29Z

src/System.Drawing.Primitives/System.Drawing.Primitives.targets

+<Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Target Name="_BuildSystemDrawingPrimitivesFacade">
+   <MSBuild Projects="..\Mono.Android\Mono.Android.csproj"
+             Targets="GetTargetPath"


Indentation conventions for .csproj/.targets files:

Spaces, not tabs. (Not that you have tabs...)

2-space "tab stops".

If attributes are on a new line, then they should be indented an "extra" tab stop, so that they're indented more than child content.

<Output/> should be on a single line.

Applying the above:

<MSBuild Projects="..\Mono.Android\Mono.Android.csproj" Targets="GetTargetPath" Properties="Configuration=$(Configuration)"> <Output TaskParameter="TargetOutputs" ItemName="MonoAndroidAssembly" /> </MSBuild>

jonpryor · 2017-04-21T14:46:52Z

src/System.Drawing.Primitives/System.Drawing.Primitives.targets

+    </MSBuild>
+    <Exec
+        Command="make -C $(MonoSourceFullPath)\mcs\class\Facades\System.Drawing.Primitives PROFILE=monodroid EXTRA_LIB_MCS_FLAGS=&quot;-r:%(MonoAndroidAssembly.Identity)&quot;"
+        WorkingDirectory="$(MonoSourceFullPath)"


I don't think that WorkingDirectory is required, as make -C will also change the working directory as well. Thus, this seems redundant.

jonpryor · 2017-04-21T14:47:22Z

src/System.Drawing.Primitives/System.Drawing.Primitives.targets

+    <Delete Files="$(OutputPath)\System.Drawing.Primitives.dll" />
+    <Exec
+        Command="make -C $(MonoSourceFullPath)\mcs\class\Facades\System.Drawing.Primitives PROFILE=monodroid clean"
+        WorkingDirectory="$(MonoSourceFullPath)"


jonpryor · 2017-04-21T14:48:06Z

src/netstandard/netstandard.targets

@@ -0,0 +1,27 @@
+<Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Target Name="_BuildNetstandardFacade">
+   <MSBuild Projects="..\Mono.Android\Mono.Android.csproj"


Please see the System.Drawing.Primitives.targets comments and apply here as well.

akoeplinger · 2017-04-21T18:14:29Z

Closing in favor of #572

Context: dotnet/java-interop@8f30933 Changes: dotnet/java-interop@3226a4b...8f30933 * dotnet/java-interop@8f30933: [generator] Mark some Obsolete fields as errors (#568) * dotnet/java-interop@47201c4: [generator] Change protected members in final class to private (#569) * dotnet/java-interop@c5815fb: [generator] Be smarter about when members with same names need "new" (#567) * dotnet/java-interop@bfc0273: [generator] Ensure property setter parameter name is "value" (#566) This Java.Interop bump includes two changes which "break" API: * Certain fields are changed from `[Obsolete]` to `[Obsolete(error:true)]`, turning *use* of the field within C# code into a CS0619 error. * The C# compiler has long warned about the presence of `protected` members within `sealed` types, as they cannot be used. However, they were still emitted, resulting in CS0628 warnings when compiling `Mono.Android.dll`. We consider changing `[Obsolete]` to `[Obsolete(error:true)]` to be acceptable because this is being done to `const` fields, so this won't break ABI, as there are no IL references to `const` fields, and because (1) there are "replacement" members which can be used, and (2) these fields have been obsolete for *years*. The "problem" here is that `Microsoft.DotNet.ApiCompat.exe`, which the `<CheckApiCompatibility/>` task uses from `src/Mono.Android/Mono.Android.targets`, sees every such change as an API break. This in and of itself is plausibly sensible, but the error message *itself* is bananas, e.g.: namespace Android.AccessibilityServices { public abstract partial class AccessibilityService : Android.App.Service { [Register ("GESTURE_SWIPE_DOWN", ApiSince = 16)] [Obsolete ("This constant will be removed in the future version. Use Android.AccessibilityServices.AccessibilityGesture enum directly instead of this field.", error: true)] public const Android.AccessibilityServices.AccessibilityGesture GestureSwipeDown = (Android.AccessibilityServices.AccessibilityGesture) 2; } } results in: CannotRemoveAttribute : Attribute 'System.ObsoleteAttribute' exists on 'Android.AccessibilityServices.AccessibilityGesture Android.AccessibilityServices.AccessibilityService.GestureSwipeDown' in the contract but not the implementation. The message implies that `[Obsolete]` was removed, which isn't strictly true! `[Obsolete]` is still there! It's just that the constructor parameters have changed. This results in a ginormous change to `tests/api-compatibility/acceptable-breakages-vReference.txt`. The "`protected` members within `sealed` types" fix for CS0628 also resulted in errors from the Tests > API Compatibility job, which runs via `external/xamarin-android-api-compatibility`. Given that commit 07e7477 was intended to *replace* the Tests > API Compatibility job, *remove* the `external/xamarin-android-api-compatibility` submodule reference. We will no longer be using that submodule for API compat checks. Co-authored-by: Jonathan Pobst <monkey@jpobst.com>

Context: dotnet/java-interop@8f30933 Changes: dotnet/java-interop@423e27f...fefe0ca * dotnet/java-interop@fefe0ca: [generator] Mark some Obsolete fields as errors (#568) * dotnet/java-interop@d969a0c: [generator] Change protected members in final class to private (#569) * dotnet/java-interop@3c4accf: [generator] Be smarter about when members with same names need "new" (#567) * dotnet/java-interop@41b87d0: [generator] Ensure property setter parameter name is "value" (#566) This Java.Interop bump includes two changes which "break" API: * Certain fields are changed from `[Obsolete]` to `[Obsolete(error:true)]`, turning *use* of the field within C# code into a CS0619 error. * The C# compiler has long warned about the presence of `protected` members within `sealed` types, as they cannot be used. However, they were still emitted, resulting in CS0628 warnings when compiling `Mono.Android.dll`. We consider changing `[Obsolete]` to `[Obsolete(error:true)]` to be acceptable because this is being done to `const` fields, so this won't break ABI, as there are no IL references to `const` fields, and because (1) there are "replacement" members which can be used, and (2) these fields have been obsolete for *years*. The "problem" here is that `Microsoft.DotNet.ApiCompat.exe`, which the `<CheckApiCompatibility/>` task uses from `src/Mono.Android/Mono.Android.targets`, sees every such change as an API break. This in and of itself is plausibly sensible, but the error message *itself* is bananas, e.g.: namespace Android.AccessibilityServices { public abstract partial class AccessibilityService : Android.App.Service { [Register ("GESTURE_SWIPE_DOWN", ApiSince = 16)] [Obsolete ("This constant will be removed in the future version. Use Android.AccessibilityServices.AccessibilityGesture enum directly instead of this field.", error: true)] public const Android.AccessibilityServices.AccessibilityGesture GestureSwipeDown = (Android.AccessibilityServices.AccessibilityGesture) 2; } } results in: CannotRemoveAttribute : Attribute 'System.ObsoleteAttribute' exists on 'Android.AccessibilityServices.AccessibilityGesture Android.AccessibilityServices.AccessibilityService.GestureSwipeDown' in the contract but not the implementation. The message implies that `[Obsolete]` was removed, which isn't strictly true! `[Obsolete]` is still there! It's just that the constructor parameters have changed. This results in a ginormous change to `tests/api-compatibility/acceptable-breakages-vReference.txt`. The "`protected` members within `sealed` types" fix for CS0628 also resulted in errors from the Tests > API Compatibility job, which runs via `external/xamarin-android-api-compatibility`. Given that commit 07e7477 was intended to *replace* the Tests > API Compatibility job, *remove* the `external/xamarin-android-api-compatibility` submodule reference. We will no longer be using that submodule for API compat checks. Co-authored-by: Jonathan Pobst <monkey@jpobst.com>

akoeplinger added 2 commits April 21, 2017 16:21

Build netstandard.dll and System.Drawing.Primitives.dll facades

7a26a18

The old project for System.Drawing.Primitives was removed in favor of building the assembly with the existing logic in Mono.

dnfclas added the cla-already-signed label Apr 21, 2017

jonpryor reviewed Apr 21, 2017

View reviewed changes

akoeplinger closed this Apr 21, 2017

akoeplinger deleted the d15-2-netstandard branch April 21, 2017 18:14

github-actions bot locked and limited conversation to collaborators Feb 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[15.2] Build netstandard.dll and System.Drawing.Primitives.dll facades #569

[15.2] Build netstandard.dll and System.Drawing.Primitives.dll facades #569

Uh oh!

akoeplinger commented Apr 21, 2017

Uh oh!

jonpryor Apr 21, 2017

Uh oh!

akoeplinger Apr 21, 2017

Uh oh!

jonpryor Apr 21, 2017

Uh oh!

jonpryor Apr 21, 2017

Uh oh!

jonpryor Apr 21, 2017

Uh oh!

jonpryor Apr 21, 2017

Uh oh!

akoeplinger commented Apr 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[15.2] Build netstandard.dll and System.Drawing.Primitives.dll facades #569

[15.2] Build netstandard.dll and System.Drawing.Primitives.dll facades #569

Uh oh!

Conversation

akoeplinger commented Apr 21, 2017

Uh oh!

jonpryor Apr 21, 2017

Choose a reason for hiding this comment

Uh oh!

akoeplinger Apr 21, 2017

Choose a reason for hiding this comment

Uh oh!

jonpryor Apr 21, 2017

Choose a reason for hiding this comment

Uh oh!

jonpryor Apr 21, 2017

Choose a reason for hiding this comment

Uh oh!

jonpryor Apr 21, 2017

Choose a reason for hiding this comment

Uh oh!

jonpryor Apr 21, 2017

Choose a reason for hiding this comment

Uh oh!

akoeplinger commented Apr 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants