PowerToys icon indicating copy to clipboard operation
PowerToys copied to clipboard

CmdPal: Get rid of all of our dependencies

Open zadjii-msft opened this issue 8 months ago • 20 comments

I almost definitely don't need the Targets I added to the two vcxproj files

I'm just here to spin a CI to test using the UCRT

zadjii-msft avatar Apr 03 '25 18:04 zadjii-msft

The latest status is about the powertoys.interop.dll also depends on the vcrt, as the result it failed with exception when launch cmdpal: Exception Info: System.Runtime.InteropServices.COMException (0x80040154): Class not registered (0x80040154 (REGDB_E_CLASSNOTREG)) at System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(Int32 errorCode) at WinRT.ActivationFactory.Get(String typeName, Guid iid) at PowerToys.Interop.Constants.get__objRef_global__PowerToys_Interop_IConstantsStatics() at ManagedCommon.Logger.InitializeLogger(String applicationLogPath, Boolean isLocalLow) at Microsoft.CmdPal.UI.Program.Main(String[] args)

yeelam-gordon avatar Apr 09 '25 04:04 yeelam-gordon

@zadjii-msft , given @vanzue (Kai) already help a lot on this setup problem, do you mind to let Kai to take over this PR for completion?

yeelam-gordon avatar Apr 10 '25 01:04 yeelam-gordon

HAPPY to hand this off. I just pushed 6426fe5, which is where I was going with this.

I think basically every c++ project that CmdPal ends up ingesting is gonna need to use the hybrid CRT, so that none of those dependencies fail to load because it's missing.

I didn't have a chance to test that commit, and I'm guessing I only half moved things over to using the HybridCrt.*.props.

Lemme know if you have any questions!

(I'm also just gonna low-key /cc @dhowett, cause he did the hybrid CRT stuff for Terminal, which is where I'm cribbing this from)

zadjii-msft avatar Apr 10 '25 11:04 zadjii-msft

Looks like "terminal.UI" and "commandpalette.extensions" have already been correctly configured hybrid crt, what we left is: AdaptiveCards & microsoft.interop.dll.

vanzue avatar Apr 18 '25 03:04 vanzue

AdaptiveCards.Rendering.WinUI3.dll, the dependency is gone, due to recent release. I'll update the package.

Only 1 left is AdaptiveCards.ObjectModel.WinUI3. The project has recently been transitioned to another team, and I'm asking whether they can do hybrid crt for us.

Meanwhile, I'll do testing against our new installer with no reference to vclib.

vanzue avatar Apr 22 '25 00:04 vanzue

Launch without crash without vclib, but functionalities are not available, just keep spinning: image It just keeps spinning.

I think what we left is AdaptiveCards.ObjectModel.WinUI3.

vanzue avatar Apr 23 '25 06:04 vanzue

@yeelam-gordon @crutkas @zadjii-msft The pr can't be checked in before "AdaptiveCards.ObjectModel.WinUI3" is hybrid crt configred. I gave a test based on current situation, can't launch commandpal without dependency to vcrt.

vanzue avatar Apr 27 '25 03:04 vanzue

Faulting application name: Microsoft.CmdPal.UI.exe, version: 0.0.1.0, time stamp: 0x67890000 Faulting module name: Microsoft.UI.Xaml.dll, version: 3.1.7.0, time stamp: 0xda76db6a Exception code: 0xc000027b Fault offset: 0x000000000000a320 Faulting process id: 0x1398 Faulting application start time: 0x1DBB81161406CAC Faulting application path: C:\Program Files\WindowsApps\Microsoft.CommandPalette.Dev_0.0.1.0_arm64__8wekyb3d8bbwe\Microsoft.CmdPal.UI.exe Faulting module path: C:\Program Files\WindowsApps\Microsoft.CommandPalette.Dev_0.0.1.0_arm64__8wekyb3d8bbwe\Microsoft.UI.Xaml.dll Report Id: 97327aff-ecc2-47da-b19e-f736eeed1a04 Faulting package full name: Microsoft.CommandPalette.Dev_0.0.1.0_arm64__8wekyb3d8bbwe Faulting package-relative application ID: App

And exception in dump: at System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(Int32 errorCode) at WinRT.ActivationFactory.Get(String typeName) at CmdPalKeyboardService.KeyboardListener.get__objRef_global__CmdPalKeyboardService_KeyboardListener() in C:\PowerToys\src\modules\cmdpal\Microsoft.CmdPal.UI\obj\ARM64\Debug\Generated Files\CsWinRT\CmdPalKeyboardService.cs:line 67 at CmdPalKeyboardService.KeyboardListener..ctor() in C:\PowerToys\src\modules\cmdpal\Microsoft.CmdPal.UI\obj\ARM64\Debug\Generated Files\CsWinRT\CmdPalKeyboardService.cs:line 74 at Microsoft.CmdPal.UI.MainWindow..ctor() in C:\PowerToys\src\modules\cmdpal\Microsoft.CmdPal.UI\MainWindow.xaml.cs:line 73 at Microsoft.CmdPal.UI.App.OnLaunched(LaunchActivatedEventArgs args) in C:\PowerToys\src\modules\cmdpal\Microsoft.CmdPal.UI\App.xaml.cs:line 78 at Microsoft.UI.Xaml.Application.Microsoft.UI.Xaml.IApplicationOverrides.OnLaunched(LaunchActivatedEventArgs args) at ABI.Microsoft.UI.Xaml.IApplicationOverrides.Do_Abi_OnLaunched_0(IntPtr thisPtr, IntPtr args)

vanzue avatar Apr 28 '25 08:04 vanzue

Well that makes sense. Looks like we need the hybridcrt props to the keyboard service project too (which was merged after this PR was started)

zadjii-msft avatar Apr 28 '25 13:04 zadjii-msft

Tried to get rid of keyboard service error, while we encounter this when running in sandbox without dependency to vcruntime: 000001504b3ff6a0 00007ffd960dc63c combase+0x12c63c 000001504b3ff6a8 00007ffd5e2bb204 CoreMessagingXP!Microsoft::UI::Dispatching::DispatcherQueue::DeferInvokeCallback+0x74 000001504b3ff6b0 00007ffd5e263dc8 CoreMessagingXP!CFlat::SehSafe::Execute<<lambda_e7b49cfe4ef82ee5125e2a9b812cfc45> >+0x30 000001504b3ff6b8 00007ffd5e267b60 CoreMessagingXP!Microsoft::CoreUI::ActionCallback::ImportAdapter$+0x60 000001504b3ff6c0 00007ffd5e26de00 CoreMessagingXP!Microsoft::CoreUI::Messaging::MessageSession::Callback_InvokeDeferInvoke+0x68 000001504b3ff6c8 00007ffd5e23cf54 CoreMessagingXP!Microsoft::CoreUI::Dispatch::DeferredCall::Callback_Dispatch+0x134 000001504b3ff6d0 00007ffd5e27c1e0 CoreMessagingXP!Microsoft::CoreUI::Dispatch::DeferredCallDispatcher::Callback_OnDispatch+0x140 000001504b3ff6d8 00007ffd5e23e51c CoreMessagingXP!Microsoft::CoreUI::Dispatch::Dispatcher::Callback_DispatchNextItem+0x1fc 000001504b3ff6e0 00007ffd5e23e24c CoreMessagingXP!Microsoft::CoreUI::Dispatch::Dispatcher::Callback_DispatchLoop+0x1c4

Still, if we state our dependency to vcrt, everything will be fine. I'm not sure whether it's related to that adaptivecard dll. But it's the only one left by dependency analysis.

vanzue avatar Apr 29 '25 11:04 vanzue

Do you still believe this is the fix to shell error? I'm not sure we should cover this in .91.

vanzue avatar Apr 29 '25 11:04 vanzue

TLDR;

  • Even we remove the dependency to vclib manually, our projects still have dependency to that dll. We'v not fully hybrid contained.
  • Only 1 left is Adaptive cards
  • Interesting thing is that, the same package can be run in our normal env(my dev machine), and that is because in "c:\windows\system32" we do have the vcruntime140.dll etc - Although not sure whether they are stable and the same with our packaged vclib ones, Cmdpal can be run.

I think the other thing we can do is that we analyze which module in hell introduce the vcruntime.dll dependency, adaptive card, sure, but I'm not sure whether it should cause app to crash right after start up. Maybe something else.

Latest crash dump exception I got is from "CoreMessagingXP.dll", trying to construct a winrt object, while not sure which object initialization causes that.

vanzue avatar Apr 29 '25 14:04 vanzue

@vanzue if you run under a debugger you can use Global Flags to turn on "loader snaps". It will print 10,000 lines of debugging information about every DLL that was loaded, every entrypoint that was bound, and every dependency that fails to load.

https://learn.microsoft.com/en-us/windows-hardware/drivers/debugger/gflags

https://learn.microsoft.com/en-us/windows-hardware/drivers/debugger/show-loader-snaps

DHowett avatar Apr 29 '25 23:04 DHowett

@DHowett I tried the method you suggested and confirmed that the crash was caused by adaptivecards.objectmodel.winui3 failing to load vcruntime140.dll and msvcp140.dll. As an experiment, I copied these two DLLs from my host machine into the sandbox's Windows/System32 directory, and after that, Command Palette was able to start normally.

vanzue avatar May 06 '25 09:05 vanzue

Finally make app run without fully self-contain with last commit While adaptive cards rely on vclib explictly, we can run command palette without that

vanzue avatar May 06 '25 12:05 vanzue

@DHowett I tried the method you suggested and confirmed that the crash was caused by adaptivecards.objectmodel.winui3 failing to load vcruntime140.dll and msvcp140.dll. As an experiment, I copied these two DLLs from my host machine into the sandbox's Windows/System32 directory, and after that, Command Palette was able to start normally.

Wait isn't that weird?

Wasn't the whole point of asking them (Adaptive Cards) to release a hybrid crt version to fix that exact issue?

I can't find any reference to what changed in AdaptiveCards.ObjectModel.WinUI3 Version="2.0.1-beta either. Did that just bump the dependency version, but not actually make the AdaptiveCards.ObjectModel.WinUI3 library also use the hybrid CRT? it seems like we'd need both, but maybe I'm just missing something here

zadjii-msft avatar May 06 '25 14:05 zadjii-msft

@DHowett I tried the method you suggested and confirmed that the crash was caused by adaptivecards.objectmodel.winui3 failing to load vcruntime140.dll and msvcp140.dll. As an experiment, I copied these two DLLs from my host machine into the sandbox's Windows/System32 directory, and after that, Command Palette was able to start normally.

Wait isn't that weird?

Wasn't the whole point of asking them (Adaptive Cards) to release a hybrid crt version to fix that exact issue?

I can't find any reference to what changed in AdaptiveCards.ObjectModel.WinUI3 Version="2.0.1-beta either. Did that just bump the dependency version, but not actually make the AdaptiveCards.ObjectModel.WinUI3 library also use the hybrid CRT? it seems like we'd need both, but maybe I'm just missing something here

I don't either. I first realized it's adaptive card family causes the dependency, then I found a new adaptivecards.rendering.winui3 was released, and that has done the hybrid-crt. So I gave it a try on the adaptivecards.objectmodel.winui3 upgrade, result is that it still has dependency on vclib..

vanzue avatar May 06 '25 15:05 vanzue

I mean, I found a new version AdaptiveCards.ObjectModel.WinUI3 2.0.1-beta and I tried that, they still have dependency After that I asked the team to do hybrid crt. But the team has not committed yet

vanzue avatar May 06 '25 15:05 vanzue

Ahhhhhh okay that explains it. I assumed that they had already fixed it (which would have been an amazing turnaround).

I dunno if this makes sense to hold until they're fixed upstream. On one hand, this is definitely still good engineering health for us, and it does work for the 99% use case, but forms will magically not work (including settings).

zadjii-msft avatar May 06 '25 15:05 zadjii-msft

Agree, while I'm not sure whether it must be in 91, Maybe we can hold until 91 released? I think most pcs now have vcruntime in c:\windows\system32, in most cases we should be fine even we fully self-contain right now.

Or we can still depends on vclib, and this dependency is only for adaptivecards

vanzue avatar May 07 '25 01:05 vanzue

@vanzue, I tried to build CmdPal locally, and run: dumpbin /dependents Microsoft.CmdPal.UI.exe

It doesn't show me the dependent on vcruntime. Is that all good? Or in the past your testing on dumpbin is another dll or exe?

If we don't depends vc runtime already, the last change we should be doing (and testing) is remove the dependency and installation of the vc runtime as part of cmdpal msix

yeelam-gordon avatar Sep 19 '25 03:09 yeelam-gordon

@yeelam-gordon

$targetFolder = "C:\Program Files\WindowsApps\Microsoft.CommandPalette_0.1.1.0_arm64__8wekyb3d8bbwe"

$dllFiles = Get-ChildItem -Path $targetFolder -Filter *.dll -Recurse

foreach ($dll in $dllFiles) {
    $output = dumpbin /nologo /dependents $dll.FullName 2>$null

    if ($output -match "VCRUNTIME140\.dll") {
        Write-Host "$($dll.FullName) depends on VCRUNTIME140.dll"
    }
}

I used this to determine, The exe may not itself depends on vcruntime, but the sub dll may. And this is when build system find the dependency and register a vcruntime msix for us

vanzue avatar Sep 19 '25 08:09 vanzue

Close for now, if we got update from adaptive card, we can do this again.

vanzue avatar Nov 05 '25 07:11 vanzue