Implement FormsStack and FormsQueue internally for VS 2017 Compatibility #8403

PureWeen · 2019-11-06T19:05:38Z

Description of Change

Update everything back to using VS 2019 build components
Copy in Stack and Queue implementations from corefx so that the moved types in mono don't break VS2017
- I just linked the files into the platform projects so they can all stay internal/sealed. I assume once we totally drop VS 2017 we'll just delete all of this and roll back to using the packaged versions of these

Here are some comparison tests ran through Benchmark dotnet

Method	Mean	Error	StdDev
PushQueue	27.780 ms	0.5488 ms	1.3148 ms
PopQueue	5.082 ms	0.0992 ms	0.0928 ms
PushFQueue	26.596 ms	0.5269 ms	0.8046 ms
PopFQueue	2.397 ms	0.0453 ms	0.0378 ms

Method	Mean	Error	StdDev	Median
PushStack	28.324 ms	1.0790 ms	3.0433 ms	26.999 ms
PopStack	4.148 ms	0.0827 ms	0.0813 ms	4.125 ms
PushFStack	26.596 ms	0.6836 ms	1.2670 ms	26.244 ms
PopFStack	2.086 ms	0.0454 ms	0.0819 ms	2.053 ms

Here's the test project if you would like to verify or add additional tests

TestCollections.zip

Issues Resolved

fixes Investigate removing/changing all uses of Stack<T> and Queue<T> #8126

Platforms Affected

Core/XAML (all platforms)
iOS
Android
UWP

Testing Procedure

pull down the nuget and make sure you can build and deploy projects on VS 2017

PR Checklist

Targets the correct branch
Tests are passing (or failures are unrelated)

rmarinho · 2019-11-06T19:14:10Z

Can we have 2 pr's ? one just for the Stack and Queue and other with the other changes?

Also we should format the corefx code with our guidelines, drop private etc..

PureWeen · 2019-11-06T19:56:45Z

Can we have 2 pr's ? one just for the Stack and Queue and other with the other changes?

@rmarinho the whole of this PR is to create a nuget that's built on VS 2019 that we can test works on VS 2017

If we split out all the changes we can't really do that

Also we should format the corefx code with our guidelines, drop private etc..

yea I wasn't sure. This isn't really code that we are going to iterate or own we're just going to delete it sometime next year so it didn't seem worth the effort to format according to all our guidelines

samhouts · 2019-11-07T16:52:17Z

Let's leave the code style the same as corefx, just in case we need to pull in an update.

PureWeen · 2019-11-07T17:05:54Z

I tested the nuget with the sample given here and it now runs with these changes

#7602 (comment)

Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj

…csproj

rmarinho

Can we add this per parts? first the code changes, that would not need vs2019.
Then we add changes to cake and provisioning.

rmarinho · 2019-11-11T15:28:14Z

azure-pipelines.yml

+- name: DOTNET_VERSION
+  value: 3.0.100
+- name: winVmImage
+  value: Hosted Windows 2019 with VS2019


This will keeps us from updating let's say for XamarinForms (private agents) i prefer we had this as settable from the pipeline

Yea once we are happy with the PR I was going to delete this before merging it.

Though I still think this should be part of the yml file not the pipeline. For example if branch 4.1.0 is set to VS 2017 than when you run those builds it'll just build and work. Then when you run 4.2.0 it'll just build and work. If it's settable at the pipeline level than you are required to have knowledge external to the repository in order to make something compile.

But we can discuss that more on Thursday :-)

we can delete i m happy with it :P Let's merge

Sounds good!! I want to run some quick tests on UWP to see if incrementing the SDK build will break the icon on MDP even more or if the issue is the same.

@rmarinho I added these back in for the time being because FormsQueue and FormsStack uses C# features that need 2019. I don't really want to modify them so they compile with C# 7.0 I'd rather just leave them untouched.

So I figure once this is merged in and the build server pipeline variables are updated then we can remove the fields?

Xamarin.Forms.Core\corefx\FormsQueue.cs(263,19): Error CS8107: Feature 'default literal' is not available in C# 7.0. Please use language version 7.1 or greater. Xamarin.Forms.Core\corefx\FormsQueue.cs(278,14): Error CS8107: Feature 'default literal' is not available in C# 7.0. Please use language version 7.1 or greater. Xamarin.Forms.Core\corefx\FormsQueue.cs(285,19): Error CS8107: Feature 'default literal' is not available in C# 7.0. Please use language version 7.1 or greater. Xamarin.Forms.Core\corefx\FormsStack.cs(344,23): Error CS8107: Feature 'default literal' is not available in C# 7.0. Please use language version 7.1 or greater. Xamarin.Forms.Core\corefx\FormsStack.cs(374,24): Error CS8107: Feature 'default literal' is not available in C# 7.0. Please use language version 7.1 or greater. Xamarin.Forms.Core\corefx\FormsStack.cs(404,23): Error CS8107: Feature 'default literal' is not available in C# 7.0. Please use language version 7.1 or greater. Xamarin.Forms.Core\corefx\FormsQueue.cs(263,19): Error CS8107: Feature 'default literal' is not available in C# 7.0. Please use language version 7.1 or greater. Xamarin.Forms.Core\corefx\FormsQueue.cs(278,14): Error CS8107: Feature 'default literal' is not available in C# 7.0. Please use language version 7.1 or greater. Xamarin.Forms.Core\corefx\FormsQueue.cs(285,19): Error CS8107: Feature 'default literal' is not available in C# 7.0. Please use language version 7.1 or greater. Xamarin.Forms.Core\corefx\FormsStack.cs(344,23): Error CS8107: Feature 'default literal' is not available in C# 7.0. Please use language version 7.1 or greater.

Xamarin.Forms.Platform.UAP/Xamarin.Forms.Platform.UAP.csproj

PureWeen · 2019-11-16T21:07:12Z

build.cake

@@ -43,42 +44,48 @@ var informationalVersion = gitVersion.InformationalVersion;
 var buildVersion = gitVersion.FullBuildMetaData;
 var nugetversion = Argument<string>("packageVersion", gitVersion.NuGetVersion);

-var ANDROID_HOME = EnvironmentVariable ("ANDROID_HOME") ?? 


@rmarinho all these cake changes just make this cake script consistent with master

azure-pipelines.yml

MagicAndre1981 · 2019-11-18T17:19:55Z

what will be the first build that includes the fix, so that I can test it with VS2017?

PureWeen · 2019-11-18T17:22:59Z

@MagicAndre1981
It should be up with the nightly in a couple of nights otherwise you can download the nuget from this PR here

https://dev.azure.com/xamarin/public/_build/results?buildId=10002&view=artifacts

and test it out

MagicAndre1981 · 2019-11-19T08:44:46Z

@PureWeen

I tested the version. In my current working copy the nuget package fails first with XF001 "Xamarin.Forms targets have been imported multiple times. Please check your project file and remove the duplicate import(s)." Trying to set <XFDisableTargetsValidation>true</XFDisableTargetsValidation> causes error that I need to reference MonoAndroid90 🤔 🤷‍♂️

So I made a new checkout from my SVN source server and here the 4.2.0.910325 works with VS2017, the Android app compiles and runs fine so far.

PureWeen requested a review from rmarinho November 6, 2019 19:05

PureWeen requested review from jsuarezruiz and StephaneDelcroix as code owners November 6, 2019 19:05

samhouts added ControlGallery p/iOS 🍎 p/Android p/UWP p/macOS p/gtk a/Xaml </> proposal-accepted t/enhancement ➕ labels Nov 6, 2019

samhouts assigned StephaneDelcroix Nov 7, 2019

PureWeen force-pushed the vs_2017_compat branch from 0680ccb to cba02a5 Compare November 7, 2019 17:13

PureWeen added 4 commits November 7, 2019 10:16

FormsStack and FormsQueue for VS 2017 Compat

ea0ecbc

- sync stable vs mac versions

d789878

- mono version

a53ba42

- gtk

5ac353f

PureWeen force-pushed the vs_2017_compat branch from cba02a5 to 5ac353f Compare November 7, 2019 17:18

PureWeen changed the base branch from master to 4.2.0 November 7, 2019 17:18

- fix mono provision

3cb32cc

PureWeen force-pushed the vs_2017_compat branch from 36e0805 to 3cb32cc Compare November 8, 2019 00:26

PureWeen commented Nov 8, 2019

View reviewed changes

Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj Outdated Show resolved Hide resolved

Update Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.…

5137fcf

…csproj

rmarinho reviewed Nov 11, 2019

View reviewed changes

PureWeen assigned rmarinho Nov 13, 2019

PureWeen removed the request for review from jsuarezruiz November 13, 2019 18:17

rmarinho approved these changes Nov 15, 2019

View reviewed changes

samhouts reviewed Nov 15, 2019

View reviewed changes

Xamarin.Forms.Platform.UAP/Xamarin.Forms.Platform.UAP.csproj Outdated Show resolved Hide resolved

StephaneDelcroix approved these changes Nov 15, 2019

View reviewed changes

- remove uwp changes

6ea5b3b

samhouts added the Core label Nov 16, 2019

- add 2019 back in

06e35dc

PureWeen commented Nov 16, 2019

View reviewed changes

rmarinho reviewed Nov 18, 2019

View reviewed changes

azure-pipelines.yml Outdated Show resolved Hide resolved

Update azure-pipelines.yml

0f105ab

PureWeen merged commit 34c346c into 4.2.0 Nov 18, 2019

PureWeen deleted the vs_2017_compat branch November 18, 2019 17:10

samhouts added the approved Has two approvals, no pending reviews, and no changes requested label Nov 18, 2019

samhouts added this to the 4.3.0 milestone Nov 26, 2019

samhouts added this to Done in v4.3.0 Nov 26, 2019

felipebaltazar mentioned this pull request Dec 1, 2019

Fix issue 6878 | [Bug] ShellItem.Items.Clear() crashes when the ShellItem has bottom tabs #8215

Merged

2 tasks

samhouts mentioned this pull request Dec 6, 2019

Investigate removing/changing all uses of Stack<T> and Queue<T> #8126

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement FormsStack and FormsQueue internally for VS 2017 Compatibility #8403

Implement FormsStack and FormsQueue internally for VS 2017 Compatibility #8403

PureWeen commented Nov 6, 2019 •

edited

rmarinho commented Nov 6, 2019

PureWeen commented Nov 6, 2019 •

edited

samhouts commented Nov 7, 2019

PureWeen commented Nov 7, 2019

rmarinho left a comment

rmarinho Nov 11, 2019

PureWeen Nov 11, 2019

rmarinho Nov 15, 2019

PureWeen Nov 15, 2019

PureWeen Nov 16, 2019

PureWeen Nov 16, 2019

MagicAndre1981 commented Nov 18, 2019

PureWeen commented Nov 18, 2019

MagicAndre1981 commented Nov 19, 2019

Implement FormsStack and FormsQueue internally for VS 2017 Compatibility #8403

Implement FormsStack and FormsQueue internally for VS 2017 Compatibility #8403

Conversation

PureWeen commented Nov 6, 2019 • edited

Description of Change

Issues Resolved

Platforms Affected

Testing Procedure

PR Checklist

rmarinho commented Nov 6, 2019

PureWeen commented Nov 6, 2019 • edited

samhouts commented Nov 7, 2019

PureWeen commented Nov 7, 2019

rmarinho left a comment

Choose a reason for hiding this comment

rmarinho Nov 11, 2019

Choose a reason for hiding this comment

PureWeen Nov 11, 2019

Choose a reason for hiding this comment

rmarinho Nov 15, 2019

Choose a reason for hiding this comment

PureWeen Nov 15, 2019

Choose a reason for hiding this comment

PureWeen Nov 16, 2019

Choose a reason for hiding this comment

PureWeen Nov 16, 2019

Choose a reason for hiding this comment

MagicAndre1981 commented Nov 18, 2019

PureWeen commented Nov 18, 2019

MagicAndre1981 commented Nov 19, 2019

PureWeen commented Nov 6, 2019 •

edited

PureWeen commented Nov 6, 2019 •

edited