microsoft / XamlBehaviors

This is the official home for UWP XAML Behaviors on GitHub.
MIT License
697 stars 112 forks source link

Added ARM64 support. #190

Closed azchohfi closed 4 years ago

azchohfi commented 4 years ago

I've executed the sample app (XAMLBehaviorsSampleCpp) on a Surface Pro X and it worked fine.

azchohfi commented 4 years ago

Build is failing since it is trying to build with SDK 10.0.10586.0, which is very old. Would it make sense to update it, or is the PR process outdated somehow?

mgoertz-msft commented 4 years ago

I think as long as the TPMV remains 10.0.10240.0 it should be safe to target a newer version. If that's still not possible for ARM64 the we may need conditional TPV/TPMV just for ARM64.

azchohfi commented 4 years ago

I didn't have to change the TPMV of the library itself, only from the C++ sample app and the unit tests. I've also just noticed that the tests are not passing now...

Changing the test's target version is not the problem, but changing the TPMV to 16299 crashes a few tests. Switching them back to 10240 makes them pass.

All the tests that fails have a stack similar to this:

 VectorChanged_ActionChangedToNonAction_ExceptionThrown
   Source: ActionCollectionTest.cs line 26
   Duration: 312 ms

  Message: 
    Assert.ThrowsException failed. Threw exception Exception, but exception COMException was expected. 
    Exception Message: ResourceMap Not Found.

    ResourceMap Not Found.

    Stack Trace:    at System.Runtime.InteropServices.WindowsRuntime.IVector`1.SetAt(UInt32 index, T value)
       at System.Runtime.InteropServices.WindowsRuntime.VectorToListAdapter.SetAt[T](IVector`1 _this, UInt32 index, T value)
       at BehaviorsXamlSdkUnitTests.ActionCollectionTest.<>c__DisplayClass1_0.<VectorChanged_ActionChangedToNonAction_ExceptionThrown>b__0()
       at Microsoft.VisualStudio.TestTools.UnitTesting.Assert.<>c__DisplayClass75_0`1.<ThrowsException>b__0()
       at Microsoft.VisualStudio.TestTools.UnitTesting.Assert.ThrowsException[T](Action action, String message, Object[] parameters)
  Stack Trace: 
    ActionCollectionTest.VectorChanged_ActionChangedToNonAction_ExceptionThrown()

This is probably some code path that is different if the TPMV is newer, leading to the crash. The line that is throwing is this one: https://github.com/microsoft/XamlBehaviors/blob/c5df1887c9097438a3bcd0a2a2d65d1e28f56978/src/BehaviorsSDKNative/Microsoft.Xaml.Interactivity/ResourceHelper.h#L11

azchohfi commented 4 years ago
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Microsoft\VC\v160\Microsoft.Cpp.WindowsSDK.targets(46,5): error MSB8036: The Windows SDK version 10.0.10586.0 was not found. Install the required version of Windows SDK or change the SDK version in the project property pages or by right-clicking the solution and selecting "Retarget solution". [C:\projects\xamlbehaviors-10vjw\src\BehaviorsSDKNative\Microsoft.Xaml.Interactivity\Microsoft.Xaml.Interactivity.vcxproj]
  Microsoft.Xaml.Interactions.DesignTools -> C:\projects\xamlbehaviors-10vjw\out\BehaviorsSDKNative\bin\AnyCPU\Debug\Microsoft.Xaml.Interactions.DesignTools.dll
  Microsoft.Xaml.Interactivity.DesignTools -> C:\projects\xamlbehaviors-10vjw\out\BehaviorsSDKNative\bin\AnyCPU\Debug\Microsoft.Xaml.Interactivity.DesignTools.dll
Command exited with code 1

Yeah, the build server is now missing the older SDK.

azchohfi commented 4 years ago

@DVaughan @mgoertz-msft any idea on how to fix the build issue?

DVaughan commented 4 years ago

@DVaughan @mgoertz-msft any idea on how to fix the build issue?

From the failure log it appears Appveyor is using a VS 2019 image and according to this, 10.0.10586 isn't installed for that image. Perhaps if you add an appveyor.yml file with something like:

image:
  - Visual Studio 2017
azchohfi commented 4 years ago

@DVaughan Does it make sense to build with such an old SDK? Shouldn't we just update the code to use something newer? I don't think this SDK is even supported anymore.

DVaughan commented 4 years ago

@DVaughan Does it make sense to build with such an old SDK? Shouldn't we just update the code to use something newer? I don't think this SDK is even supported anymore.

I thought you were encountering other issues when updating the SDK. Yes, by all means use a newer SDK.

azchohfi commented 4 years ago

@DVaughan There you go!