Add Linux support to .NET language extension - #64
Conversation
|
@microsoft-github-policy-service agree company="Raincode srl" |
There was a problem hiding this comment.
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.
|
@MarkpageBxl , have you ever met this error: Error: Could not find a part of the path |
SicongLiu2000
left a comment
There was a problem hiding this comment.
Code Review — PR #64: Add Linux support to .NET language extension
I tested this end-to-end in a Docker container (SQL Server 2022 + mssql-server-extensibility on Ubuntu) and confirmed the Linux path works — the regex sample returns correct results via sp_execute_external_script.
Overall this is a clean PR. The platform #ifdef guards, nethost integration for hostfxr discovery, and the Linux build scripts are well done. A few comments below.
Issues
1. STR/CH macros duplicated in both .h and .cpp
DotnetEnvironment.h and DotnetEnvironment.cpp both define STR(s) and CH(c) with the same #ifdef guards. They should be defined in only one place (the header) to avoid maintenance drift.
2. README link mismatch in Linux build section
The Linux build instructions reference build-dotnet-core-CSharp-extension.ps1 but the link text shows .cmd. Same for the archive script.
3. Pre-existing issues on main (not from this PR, but worth noting)
While testing, I found critical memory safety bugs in CSharpParamContainer.cs that already exist on main:
ReplaceNumericParam<T>: uses&value(dangling stack pointer) instead of the correctGCHandle.Alloc(array, Pinned)+AddrOfPinnedObject()pattern. This corrupts all numeric output parameters.ReplaceStringParam:fixedblock pointer escapes its scope while theGCHandleis not pinned. This corrupts string output parameters.GetStrLenNullMap/ReplaceParamfor DotNetChar: uses.Length(char count) instead ofEncoding.UTF8.GetByteCount()— wrong for multi-byte UTF-8.
These should be tracked as a separate issue/fix.
Positive
nethost/get_hostfxr_path()for Linux hostfxr discovery is the right approachPATH_SEPARATORmacro is clean- Build scripts (
.ps1) are well-structured with proper error handling net8.0target upgrade is appropriate (net6.0 is EOL)- End-to-end verified working on Linux
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).
04be02e to
86d15ea
Compare
|
I have rebased on main, and made an additional fallout fix re the use of the |
|
I also assume we should probably upgrade to |
- Replace PowerShell build scripts with bash (build, archive, restore) - Remove checked-in libnethost.a; fetch at build time via restore-packages.sh - Add libnethost.a existence guard in build script - Use vswhere.exe for VS detection in Windows build scripts - Add convert_string() abstraction in DotnetEnvironment - Conditional nethost.h include (Linux only), HOSTFXR_PATH_BUFFER_SIZE constant - Add LOG_ERROR on hostfxr lookup failure - Fix load_hostfxr comment (was incorrectly named get_export) - Add Linux test build/run scripts and CMake platform support - Add -fshort-wchar and fix wstring ABI issues in test code - Add platform-conditional HintPath in test csproj - Add Linux dlopen/dlsym equivalents in test API code - Add Dockerfile for Linux build/test CI
- Replace PowerShell build scripts with bash (build, archive, restore) - Remove checked-in libnethost.a; fetch at build time via restore-packages.sh - Add libnethost.a existence guard in build script - Use vswhere.exe for VS detection in Windows build scripts - Add convert_string() abstraction in DotnetEnvironment - Conditional nethost.h include (Linux only), HOSTFXR_PATH_BUFFER_SIZE constant - Add LOG_ERROR on hostfxr lookup failure - Fix load_hostfxr comment (was incorrectly named get_export) - Add Linux test build/run scripts and CMake platform support - Add -fshort-wchar and fix wstring ABI issues in test code - Add platform-conditional HintPath in test csproj - Add Linux dlopen/dlsym equivalents in test API code - Add Dockerfile for Linux build/test CI (cherry picked from commit 2497aa2)
- Replace PowerShell build scripts with bash (build, archive, restore) - Remove checked-in libnethost.a; fetch at build time via restore-packages.sh - Add libnethost.a existence guard in build script - Use vswhere.exe for VS detection in Windows build scripts - Add convert_string() abstraction in DotnetEnvironment - Conditional nethost.h include (Linux only), HOSTFXR_PATH_BUFFER_SIZE constant - Add LOG_ERROR on hostfxr lookup failure - Fix load_hostfxr comment (was incorrectly named get_export) - Add Linux test build/run scripts and CMake platform support - Add -fshort-wchar and fix wstring ABI issues in test code - Add platform-conditional HintPath in test csproj - Add Linux dlopen/dlsym equivalents in test API code - Add Dockerfile for Linux build/test CI (cherry picked from commit 2497aa2)
d8ec67b to
a6de766
Compare
- Replace PowerShell build scripts with bash (build, archive, restore) - Remove checked-in libnethost.a; fetch at build time via restore-packages.sh - Add libnethost.a existence guard in build script - Use vswhere.exe for VS detection in Windows build scripts - Add convert_string() abstraction in DotnetEnvironment - Conditional nethost.h include (Linux only), HOSTFXR_PATH_BUFFER_SIZE constant - Add LOG_ERROR on hostfxr lookup failure - Fix load_hostfxr comment (was incorrectly named get_export) - Add Linux test build/run scripts and CMake platform support - Add -fshort-wchar and fix wstring ABI issues in test code - Add platform-conditional HintPath in test csproj - Add Linux dlopen/dlsym equivalents in test API code - Add Dockerfile for Linux build/test CI (cherry picked from commit 2497aa2)
a6de766 to
a82ff36
Compare
- Replace PowerShell build scripts with bash (build, archive, restore) - Remove checked-in libnethost.a; fetch at build time via restore-packages.sh - Add libnethost.a existence guard in build script - Add convert_string() abstraction in DotnetEnvironment - Conditional nethost.h include (Linux only), HOSTFXR_PATH_BUFFER_SIZE constant - Add LOG_ERROR on hostfxr/dlopen/dlsym failures (release-safe, not just assert) - Guard S_OK/E_FAIL macros (#ifndef) on non-Windows - Add Linux test build/run scripts and CMake platform support - Add -fshort-wchar and replace std::wstring with vector<wchar_t>/raw wchar_t* in test code to avoid ABI mismatch under -fshort-wchar on Linux - Add platform-conditional HintPath in test csproj - Add Linux dlopen/dlsym equivalents in test API code; throw on readlink failure (cherry picked from commit 2497aa2)
a82ff36 to
ce93292
Compare
Reconcile the Linux support with main, which has since added DECIMAL (microsoft#83), ZIP archive InstallExternalLibrary (microsoft#85), and SetHostCallbacks/API v3 (microsoft#90). Conflict + fallout resolutions (Linux portability for the new main code): - CMakeLists.txt: keep the WIN32/else split (Linux gtest-from-source, -fshort-wchar, dl/pthread linking); drop main''s unconditional -D_WIN64/_WINDOWS which broke Linux. - CSharpExtensionApiTests.cpp GetHandles(): resolve the InstallExternalLibrary/ UninstallExternalLibrary symbol lookups for both the Windows (GetProcAddress) and Linux (dlsym) branches. - CSharpLibraryTests.cpp (new, microsoft#85): add cross-platform GetExecutablePath() helper (readlink on Linux), guard LocalFree/HLOCAL with free() on Linux. - CSharpSetHostCallbacksTests.cpp (new, microsoft#90): make RESOLVE_SET_HOST_CALLBACKS use dlsym on Linux. - run-...-test.sh: export ENL_ROOT so the test binary can locate test_packages.
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.