starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-03 21:25:34 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

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

New Issue
Closed
opened 2026-01-30 22:24:38 +00:00 by claunia · 9 comments
claunia commented 2026-01-30 22:24:38 +00:00
Owner
Copy Link

Originally created by @zadjii-msft on GitHub (May 28, 2019).

Originally assigned to: @zadjii-msft on GitHub.

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:

  1. 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 #946.
  2. 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.
Originally created by @zadjii-msft on GitHub (May 28, 2019). Originally assigned to: @zadjii-msft on GitHub. 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: 1. 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 #946. 2. 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.
claunia commented 2026-01-30 22:24:39 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (May 28, 2019):

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

@zadjii-msft commented on GitHub (May 28, 2019): @DHowett-MSFT I'm pulling the triage tag on this one since we've discussed this in chat.
claunia commented 2026-01-30 22:24:40 +00:00
Author
Owner
Copy Link

@DHowett-MSFT commented on GitHub (May 28, 2019):

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.

@DHowett-MSFT commented on GitHub (May 28, 2019): 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.
claunia commented 2026-01-30 22:24:40 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (May 28, 2019):

Also @Scottj1s because he might be curious.

@zadjii-msft commented on GitHub (May 28, 2019): Also @Scottj1s because he might be curious.
claunia commented 2026-01-30 22:24:40 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (May 29, 2019):

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 commented on GitHub (May 29, 2019): 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.
claunia commented 2026-01-30 22:24:41 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (May 29, 2019):

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.

@zadjii-msft commented on GitHub (May 29, 2019): 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.
claunia commented 2026-01-30 22:24:41 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (May 29, 2019):

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 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.
  1. 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.
  2. 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.
  3. 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.
  4. 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
  5. 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 commented on GitHub (May 29, 2019): 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 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. ``` 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. 6061168 is my progress up until this point.
claunia commented 2026-01-30 22:24:41 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (May 29, 2019):

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 commented on GitHub (May 29, 2019): 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 👍
claunia commented 2026-01-30 22:24:41 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Jun 3, 2019):

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 commented on GitHub (Jun 3, 2019): 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.
claunia commented 2026-01-30 22:24:42 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Jun 6, 2019):

Okay, after a LOT of finagling, looks like I've finally got the tests all working.
image
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.

@zadjii-msft commented on GitHub (Jun 6, 2019): Okay, after a LOT of finagling, looks like I've finally got the tests all working. ![image](https://user-images.githubusercontent.com/18356694/59051774-4999f380-8853-11e9-873d-9f2f22df8bbc.png) 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.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label Area-Build Issue-Task Needs-Tag-Fix Product-Terminal Resolution-Fix-Committed
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#1394
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?