-
Notifications
You must be signed in to change notification settings - Fork 48
Add Linux support to .NET language extension #64
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: main
Are you sure you want to change the base?
Conversation
SQL Server on Linux expects a .tar.gz file instead of a .zip when using CREATE EXTERNAL LANGUAGE.
We will use the nethost to locate the system's installed hostfxr, as using a local libhostfxr.so breaks the .NET runtime lookup. nethost contains the logic needed to find the .NET root based on the DOTNET_ROOT environment variable or the install_location configuration file, which we will require for .NET runtime lookup on diverging Linux distributions (e.g. RHEL and Debian packages not installing .NET in the same base directories).
|
@microsoft-github-policy-service agree company="Raincode srl" |
| @@ -0,0 +1,106 @@ | |||
| #!/usr/bin/env pwsh | |||
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.
Why are we using powershell script for linux instead of bash script?
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.
As written in the description, PowerShell was chosen as a scripting language to serve as a potential base for a cross-platform script. If this is not desired, I can rewrite it in bash.
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.
Do you mean this script can be used for both windows/linux now or we should have a follow-up work item for this?
language-extensions/dotnet-core-CSharp/build/linux/build-dotnet-core-CSharp-extension.ps1
Outdated
Show resolved
Hide resolved
language-extensions/dotnet-core-CSharp/build/linux/build-dotnet-core-CSharp-extension.ps1
Show resolved
Hide resolved
language-extensions/dotnet-core-CSharp/build/linux/create-dotnet-core-CSharp-extension-zip.ps1
Outdated
Show resolved
Hide resolved
language-extensions/dotnet-core-CSharp/include/DotnetEnvironment.h
Outdated
Show resolved
Hide resolved
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 adds Linux support to the .NET language extension, which previously only supported Windows. The changes include:
- Platform-specific conditional compilation using preprocessor defines to support both Windows and Linux
- Integration of nethost library for dynamic hostfxr lookup on Linux (avoiding hardcoded paths)
- Updated build artifacts and project files to target .NET 8.0 (from .NET 6.0, which is EOL)
- Platform-specific APIs for file operations, dynamic library loading, and path handling
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Microsoft.SqlServer.CSharpExtensionTest.csproj | Updated target framework from net6.0 to net8.0 |
| Microsoft.SqlServer.CSharpExtension.csproj | Updated target framework from net6.0 to net8.0 |
| RegexSample.csproj | Updated target framework from net6.0 to net8.0 |
| Logger.cpp | Removed Windows-specific includes (stdio.h, windows.h) |
| Logger.h | Removed stdio.h include |
| DotnetEnvironment.cpp | Added platform-specific code for library loading, path handling, and hostfxr lookup using nethost |
| DotnetEnvironment.h | Added platform-specific macros and definitions for path separators and string types |
| nativecsharpextension.h | Guarded windows.h include with platform check |
| nethost.h | Added header file for nethost API |
| libnethost.a | Added static library for nethost (Linux) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if defined(_WIN32) || defined(WINDOWS) | ||
| m_root_path(to_utf16_str(language_path)) | ||
| #else | ||
| m_root_path(language_path) | ||
| #endif |
Copilot
AI
Nov 10, 2025
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 conditional initialization of m_root_path in the constructor initializer list could be simplified. Consider using a helper function to convert the path conditionally, which would make the code cleaner and easier to maintain. For example, create a convert_path(const std::string& path) function that returns either the UTF-16 or UTF-8 version based on the platform.
| char buffer[4096]; | ||
| size_t buffer_size = sizeof(buffer); | ||
| if (get_hostfxr_path(buffer, &buffer_size, nullptr) != 0) { | ||
| return false; | ||
| } |
Copilot
AI
Nov 10, 2025
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 hardcoded buffer size of 4096 bytes should be defined as a named constant (e.g., constexpr size_t HOSTFXR_PATH_BUFFER_SIZE = 4096;) to improve code maintainability and make it easier to adjust if needed.
| #include "DotnetEnvironment.h" | ||
| #include "Logger.h" | ||
|
|
||
| #if defined(_WIN32) || defined(WINDOWS) |
Copilot
AI
Nov 10, 2025
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 platform check uses both _WIN32 and WINDOWS macros. The _WIN32 macro is the standard Microsoft compiler define for Windows (both 32-bit and 64-bit). The WINDOWS macro appears to be custom. Consider using only _WIN32 for consistency with common practice, or document why both are needed if there's a specific reason.
|
@MarkpageBxl , have you ever met this error: Error: Could not find a part of the path |
This PR adds Linux support to the .NET language extension, which currently only supports Windows.
This involves the following:
hostfxrbetween Linux and Windows, using a hardcodedhostfxrlibrary path was a no go for portability (e.g. .NET packages on RHEL and Debian do not install .NET in the same base directories). Instead, we make use ofnethost(in the form of thelibnethost.astatic library provided by the .NET runtime) to first do the hostfxr lookup. This approach has the advantage of honoringDOTNET_ROOTandinstall_locationfiles if they exist, which hostfxr does not.Additionally, this bumps references to
net6.0up tonet8.0, since the former is EOL.At this point, this has been tested manually on SQL Server running on Ubuntu 22.04 LTS. A .NET 8.0 assembly was successfully executed.