Include Microsoft.WindowsDesktop.App and host folder in DLL search path#37
Include Microsoft.WindowsDesktop.App and host folder in DLL search path#37breyed wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the .NET Core CLR host to support Windows Desktop applications and improve assembly loading paths. The changes enable FoxPro to use Windows Forms and WPF by adding Microsoft.WindowsDesktop.App to the assembly search path, while also including the host DLL's directory for more convenient deployment.
- Added Microsoft.WindowsDesktop.App framework path to TPA list for Windows Forms/WPF support
- Included the host DLL directory in assembly search paths for easier deployment
- Refactored path building code for better maintainability
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| std::string tpaList; | ||
| std::string desktopPath = ReplaceAll(runtimePath, "Microsoft.NETCore.App", "Microsoft.WindowsDesktop.App"); | ||
| BuildTpaList(desktopPath.c_str(), ".dll", tpaList); // Desktop must come first so that its assemblies take precedence over .NET Core assemblies when the same assembly is present in different forms in both runtimes (e.g. WindowsBase.dll). |
There was a problem hiding this comment.
Adding the desktop path without verifying it exists could cause BuildTpaList to process an invalid directory. Consider checking if the desktop path exists before calling BuildTpaList to avoid potential issues when only .NET Core is installed.
| BuildTpaList(desktopPath.c_str(), ".dll", tpaList); // Desktop must come first so that its assemblies take precedence over .NET Core assemblies when the same assembly is present in different forms in both runtimes (e.g. WindowsBase.dll). | |
| DWORD desktopPathAttr = GetFileAttributesA(desktopPath.c_str()); | |
| if (desktopPathAttr != INVALID_FILE_ATTRIBUTES && (desktopPathAttr & FILE_ATTRIBUTE_DIRECTORY)) { | |
| BuildTpaList(desktopPath.c_str(), ".dll", tpaList); // Desktop must come first so that its assemblies take precedence over .NET Core assemblies when the same assembly is present in different forms in both runtimes (e.g. WindowsBase.dll). | |
| } |
| char path[MAX_PATH]; | ||
| GetModuleFileNameA(hModule, path, MAX_PATH); |
There was a problem hiding this comment.
Using a fixed-size buffer with GetModuleFileNameA without checking the return value could lead to truncated paths on systems with very long path names. Consider checking the return value and using GetModuleFileNameA's return value to verify the path wasn't truncated.
| char path[MAX_PATH]; | |
| GetModuleFileNameA(hModule, path, MAX_PATH); | |
| DWORD length = GetModuleFileNameA(hModule, path, MAX_PATH); | |
| // If the function fails or truncates, return empty string | |
| if (length == 0 || length == MAX_PATH) { | |
| return ""; | |
| } |
| appPaths.append(FS_SEPARATOR); | ||
| appPaths.append("bin"); | ||
|
|
||
| appPaths.append((const char *)curDir).append(FS_SEPARATOR).append("bin"); |
There was a problem hiding this comment.
[nitpick] The method chaining on a single line makes the code harder to read and debug. Consider breaking this into separate append calls or using a more readable string building approach.
| appPaths.append((const char *)curDir).append(FS_SEPARATOR).append("bin"); | |
| appPaths.append((const char *)curDir); | |
| appPaths.append(FS_SEPARATOR); | |
| appPaths.append("bin"); |
For .NET (Core and 6+), I added paths in
CoreClrHost.cppforMicrosoft.WindowsDesktop.Appto supplement the existing paths toMicrosoft.NETCore.App. This allows FoxPro to use Windows Forms and WPF. If only core is installed (no desktop), the extra path element will be ignored.I also included the directory of
ClrHost.dllto the .NET assembly search path. This allows users to conveniently package all their DLLs (includingClrHost.dllandwwDotNetBridge.dll) in a single folder.