-
-
Notifications
You must be signed in to change notification settings - Fork 39
Include Microsoft.WindowsDesktop.App and host folder in DLL search path #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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). | |
} |
Copilot uses AI. Check for mistakes.
char path[MAX_PATH]; | ||
GetModuleFileNameA(hModule, path, MAX_PATH); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 ""; | |
} |
Copilot uses AI. Check for mistakes.
appPaths.append(FS_SEPARATOR); | ||
appPaths.append("bin"); | ||
|
||
appPaths.append((const char *)curDir).append(FS_SEPARATOR).append("bin"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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"); |
Copilot uses AI. Check for mistakes.
For .NET (Core and 6+), I added paths in
CoreClrHost.cpp
forMicrosoft.WindowsDesktop.App
to 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.dll
to the .NET assembly search path. This allows users to conveniently package all their DLLs (includingClrHost.dll
andwwDotNetBridge.dll
) in a single folder.