Refactor pure C++ types from TerminalApp.dll into a lib #1042

zadjii-msft · 2019-05-28T22:03:57Z

Right now we have one big dll that is declaring both cppwinrt types (e.g. AppKeyBindings, App) and pure c++ types (Tab, Profile, etc.).

If one were to try and create a unittesting project for the TerminalApp project, you unfortunately would be unable to do this, because many of the types are only linked into this TerminalApp.dll. For pure cppwinrt types, this isn't that big of a deal, because instantiating the types just does cppwinrt magic to activate them. However, for pure cpp classes, we can't actually link them. This means we can't test things like json parsing, Pane recursing, settings synthesis.

So we'll have to pull all the types that aren't winrt types out into their own lib, and have the TerminalApp project consume that lib.

I'm not worried about TermControl and TermConnection so much, since those are only using cppwinrt types at the moment. Should they get more elaborate, then it might be necessary to do a similar refactoring.

Worries:

Does this mean we're going to have now two pch.pch files for the TerminalApp, one for the lib and one for the dll? could we avoid that? See Share PCH files #946.
The lib seems to need to include winrt/coroutine.h (and any other cppwinrt headers that components might need). Those are generated files. Right now they're generated by the TerminalApp(dll) build, but theoretically they'll need to be created before that, because the lib will theoretically be built before the dll. This may mean that the lib needs to have some sort of cppwinrt tooling turned on, but I don't know what quite yet.

The text was updated successfully, but these errors were encountered:

zadjii-msft · 2019-05-28T22:04:39Z

@DHowett-MSFT I'm pulling the triage tag on this one since we've discussed this in chat.

DHowett-MSFT · 2019-05-28T22:05:54Z

There should definitely not be two pch files -- the dll should exist solely and exclusively to turn the lib into a dll. There's probably some stupidity regarding making sure every .obj from the lib is actually included in the dll though, because the linker loves to remove things it thinks nobody can see.

…pLib project to make a lib. See #1042

zadjii-msft · 2019-05-28T22:07:23Z

Also @Scottj1s because he might be curious.

zadjii-msft · 2019-05-29T15:09:04Z

Oh there's even more insanity here.

TerminalApp depends on 4 other cppwinrt projects:

TerminalSettings, TerminalControl, and Microsoft.UI.Xaml.Markup (MUX.Markup) are all projects in our solution
Microsoft.UI.Xaml is a project we're including from nuget. It's included in the build as a Reference paired with a ReferenceCopyLocalPaths

If you try to move all the types to the lib, you're going to have a bad day. You're going to need to make the lib project a cppwinrt project, but it'll be ConfigurationType=StaticLibrary. If you make it a cppwinrt project, it's somehow unable to find the winmd's from the projects it references. They're all DynamicLibrary projects, so presumably they'd be found by <Target Name="GetCppWinRTDynamicWinMDReferences" in the cppwinrt targets, but they somehow aren't found at all.

The actual error is:

2>C:\Users\migrie\dev\private\OpenConsole\src\cascadia\TerminalApp\lib\App.idl(9): error MIDL2011: 
[msg]unresolved type declaration [context]: Microsoft.UI.Xaml.Markup.XamlApplication [ RuntimeClass 'TerminalApp.App'  ]

Which suggests it can't find the MUX.Markup winmd. This is true, it can't:

2>Target GetCppWinRTStaticProjectReferences:
2>  Task "Message"
2>    CppWinRTStaticProjectReferences: C:\Users\migrie\dev\private\OpenConsole\bin\x64\Debug\ConTypes.lib
2>  Done executing task "Message".
2>Target GetCppWinRTStaticWinMDReferences:
2>  Task "Message"
2>    CppWinRTStaticWinMDReferences: C:\Users\migrie\dev\private\OpenConsole\bin\x64\Debug\Microsoft.Terminal.TerminalControl.winmd
2>  Done executing task "Message".
2>Target "ResolvedLinkLib" skipped. Previously built successfully.
2>Target GetCppWinRTDynamicProjectReferences:
2>  Task "Message"
2>    CppWinRTDynamicProjectReferences:
2>  Done executing task "Message".
2>Target GetCppWinRTDynamicWinMDReferences:
2>  Task "Message"
2>    CppWinRTDynamicWinMDReferences:
2>  Done executing task "Message".
2>Target "ResolveAssemblyReferences" skipped. Previously built successfully.
2>Target GetCppWinRTDirectWinMDReferences:
2>  Task "Message"
2>    CppWinRTDirectWinMDReferences: C:\Users\migrie\dev\private\OpenConsole\packages\Microsoft.UI.Xaml.2.1.190405001-prerelease\lib\uap10.0\Microsoft.UI.Xaml.winmd
2>  Done executing task "Message".

Actually more insane, is that the project does seem to find the TerminalControl winmd, but not the TerminalSettings or MUX.Markup ones. If you inspect the midlrt.rsp file, you'll see

/reference "C:\Users\migrie\dev\private\OpenConsole\packages\Microsoft.UI.Xaml.2.1.190405001-prerelease\lib\uap10.0\Microsoft.UI.Xaml.winmd"
/reference "C:\Users\migrie\dev\private\OpenConsole\bin\x64\Debug\Microsoft.Terminal.TerminalControl.winmd"

at the top of the file.

So that's where I'm at with this.

zadjii-msft · 2019-05-29T15:26:00Z

Okay my earlier comment about TerminalControl was misguided - somehow a rouge Microsoft.Terminal.TerminalControl.winmd found its way to my bin/ folder. So it was automagically getting included when it shouldn't have been. This might clear up some of the insanity I was having.

So current state: 1. If you try to add a `Reference` to all of MUX.Markup, TerminalControl and TerminalSettings, then mdmerge will complain about all the types from TerminalSettings being defined twice. In this magic scenario, the dependencies of TerminalControl are used directly for some reason: ``` 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalSettings\Microsoft.Terminal.Settings.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.Settings.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalConnection.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalControl.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\Microsoft.UI.Xaml.Markup\Microsoft.UI.Xaml.Markup.winmd. ``` 2. If you don't add a `Reference` TerminalControl, then it'll complain about being unable to find the type TitleChangedEventArgs, which is defined in TerminalControl. 3. If you don't add a `Reference` TerminalSettings, then it'll complain about being unable to find the type KeyChord and other types from TerminalSettings. In this scenario, it doesn't recurse on the other dependencies from TerminalControl for whatever reason. 4. If you instead try to add all 3 as a `ProjectReference`, then it'll complain about being unable to find TitleChangedEventArgs, as in 2. Presumably, it;ll have troubles with the other types too, as none of the 3 are actually included in the midlrt.rsp file. 5. If you add all 3 as a `ProjectReference`, then also add TerminalControl as a `Reference`, you'll get a `MIDL2011: [msg] unresolved type declaration Microsoft.UI.Xaml.Markup.XamlApplication` 6. If you add all 3 as a `ProjectReference`, then also add TerminalControl AND MUX.Markup as a `Reference`, you'll get the same result as 3.

zadjii-msft · 2019-05-29T16:18:41Z

So current state:

If you try to add a Reference to all of MUX.Markup, TerminalControl and TerminalSettings, then mdmerge will complain about all the types from TerminalSettings being defined twice. In this magic scenario, the dependencies of TerminalControl are used directly for some reason:

12>    Load input metadata file C:\Users\migrie\dev\private\OpenConsole\x64\Debug\TerminalSettings\Microsoft.Terminal.Settings.winmd.
12>    Load input metadata file C:\Users\migrie\dev\private\OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.Settings.winmd.
12>    Load input metadata file C:\Users\migrie\dev\private\OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalConnection.winmd.
12>    Load input metadata file C:\Users\migrie\dev\private\OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalControl.winmd.
12>    Load input metadata file C:\Users\migrie\dev\private\OpenConsole\x64\Debug\Microsoft.UI.Xaml.Markup\Microsoft.UI.Xaml.Markup.winmd.

If you don't add a Reference TerminalControl, then it'll complain about being unable to find the type TitleChangedEventArgs, which is defined in TerminalControl.
If you don't add a Reference TerminalSettings, then it'll complain about being unable to find the type KeyChord and other types from TerminalSettings. In this scenario, it doesn't recurse on the other dependencies from TerminalControl for whatever reason.
If you instead try to add all 3 as a ProjectReference, then it'll complain about being unable to find TitleChangedEventArgs, as in 2. Presumably, it;ll have troubles with the other types too, as none of the 3 are actually included in the midlrt.rsp file.
If you add all 3 as a ProjectReference, then also add TerminalControl as a Reference, you'll get a MIDL2011: [msg]unresolved type declaration Microsoft.UI.Xaml.Markup.XamlApplication
If you add all 3 as a ProjectReference, then also add TerminalControl AND MUX.Markup as a Reference, you'll get the same result as 3.

6061168 is my progress up until this point.

zadjii-msft · 2019-05-29T19:35:08Z

As of fc38e3e, I've got it compiling and running again.

I've got a TerminalAppLib that builds all the classes. This project does all the buisiness except compiling .idl and .xaml files.

I've got the TerminalApp project that builds a dll using that lib. TerminalApp still has a pch, but I'm going to see what from that I can take out. I'm hoping it's most of it.

And finally, we've got UnitTests_TerminalApp that builds against the lib to test json parsing (definitely just a POC now but hey, something!)

It needs a terrible lot of cleanup, but progress 👍

zadjii-msft · 2019-06-03T14:51:16Z

Okay so for updates:

I've made a lot of progress since the previous commit. There was a lot more work that needed doing than what I had in the above commit. Fortunately, it's building now.

Unfortunately, I can't instantiate any xaml-y winrt types in the unittest dll I'm building. So until I can make sure that's working too, I'm going to leave this open and not make a PR for this.

zadjii-msft · 2019-06-06T17:07:36Z

Okay, after a LOT of finagling, looks like I've finally got the tests all working.

There was a long thread internally on how to get WinRT types working with TAEF tests, then on how to get Xaml Islands initialized in a test, then how to get MUX working as well. Fortunately however, they all work now!
As of fabd1cd, the changes are really dirty, so I'm going to add a bunch of comments explaining all the witchcraft-y parts of getting this to work.

@Austin-Lamb

* Refactors TerminalApp into two projects: - TerminalAppLib, which builds a .lib, and includes all the code - TerminalApp, which builds a dll by linking the lib * Adds a TerminalApp.Unit.Tests project - Includes the ability to test cppwinrt types we've authored using a SxS manifest for unpackaged winrt activation - includes the ability to test types with XAML content using an appxmanifest * Adds a giant doc explaining how this was all done. Really, just go read that doc, it'll really help you understand what's going on in this PR. ------------------------- These are some previous commit messages. They may be helpful to future readers. * Start adding unittests for json parsing, end up creating a TerminalAppLib project to make a lib. See #1042 * VS automatically did this for me * This is a dead end I tried including the idl-y things into the lib, but that way leads insanity If you want to make a StaticLibrary, then suddenly the winrt toolchain forgets that ProjectReferences can have winmd's in them, so it won't be able to compile any types from the referenced projects. If you instead try to manually reference the types, you'll get duplicate types up the wazoo, which of course is insane, since we're referencing them the _one_ time * Yea just follow #1042 on github for status So current state: 1. If you try to add a `Reference` to all of MUX.Markup, TerminalControl and TerminalSettings, then mdmerge will complain about all the types from TerminalSettings being defined twice. In this magic scenario, the dependencies of TerminalControl are used directly for some reason: ``` 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalSettings\Microsoft.Terminal.Settings.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.Settings.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalConnection.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalControl.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\Microsoft.UI.Xaml.Markup\Microsoft.UI.Xaml.Markup.winmd. ``` 2. If you don't add a `Reference` TerminalControl, then it'll complain about being unable to find the type TitleChangedEventArgs, which is defined in TerminalControl. 3. If you don't add a `Reference` TerminalSettings, then it'll complain about being unable to find the type KeyChord and other types from TerminalSettings. In this scenario, it doesn't recurse on the other dependencies from TerminalControl for whatever reason. 4. If you instead try to add all 3 as a `ProjectReference`, then it'll complain about being unable to find TitleChangedEventArgs, as in 2. Presumably, it;ll have troubles with the other types too, as none of the 3 are actually included in the midlrt.rsp file. 5. If you add all 3 as a `ProjectReference`, then also add TerminalControl as a `Reference`, you'll get a `MIDL2011: [msg] unresolved type declaration Microsoft.UI.Xaml.Markup.XamlApplication` 6. If you add all 3 as a `ProjectReference`, then also add TerminalControl AND MUX.Markup as a `Reference`, you'll get the same result as 3. * what if we just don't idl This seems to compile * This compiles but I broke the MUX resources look at the App.xaml change. in this changelist. That's what's broken right now. Lets fix that! * lets do this If I leave the MUX nuget out of the project, I'll get a compile error in App.xaml: ``` ...OpenConsole\src\cascadia\TerminalApp\App.xaml(21,40): XamlCompiler error WMC0001: Unknown type 'XamlControlsResources' in XML namespace 'using:Microsoft.UI.Xaml.Controls' ``` If I add it back to the project, it works * Some cleanup from the previous commit * This is busted again. Doing a clean build didn't work. A clean rebuild of the project, paired with some removal of dead code revealed a problem with what I have so far. TerminalAppLib depends on the generation of two headers, `AppKeyBindings.g.h` and `App.g.h`, as those define some of bits of the winrt types. They're needed to be able to compile the implementations. Presumably that's not getting generated by the lib project, because the dll project is the one to generate that file. So we need to move the idl's to the lib project. This created maddness, because of course the Duplicate Type thing. The solution to that is to actually mark the winrt DLLs that we're chaining up through us as ``` <Private>false</Private> <CopyLocalSatelliteAssemblies>false</CopyLocalSatelliteAssemblies> ``` This will prevent them from getting double-included. This still doesn't work however, since ``` app.cpp(40): error C2039: 'XamlMetaDataProvider': is not a member of 'winrt::TerminalApp' error C3861: 'XamlMetaDataProvider': identifier not found ``` So we need to figure that out. The dll project is still generating the right header, so lets look there. * Move the xaml stuff to the lib This compiles, but when we launch, we fail to load the tabviewcontrol resources again. So that's not what you want. Why is it not included? * It works again! * Use the pri, xbf files from TerminalAppLib, not TerminalApp * Manually make TerminalApp include a reference to TerminalAppLib's TerminalApp.winmd. This will force the build to copy TerminalApp.winmd to TerminalApp/, which WindowsTerminal needs to be able to ProjectReference the TerminalApp project (it's expecting it to have a winmd) * Remove the module.g.cpp from TerminalApp, and move to TerminalAppLib. The dll doesn't do any codegen anymore. * Agressively clean up these files * Clean up unnecessary includes in the dll pch.h * This does NOT work. The WindowsxamlManager call crashes. I'm thinking it has to do with activation of winrt types from a dll. Email out to @Austin-Lamb to see if he can assist * This gets our cppwinrt types working, but xaml islands is still broken * Split the tests apart, so they aren't insane * These are the magic words to make xaml islands work * All this witchcraft is necessary to make XAML+MUX work right * Clean this up a bit and add comments * Create an enormous doc explaining this madness * Unsure how this got changed. * Trying to get the CI build to work again. This resolves the MUX issue. We need to manually include it, because their package's target doesn't mark it as CopyLocalSatelliteAssemblies=false, Private=false. However, the TerminalApp project is still able to magically reason that the TerminalAppLib project should be included in the MdMerge step, because it think's it's a `GetCppWinRTStaticProjectReferences` reference. * Update cppwinrt to the latest version - this fixes the MSBuild * I still need to re-add the KeyModifiers checks from TermControl. I think this update broke `operator&` for that enum. * There needs to be some cleanup obviously * The doc should be updated as well * Clean up changes from cppwinrt update * Try doing this, even though it seems wrong * Lets try this (press x to doubt) * Clean up vcxproj file, and remove appxmanifest change from previous commit * Update to the latest TAEF release, maybe that'll work * Let's try a prerelease version, shall we? * Add notes about TAEF package, comment out tests * Format the code * Hopefully fix the arm64 and x86 builds also a typo * Fix PR nits * Fix some bad merge conflicts * Some cleanup from the merge * Well I was close to getting the merge right * I believe this will fix CI * Apply suggestions from code review Co-Authored-By: Carlos Zamora <carlos.zamora@microsoft.com> * These definitely need to be fixed * Try version detecting in the test IDK if this will build, I'm letting the CI try while I clean rebuild locally * Try blindly updating to the newest nuget version * Revert "Try blindly updating to the newest nuget version" This reverts commit b72bd9e. * We're just going to see if these work in CI with this change * Comment the tests back out. Windows Server 2019 is 10.0.17763.557 * Remove the nuget package We don't need this package anymore now that we're hosting it * Okay this _was_ important

@Austin-Lamb

* Refactors TerminalApp into two projects: - TerminalAppLib, which builds a .lib, and includes all the code - TerminalApp, which builds a dll by linking the lib * Adds a TerminalApp.Unit.Tests project - Includes the ability to test cppwinrt types we've authored using a SxS manifest for unpackaged winrt activation - includes the ability to test types with XAML content using an appxmanifest * Adds a giant doc explaining how this was all done. Really, just go read that doc, it'll really help you understand what's going on in this PR. ------------------------- These are some previous commit messages. They may be helpful to future readers. * Start adding unittests for json parsing, end up creating a TerminalAppLib project to make a lib. See microsoft#1042 * VS automatically did this for me * This is a dead end I tried including the idl-y things into the lib, but that way leads insanity If you want to make a StaticLibrary, then suddenly the winrt toolchain forgets that ProjectReferences can have winmd's in them, so it won't be able to compile any types from the referenced projects. If you instead try to manually reference the types, you'll get duplicate types up the wazoo, which of course is insane, since we're referencing them the _one_ time * Yea just follow microsoft#1042 on github for status So current state: 1. If you try to add a `Reference` to all of MUX.Markup, TerminalControl and TerminalSettings, then mdmerge will complain about all the types from TerminalSettings being defined twice. In this magic scenario, the dependencies of TerminalControl are used directly for some reason: ``` 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalSettings\Microsoft.Terminal.Settings.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.Settings.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalConnection.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalControl.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\Microsoft.UI.Xaml.Markup\Microsoft.UI.Xaml.Markup.winmd. ``` 2. If you don't add a `Reference` TerminalControl, then it'll complain about being unable to find the type TitleChangedEventArgs, which is defined in TerminalControl. 3. If you don't add a `Reference` TerminalSettings, then it'll complain about being unable to find the type KeyChord and other types from TerminalSettings. In this scenario, it doesn't recurse on the other dependencies from TerminalControl for whatever reason. 4. If you instead try to add all 3 as a `ProjectReference`, then it'll complain about being unable to find TitleChangedEventArgs, as in 2. Presumably, it;ll have troubles with the other types too, as none of the 3 are actually included in the midlrt.rsp file. 5. If you add all 3 as a `ProjectReference`, then also add TerminalControl as a `Reference`, you'll get a `MIDL2011: [msg] unresolved type declaration Microsoft.UI.Xaml.Markup.XamlApplication` 6. If you add all 3 as a `ProjectReference`, then also add TerminalControl AND MUX.Markup as a `Reference`, you'll get the same result as 3. * what if we just don't idl This seems to compile * This compiles but I broke the MUX resources look at the App.xaml change. in this changelist. That's what's broken right now. Lets fix that! * lets do this If I leave the MUX nuget out of the project, I'll get a compile error in App.xaml: ``` ...OpenConsole\src\cascadia\TerminalApp\App.xaml(21,40): XamlCompiler error WMC0001: Unknown type 'XamlControlsResources' in XML namespace 'using:Microsoft.UI.Xaml.Controls' ``` If I add it back to the project, it works * Some cleanup from the previous commit * This is busted again. Doing a clean build didn't work. A clean rebuild of the project, paired with some removal of dead code revealed a problem with what I have so far. TerminalAppLib depends on the generation of two headers, `AppKeyBindings.g.h` and `App.g.h`, as those define some of bits of the winrt types. They're needed to be able to compile the implementations. Presumably that's not getting generated by the lib project, because the dll project is the one to generate that file. So we need to move the idl's to the lib project. This created maddness, because of course the Duplicate Type thing. The solution to that is to actually mark the winrt DLLs that we're chaining up through us as ``` <Private>false</Private> <CopyLocalSatelliteAssemblies>false</CopyLocalSatelliteAssemblies> ``` This will prevent them from getting double-included. This still doesn't work however, since ``` app.cpp(40): error C2039: 'XamlMetaDataProvider': is not a member of 'winrt::TerminalApp' error C3861: 'XamlMetaDataProvider': identifier not found ``` So we need to figure that out. The dll project is still generating the right header, so lets look there. * Move the xaml stuff to the lib This compiles, but when we launch, we fail to load the tabviewcontrol resources again. So that's not what you want. Why is it not included? * It works again! * Use the pri, xbf files from TerminalAppLib, not TerminalApp * Manually make TerminalApp include a reference to TerminalAppLib's TerminalApp.winmd. This will force the build to copy TerminalApp.winmd to TerminalApp/, which WindowsTerminal needs to be able to ProjectReference the TerminalApp project (it's expecting it to have a winmd) * Remove the module.g.cpp from TerminalApp, and move to TerminalAppLib. The dll doesn't do any codegen anymore. * Agressively clean up these files * Clean up unnecessary includes in the dll pch.h * This does NOT work. The WindowsxamlManager call crashes. I'm thinking it has to do with activation of winrt types from a dll. Email out to @Austin-Lamb to see if he can assist * This gets our cppwinrt types working, but xaml islands is still broken * Split the tests apart, so they aren't insane * These are the magic words to make xaml islands work * All this witchcraft is necessary to make XAML+MUX work right * Clean this up a bit and add comments * Create an enormous doc explaining this madness * Unsure how this got changed. * Trying to get the CI build to work again. This resolves the MUX issue. We need to manually include it, because their package's target doesn't mark it as CopyLocalSatelliteAssemblies=false, Private=false. However, the TerminalApp project is still able to magically reason that the TerminalAppLib project should be included in the MdMerge step, because it think's it's a `GetCppWinRTStaticProjectReferences` reference. * Update cppwinrt to the latest version - this fixes the MSBuild * I still need to re-add the KeyModifiers checks from TermControl. I think this update broke `operator&` for that enum. * There needs to be some cleanup obviously * The doc should be updated as well * Clean up changes from cppwinrt update * Try doing this, even though it seems wrong * Lets try this (press x to doubt) * Clean up vcxproj file, and remove appxmanifest change from previous commit * Update to the latest TAEF release, maybe that'll work * Let's try a prerelease version, shall we? * Add notes about TAEF package, comment out tests * Format the code * Hopefully fix the arm64 and x86 builds also a typo * Fix PR nits * Fix some bad merge conflicts * Some cleanup from the merge * Well I was close to getting the merge right * I believe this will fix CI * Apply suggestions from code review Co-Authored-By: Carlos Zamora <carlos.zamora@microsoft.com> * These definitely need to be fixed * Try version detecting in the test IDK if this will build, I'm letting the CI try while I clean rebuild locally * Try blindly updating to the newest nuget version * Revert "Try blindly updating to the newest nuget version" This reverts commit b72bd9e. * We're just going to see if these work in CI with this change * Comment the tests back out. Windows Server 2019 is 10.0.17763.557 * Remove the nuget package We don't need this package anymore now that we're hosting it * Okay this _was_ important

@Austin-Lamb

* Refactors TerminalApp into two projects: - TerminalAppLib, which builds a .lib, and includes all the code - TerminalApp, which builds a dll by linking the lib * Adds a TerminalApp.Unit.Tests project - Includes the ability to test cppwinrt types we've authored using a SxS manifest for unpackaged winrt activation - includes the ability to test types with XAML content using an appxmanifest * Adds a giant doc explaining how this was all done. Really, just go read that doc, it'll really help you understand what's going on in this PR. ------------------------- These are some previous commit messages. They may be helpful to future readers. * Start adding unittests for json parsing, end up creating a TerminalAppLib project to make a lib. See microsoft#1042 * VS automatically did this for me * This is a dead end I tried including the idl-y things into the lib, but that way leads insanity If you want to make a StaticLibrary, then suddenly the winrt toolchain forgets that ProjectReferences can have winmd's in them, so it won't be able to compile any types from the referenced projects. If you instead try to manually reference the types, you'll get duplicate types up the wazoo, which of course is insane, since we're referencing them the _one_ time * Yea just follow microsoft#1042 on github for status So current state: 1. If you try to add a `Reference` to all of MUX.Markup, TerminalControl and TerminalSettings, then mdmerge will complain about all the types from TerminalSettings being defined twice. In this magic scenario, the dependencies of TerminalControl are used directly for some reason: ``` 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalSettings\Microsoft.Terminal.Settings.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.Settings.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalConnection.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalControl.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\Microsoft.UI.Xaml.Markup\Microsoft.UI.Xaml.Markup.winmd. ``` 2. If you don't add a `Reference` TerminalControl, then it'll complain about being unable to find the type TitleChangedEventArgs, which is defined in TerminalControl. 3. If you don't add a `Reference` TerminalSettings, then it'll complain about being unable to find the type KeyChord and other types from TerminalSettings. In this scenario, it doesn't recurse on the other dependencies from TerminalControl for whatever reason. 4. If you instead try to add all 3 as a `ProjectReference`, then it'll complain about being unable to find TitleChangedEventArgs, as in 2. Presumably, it;ll have troubles with the other types too, as none of the 3 are actually included in the midlrt.rsp file. 5. If you add all 3 as a `ProjectReference`, then also add TerminalControl as a `Reference`, you'll get a `MIDL2011: [msg] unresolved type declaration Microsoft.UI.Xaml.Markup.XamlApplication` 6. If you add all 3 as a `ProjectReference`, then also add TerminalControl AND MUX.Markup as a `Reference`, you'll get the same result as 3. * what if we just don't idl This seems to compile * This compiles but I broke the MUX resources look at the App.xaml change. in this changelist. That's what's broken right now. Lets fix that! * lets do this If I leave the MUX nuget out of the project, I'll get a compile error in App.xaml: ``` ...OpenConsole\src\cascadia\TerminalApp\App.xaml(21,40): XamlCompiler error WMC0001: Unknown type 'XamlControlsResources' in XML namespace 'using:Microsoft.UI.Xaml.Controls' ``` If I add it back to the project, it works * Some cleanup from the previous commit * This is busted again. Doing a clean build didn't work. A clean rebuild of the project, paired with some removal of dead code revealed a problem with what I have so far. TerminalAppLib depends on the generation of two headers, `AppKeyBindings.g.h` and `App.g.h`, as those define some of bits of the winrt types. They're needed to be able to compile the implementations. Presumably that's not getting generated by the lib project, because the dll project is the one to generate that file. So we need to move the idl's to the lib project. This created maddness, because of course the Duplicate Type thing. The solution to that is to actually mark the winrt DLLs that we're chaining up through us as ``` <Private>false</Private> <CopyLocalSatelliteAssemblies>false</CopyLocalSatelliteAssemblies> ``` This will prevent them from getting double-included. This still doesn't work however, since ``` app.cpp(40): error C2039: 'XamlMetaDataProvider': is not a member of 'winrt::TerminalApp' error C3861: 'XamlMetaDataProvider': identifier not found ``` So we need to figure that out. The dll project is still generating the right header, so lets look there. * Move the xaml stuff to the lib This compiles, but when we launch, we fail to load the tabviewcontrol resources again. So that's not what you want. Why is it not included? * It works again! * Use the pri, xbf files from TerminalAppLib, not TerminalApp * Manually make TerminalApp include a reference to TerminalAppLib's TerminalApp.winmd. This will force the build to copy TerminalApp.winmd to TerminalApp/, which WindowsTerminal needs to be able to ProjectReference the TerminalApp project (it's expecting it to have a winmd) * Remove the module.g.cpp from TerminalApp, and move to TerminalAppLib. The dll doesn't do any codegen anymore. * Agressively clean up these files * Clean up unnecessary includes in the dll pch.h * This does NOT work. The WindowsxamlManager call crashes. I'm thinking it has to do with activation of winrt types from a dll. Email out to @Austin-Lamb to see if he can assist * This gets our cppwinrt types working, but xaml islands is still broken * Split the tests apart, so they aren't insane * These are the magic words to make xaml islands work * All this witchcraft is necessary to make XAML+MUX work right * Clean this up a bit and add comments * Create an enormous doc explaining this madness * Unsure how this got changed. * Trying to get the CI build to work again. This resolves the MUX issue. We need to manually include it, because their package's target doesn't mark it as CopyLocalSatelliteAssemblies=false, Private=false. However, the TerminalApp project is still able to magically reason that the TerminalAppLib project should be included in the MdMerge step, because it think's it's a `GetCppWinRTStaticProjectReferences` reference. * Update cppwinrt to the latest version - this fixes the MSBuild * I still need to re-add the KeyModifiers checks from TermControl. I think this update broke `operator&` for that enum. * There needs to be some cleanup obviously * The doc should be updated as well * Clean up changes from cppwinrt update * Try doing this, even though it seems wrong * Lets try this (press x to doubt) * Clean up vcxproj file, and remove appxmanifest change from previous commit * Update to the latest TAEF release, maybe that'll work * Let's try a prerelease version, shall we? * Add notes about TAEF package, comment out tests * Format the code * Hopefully fix the arm64 and x86 builds also a typo * Fix PR nits * Fix some bad merge conflicts * Some cleanup from the merge * Well I was close to getting the merge right * I believe this will fix CI * Apply suggestions from code review Co-Authored-By: Carlos Zamora <carlos.zamora@microsoft.com> * These definitely need to be fixed * Try version detecting in the test IDK if this will build, I'm letting the CI try while I clean rebuild locally * Try blindly updating to the newest nuget version * Revert "Try blindly updating to the newest nuget version" This reverts commit b72bd9e. * We're just going to see if these work in CI with this change * Comment the tests back out. Windows Server 2019 is 10.0.17763.557 * Remove the nuget package We don't need this package anymore now that we're hosting it * Okay this _was_ important

zadjii-msft added Area-Build Issues pertaining to the build system, CI, infrastructure, meta Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. labels May 28, 2019

zadjii-msft added this to the Terminal v1.0 milestone May 28, 2019

zadjii-msft self-assigned this May 28, 2019

ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 28, 2019

zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 28, 2019

zadjii-msft added a commit that referenced this issue May 28, 2019

Start adding unittests for json parsing, end up creating a TerminalAp…

95258fc

…pLib project to make a lib. See #1042

zadjii-msft mentioned this issue May 28, 2019

Switch to jsoncpp as our json library #1005

Merged

5 tasks

zadjii-msft mentioned this issue Jun 3, 2019

Terminal crashes on startup when initial rows on screen (initialRows setting) is set too high #859

Closed

zadjii-msft mentioned this issue Jun 3, 2019

Add a test to cover closing two sibling panes (near) simultaneously #1124

Open

1 task

zadjii-msft mentioned this issue Jun 7, 2019

Refactor TerminalApp and Add Tests for Xaml Content #1164

Merged

3 tasks

zadjii-msft closed this as completed in #1164 Jul 15, 2019

ghost added Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. labels Jul 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor pure C++ types from TerminalApp.dll into a lib #1042

Refactor pure C++ types from TerminalApp.dll into a lib #1042

zadjii-msft commented May 28, 2019

zadjii-msft commented May 28, 2019

DHowett-MSFT commented May 28, 2019

zadjii-msft commented May 28, 2019

zadjii-msft commented May 29, 2019

zadjii-msft commented May 29, 2019

zadjii-msft commented May 29, 2019

zadjii-msft commented May 29, 2019

zadjii-msft commented Jun 3, 2019

zadjii-msft commented Jun 6, 2019

Refactor pure C++ types from TerminalApp.dll into a lib #1042

Refactor pure C++ types from TerminalApp.dll into a lib #1042

Comments

zadjii-msft commented May 28, 2019

zadjii-msft commented May 28, 2019

DHowett-MSFT commented May 28, 2019

zadjii-msft commented May 28, 2019

zadjii-msft commented May 29, 2019

zadjii-msft commented May 29, 2019

zadjii-msft commented May 29, 2019

zadjii-msft commented May 29, 2019

zadjii-msft commented Jun 3, 2019

zadjii-msft commented Jun 6, 2019