-
Notifications
You must be signed in to change notification settings - Fork 71
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
feat: enable CI workflows to use NuGet WDK packages #236
base: main
Are you sure you want to change the base?
Changes from all commits
36b1d1e
7e467f6
f04f22f
b641096
6d024aa
d901e20
3038dc2
40441ad
80996e1
ffc24b3
e86b142
3922faf
03f6633
7343913
13fd747
9d1698d
52e0ac7
5e29f19
1046cbf
3b39c40
dbc2f27
edcbec9
c0eeeb8
9b9bb08
4c5cb6e
2e28af7
860b337
02e3fbd
abd946e
a656057
7692e01
70c7b6b
a7a2fe8
3c51e27
17e02da
72e518d
efcb4db
2e3024f
98df970
30f4fa6
e156edd
bf6203c
d3c0fad
4df4330
a421cd7
b8fdb0d
41a7b7e
ccdb96a
f632131
d2e3a61
beedcb0
37b4bd6
f4087df
a559f6d
a350aa5
5868673
a422a4c
0256275
058b784
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,121 @@ | ||||
param ( | ||||
[string]$wdkVersion | ||||
) | ||||
|
||||
# Extract the first three parts of the version | ||||
$majorVersionNum = ($wdkVersion -split '\.')[0..2] -join '.' | ||||
|
||||
Write-Host "Installing WDK version $wdkVersion" | ||||
Write-Host "Major version number: $majorVersionNum" | ||||
|
||||
# Update the packages.config file with the version passed as parameter | ||||
$packagesConfigPath = ".\packages.config" | ||||
|
||||
if (Test-Path $packagesConfigPath) { | ||||
[xml]$packagesConfig = Get-Content $packagesConfigPath | ||||
|
||||
foreach ($package in $packagesConfig.packages.package) { | ||||
if ($package.id -like "Microsoft.Windows.*") { | ||||
$package.version = $wdkVersion | ||||
} | ||||
} | ||||
|
||||
$packagesConfig.Save($packagesConfigPath) | ||||
Write-Host "Updated packages.config with version $wdkVersion" | ||||
} else { | ||||
Write-Error "packages.config file not found" | ||||
} | ||||
|
||||
# Install WDK using NuGet | ||||
try { | ||||
nuget restore .\packages.config -PackagesDirectory C:\WDK | ||||
Write-Host "WDK installed at C:\WDK" | ||||
} catch { | ||||
Write-Error "Failed to restore packages using NuGet. Error: $_" | ||||
exit 1 | ||||
} | ||||
|
||||
$folders = @( | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is there all this manual copying of folders after the nuget packages are restored? this seems very brittle There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this function actually uses see #236 (comment) for more details, but the tl;dr is that the nuget WDK's use in c drivers works by setting the include and bin paths via modifying the vcxproj. as stated in my linked comment, since there is no path forward to use the same solution that nuget uses in c drivers (ie. vcxproj is not supported by our rust tooling), we should rely on the env vars used by ewdk |
||||
"C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion", | ||||
"C:\WDK\Microsoft.Windows.SDK.CPP.x64.$wdkVersion", | ||||
"C:\WDK\Microsoft.Windows.SDK.CPP.arm64.$wdkVersion", | ||||
"C:\WDK\Microsoft.Windows.WDK.ARM64.$wdkVersion", | ||||
"C:\WDK\Microsoft.Windows.SDK.CPP.$wdkVersion" | ||||
) | ||||
foreach ($folder in $folders) { | ||||
if (-Not (Test-Path $folder)) { | ||||
Write-Error "Required folder $folder is missing." | ||||
exit 1 | ||||
} | ||||
} | ||||
function Copy-File { | ||||
param ( | ||||
[string]$sourcePath, | ||||
[string]$destinationPath, | ||||
[string]$fileName | ||||
) | ||||
|
||||
if (Test-Path $sourcePath) { | ||||
Copy-Item -Path $sourcePath -Destination $destinationPath -Force | ||||
Write-Host "Copied $fileName to $destinationPath" | ||||
} else { | ||||
Write-Error "$fileName not found at $sourcePath" | ||||
} | ||||
} | ||||
|
||||
# Copying signtool to WDK bin folder | ||||
$signtoolx64 = "C:\WDK\Microsoft.Windows.SDK.CPP.$wdkVersion\c\bin\$majorVersionNum.0\x64\signtool.exe" | ||||
$signtoolX86 = "C:\WDK\Microsoft.Windows.SDK.CPP.$wdkVersion\c\bin\$majorVersionNum.0\x86\signtool.exe" | ||||
$destinationx64 = "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\bin\$majorVersionNum.0\x64" | ||||
$destinationX86 = "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\bin\$majorVersionNum.0\x86" | ||||
|
||||
Copy-File -sourcePath $signtoolX64 -destinationPath $destinationX64 -fileName "signtool.exe" | ||||
Copy-File -sourcePath $signtoolX86 -destinationPath $destinationX86 -fileName "signtool.exe" | ||||
|
||||
# Copying certmgr to WDK bin folder | ||||
$certmgrx86 = "C:\WDK\Microsoft.Windows.SDK.CPP.$wdkVersion\c\bin\$majorVersionNum.0\x86\certmgr.exe" | ||||
$certmgrX64 = "C:\WDK\Microsoft.Windows.SDK.CPP.$wdkVersion\c\bin\$majorVersionNum.0\x64\certmgr.exe" | ||||
$certmgrARM64 = "C:\WDK\Microsoft.Windows.SDK.CPP.$wdkVersion\c\bin\$majorVersionNum.0\arm64\certmgr.exe" | ||||
$destinationx86 = "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\bin\$majorVersionNum.0\x86" | ||||
$destinationx64 = "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\bin\$majorVersionNum.0\x64" | ||||
$destinationARM64 = "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\bin\$majorVersionNum.0\ARM64" | ||||
|
||||
Copy-File -sourcePath $certmgrx86 -destinationPath $destinationx86 -fileName "certmgr.exe" | ||||
Copy-File -sourcePath $certmgrX64 -destinationPath $destinationx64 -fileName "certmgr.exe" | ||||
Copy-File -sourcePath $certmgrARM64 -destinationPath $destinationARM64 -fileName "certmgr.exe" | ||||
|
||||
# Copying makecert to WDK bin folder | ||||
$makecertx86 = "C:\WDK\Microsoft.Windows.SDK.CPP.$wdkVersion\c\bin\$majorVersionNum.0\x86\MakeCert.exe" | ||||
$makecertX64 = "C:\WDK\Microsoft.Windows.SDK.CPP.$wdkVersion\c\bin\$majorVersionNum.0\x64\MakeCert.exe" | ||||
$makecertARM64 = "C:\WDK\Microsoft.Windows.SDK.CPP.$wdkVersion\c\bin\$majorVersionNum.0\arm64\MakeCert.exe" | ||||
$destinationx86 = "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\bin\$majorVersionNum.0\x86" | ||||
$destinationx64 = "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\bin\$majorVersionNum.0\x64" | ||||
$destinationARM64 = "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\bin\$majorVersionNum.0\ARM64" | ||||
|
||||
Copy-File -sourcePath $makecertx86 -destinationPath $destinationx86 -fileName "MakeCert.exe" | ||||
Copy-File -sourcePath $makecertX64 -destinationPath $destinationx64 -fileName "MakeCert.exe" | ||||
Copy-File -sourcePath $makecertARM64 -destinationPath $destinationARM64 -fileName "MakeCert.exe" | ||||
|
||||
function Copy-Folder { | ||||
param ( | ||||
[string]$sourceFolder, | ||||
[string]$destinationFolder | ||||
) | ||||
|
||||
if (Test-Path $sourceFolder) { | ||||
Copy-Item -Path $sourceFolder -Destination $destinationFolder -Recurse -Force | ||||
Write-Host "Copied $sourceFolder to $destinationFolder" | ||||
} else { | ||||
Write-Error "Source folder $sourceFolder not found" | ||||
} | ||||
} | ||||
|
||||
# Copying km, um, kmdf, umdf ARM64 headers to x64 folders | ||||
Copy-Folder -sourceFolder "C:\WDK\Microsoft.Windows.WDK.ARM64.$wdkVersion\c\Lib\$majorVersionNum.0\km\arm64" -destinationFolder "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\Lib\$majorVersionNum.0\km" | ||||
Copy-Folder -sourceFolder "C:\WDK\Microsoft.Windows.WDK.ARM64.$wdkVersion\c\Lib\$majorVersionNum.0\um\arm64" -destinationFolder "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\Lib\$majorVersionNum.0\um" | ||||
Copy-Folder -sourceFolder "C:\WDK\Microsoft.Windows.WDK.ARM64.$wdkVersion\c\Lib\wdf\kmdf\ARM64" -destinationFolder "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\Lib\wdf\kmdf" | ||||
Copy-Folder -sourceFolder "C:\WDK\Microsoft.Windows.WDK.ARM64.$wdkVersion\c\Lib\wdf\umdf\ARM64" -destinationFolder "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c\Lib\wdf\umdf" | ||||
|
||||
# Set WDKContentRoot environment variable | ||||
$NugetWdkContentRoot = "C:\WDK\Microsoft.Windows.WDK.x64.$wdkVersion\c" | ||||
Write-Output "WDKContentRoot=$NugetWdkContentRoot" >> $env:GITHUB_ENV |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,10 @@ jobs: | |
matrix: | ||
wdk: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than special casing the NI WDK, I think you should explicityly add another matrix dimension for WDK source. Then you can exclude the WDK versions from nuget that aren't available via the exclude feature. It would look something like: matrix:
wdk_source: [nuget, winget]
wdk_version:
- 10.0.22621 # NI WDK
- 10.0.26100 #GE WDK
exclude:
# 10.0.22621 not available on nuget
- wdk_source: nuget
wdk_version: 10.0.22621 This way, your later steps can switch on |
||
- Microsoft.WindowsWDK.10.0.22621 # NI WDK | ||
# Use versions numbers for NuGet WDK packages | ||
# - 10.0.26100.1882 | ||
# - 10.0.26100.1591 | ||
Comment on lines
+24
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are just QFE versions right? I think either have all of the QFE versions enabled, or remove the comment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also why not use the latest QFE here, if there is only going to be one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the best thing to do here is actually leave off the QFE version and get the latest QFE for a given build number There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW the varying QFEs here do refer to meaningfully different builds of the WDK now that we've moved to a CD model. It is probably fine to just use the latest GE CD kit for the matrix rather than run each one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So for a given build number, a user should always be using the latest QFE, right?
Is this WRT to GE only, or do you think its always fine to just run on the latest QFE for every build number(including vNext wdk)? my intuition here is to have the matrix always run:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe that prior to Ge there was no meaningful difference between QFEs. Kits almost exclusively shipped once per release. I think there was a special case in one release, but the concept of "multiple kits with the same build number but different QFEs, with different content" is new to Ge and CD. |
||
- 10.0.26100.1 | ||
|
||
llvm: | ||
- 17.0.6 | ||
|
@@ -57,6 +61,7 @@ jobs: | |
clang --version | ||
|
||
- name: Install WDK (${{ matrix.wdk }}) | ||
if: matrix.wdk == 'Microsoft.WindowsWDK.10.0.22621' | ||
run: | | ||
if ((Get-WinGetPackage -Id ${{ matrix.wdk }} -Source winget -MatchOption Equals).Id -eq '${{ matrix.wdk }}') { | ||
Write-Host "${{ matrix.wdk }} is already installed. Attempting to update..." | ||
|
@@ -65,6 +70,17 @@ jobs: | |
Write-Host "Installing ${{ matrix.wdk }}..." | ||
Install-WinGetPackage -Id ${{ matrix.wdk }} -Source winget -MatchOption Equals -Mode Silent -Force | ||
} | ||
|
||
- name: Install ${{ matrix.wdk }} (NuGet) | ||
if: startsWith(matrix.wdk, '10.0.') | ||
run: | | ||
$wdk = '${{ matrix.wdk }}' | ||
if ($wdk -match '^10\.0\.\d{5}\.\d{1,4}$') { | ||
./.github/scripts/nuget-install-wdk.ps1 -wdkVersion $wdk | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needing to run this ps script seems a little hacky. Is there a reason you can't just |
||
} else { | ||
Write-Host "WDK version $wdk does not match the required format 10.0.{5 digit number}.{1 to 4 digit number}" | ||
exit 1 | ||
} | ||
|
||
- name: Install Rust Toolchain (${{ matrix.rust_toolchain }}) | ||
uses: dtolnay/rust-toolchain@master | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<packages> | ||
<package id="Microsoft.Windows.SDK.CPP" version="10.0.26100.1591" targetFramework="native" /> | ||
<package id="Microsoft.Windows.SDK.CPP.x64" version="10.0.26100.1591" targetFramework="native" /> | ||
<package id="Microsoft.Windows.SDK.CPP.arm64" version="10.0.26100.1591" targetFramework="native" /> | ||
<package id="Microsoft.Windows.WDK.x64" version="10.0.26100.1591" targetFramework="native" /> | ||
<package id="Microsoft.Windows.WDK.arm64" version="10.0.26100.1591" targetFramework="native" /> | ||
</packages> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: newline at end of file is missing |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,20 +19,7 @@ use wdk::println; | |
#[cfg(not(test))] | ||
use wdk_alloc::WdkAllocator; | ||
use wdk_sys::{ | ||
call_unsafe_wdf_function_binding, | ||
ntddk::DbgPrint, | ||
DRIVER_OBJECT, | ||
NTSTATUS, | ||
PCUNICODE_STRING, | ||
ULONG, | ||
UNICODE_STRING, | ||
WCHAR, | ||
WDFDEVICE, | ||
WDFDEVICE_INIT, | ||
WDFDRIVER, | ||
WDF_DRIVER_CONFIG, | ||
WDF_NO_HANDLE, | ||
WDF_NO_OBJECT_ATTRIBUTES, | ||
call_unsafe_wdf_function_binding, ntddk::DbgPrint, DRIVER_OBJECT, NTSTATUS, PCUNICODE_STRING, PDRIVER_OBJECT, ULONG, UNICODE_STRING, WCHAR, WDFDEVICE, WDFDEVICE_INIT, WDFDRIVER, WDF_DRIVER_CONFIG, WDF_NO_HANDLE, WDF_NO_OBJECT_ATTRIBUTES | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a blocker, but I am curious how this reformat wasn't caught sooner by CI/other pipeline checkins. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this change actually needs to be reverted? I suspect that whats going on is that:
There are some gaps in the top-level examples and tests folders where certain pipelines are not fully covering them. spent some time figuring out what gaps there are (mostly due to changes related to #186 ) and will be adding the rundown to github in a pr |
||
}; | ||
|
||
#[cfg(not(test))] | ||
|
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.
rather than updating a version thats hardcoded into a packages.config (that is only used in CI), can you just use
nuget install
in the action step?