Updated nuget packages #1256
Updated nuget packages #1256
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
AtariDreams
force-pushed
the
Update
branch
4 times, most recently
from
6b06498
to
41128ee
Compare
|
Ready! @manodasanW |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Thanks @AtariDreams for bringing all these packages to updated consistent versions. I think the Microsoft.WinUI and Microsoft.ProjectReunion version updates come with some breaking changes that may make them need to be addressed separately. But let's see how the build goes. We have plans on updating those packages to the WinAppSDK package which also comes with our changes we will need to do to get it working. |
The last couple errors look related to what I previously mentioned, but the first 2 look unrelated. |
|
@AtariDreams I have PR #1261 out which addresses moving our uses of the old WinUI and ProjectReunion packages to the WinAppSDK package and addresses the breaking changes from that. Once that is in, I think you should be able to rebase your branch with those changes and then your remaining changes for the other NuGet packages should build without issues. |
|
@manodasanW Fixed! |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
AtariDreams
force-pushed
the
Update
branch
2 times, most recently
from
2004909
to
1444141
Compare
|
@manodasanW Sorry, could you run that one more time, there was a build error I fixed. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
|
@manodasanW Done, ready, and it builds! |
|
|
Looks like the build is still hitting the error from the previous comment at this line. I looked into it a bit and it might be the result of updating the CppWinRT package from the much older version we were on. It seems like |
|
How old? 😜 @sylveon added C++20 range support here (microsoft/cppwinrt#900) and may have affected this. Generally ADL should be used to pick up the |
|
Looks like we need to also special case the WinUI 3 IBindableIterable and IBindableIterator interfaces, right now we only do it for system XAML: https://github.com/microsoft/cppwinrt/blob/6ce4fa91bb98763c099e24a744b3234f6b23727c/cppwinrt/code_writers.h#L1456 Should be fairly trivial to do... ctrl-c ctrl-v and replace Windows for Microsoft. |
|
Why would that be necessary? We've only ever supported the |
|
Because |
|
Sure, but cppwinrt does not provide iterator support for an open-ended list of collections. Only those based on |
|
This could be fixed at the WinUI 3 level by having IBindableIterable do so, and should make the range-for and everything else just light up, but I assume the API is kind of frozen at this point (so much for being decoupled) |
|
Oh, I see. Prior to your change this worked with any "collection" with a |
|
I had to remove them because otherwise using std::begin;
begin(someWinrtType);Would give conflicting overload errors, and I could not get ranges to work with cppwinrt's ADL begin... We could either say that not using IIterable is not supported here and hope WinUI 3 adapts or apply the same special case to the WinUI 3 IBindableIterable than we do to system XAML (which doesn't seem too bad, it's fairly simple). I would vote for the former because it follows precedent (IE not supporting WinUI 3 DispatcherQueue after they broke the API on us once) but could also whip up a quick PR of the latter. |
|
Note that just adding those two methods back wouldn't work in some cases (e.g. trying to use IBindableIterator as a normal iterator wouldn't work). |
|
Right, this was never supported. It just happened to work by coincidence. Since these types don't derive from |
|
@manodasanW you can trivially work around this by providing your own inline auto begin(IBindableIterable const& collection) -> decltype(collection.First())
{
auto result = collection.First();
if (!result.HasCurrent())
{
return {};
}
return result;
}
inline auto end([[maybe_unused]] IBindableIterable const& collection) noexcept -> decltype(collection.First())
{
return {};
} |
|
This is the part where using IBindableIterator as a normal iterator won't work. |
|
Though if you're okay with reaching into winrt::impl, you can directly make it return fast_iterator and should be okay |
|
Right, that's where Xaml needs to fix their API if they want this to work. |
|
Thanks @kennykerr and @sylveon. @AtariDreams looking at the test, I think to unblock this PR you can remove the for loop from here as we already test the other functions like |
Many of these packages are old beta packages that have long been superseded