Script to update WDK version #848
Conversation
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
WalkthroughThe pull request introduces enhancements to the PowerShell script Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (1)
scripts/Update-WdkVersion.ps1 (1)
30-35: Consider using configuration file for file pathsThe hardcoded file paths make the script less maintainable. Consider moving them to a configuration file.
+# Read paths from config file +$config_path = Join-Path $PSScriptRoot "update_wdk_config.json" +$config = Get-Content $config_path | ConvertFrom-Json +$vs_files_to_update = $config.files_to_update -$vs_files_to_update = @( - "wdk.props", - "tools\bpf2c\templates\kernel_mode_bpf2c.vcxproj", - "tools\bpf2c\templates\user_mode_bpf2c.vcxproj" -)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
scripts/Update-WdkVersion.ps1(1 hunks)scripts/setup_build/packages.config.template(1 hunks)tools/bpf2c/templates/kernel_mode_bpf2c.vcxproj(2 hunks)tools/bpf2c/templates/user_mode_bpf2c.vcxproj(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- scripts/setup_build/packages.config.template
- tools/bpf2c/templates/kernel_mode_bpf2c.vcxproj
🔇 Additional comments (1)
tools/bpf2c/templates/user_mode_bpf2c.vcxproj (1)
51-55: LGTM: Import statements updated correctly
The import statements have been correctly updated to use the WDKVersion variable.
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
scripts/Update-WdkVersion.ps1 (1)
1-111: Consider adding advanced features for better maintainability and user experienceThe script could benefit from the following architectural improvements:
- Add a logging mechanism for better troubleshooting
- Implement a dry-run option to preview changes
- Add version comparison to skip unnecessary updates
- Add a progress bar for multiple file updates
- Consider adding a configuration file for file paths and patterns
These improvements would make the script more maintainable and user-friendly.
Would you like me to provide implementation details for any of these improvements?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
scripts/Update-WdkVersion.ps1(1 hunks)
🔇 Additional comments (4)
scripts/Update-WdkVersion.ps1 (4)
1-3: LGTM: License header is appropriate
The MIT license header with copyright notice is correctly formatted.
4-33: LGTM: Robust implementation of package version retrieval
The implementation includes comprehensive error handling and validation as previously suggested.
35-67: Consider including original error message in the throw statement
While the implementation is robust and follows best practices, the error message could be more informative.
- throw "Failed to update version in file: $vs_file_path"
+ throw "Failed to update version in file $vs_file_path`: $_"88-111:
Add overall error handling and rollback mechanism
The main execution flow needs error handling to ensure all files are either updated successfully or rolled back to their original state.
+# Track updated files for rollback
+$updated_files = @()
+
# Paths relative to the root of the repository
$vs_files_to_update = @(
"wdk.props",
"tools\bpf2c\templates\kernel_mode_bpf2c.vcxproj",
"tools\bpf2c\templates\user_mode_bpf2c.vcxproj"
)
-# Get the current WDK version
-$wdk_version_number = Get-PackageVersion "Microsoft.Windows.WDK.x64"
+try {
+ # Verify all paths exist before starting
+ foreach ($vs_file in $vs_files_to_update) {
+ $full_path = Join-Path $PSScriptRoot "..\$vs_file"
+ if (-not (Test-Path $full_path)) {
+ throw "Required file not found: $vs_file"
+ }
+ }
-# Print the version number
-Write-Output "Found WDK version: $wdk_version_number"
+ # Get the current WDK version
+ $wdk_version_number = Get-PackageVersion "Microsoft.Windows.WDK.x64"
+ Write-Output "Found WDK version: $wdk_version_number"
-# Replace version in each VS file
-foreach ($vs_file in $vs_files_to_update) {
- Write-Host "Updating WDK version in $vs_file"
- $vs_file = $PSScriptRoot + "\..\" + $vs_file
- Update-VersionInVsFile $vs_file $wdk_version_number
-}
+ # Replace version in each VS file
+ foreach ($vs_file in $vs_files_to_update) {
+ Write-Host "Updating WDK version in $vs_file"
+ $full_path = Join-Path $PSScriptRoot "..\$vs_file"
+ Update-VersionInVsFile $full_path $wdk_version_number
+ $updated_files += $full_path
+ }
-Update-TemplateFile -template_file_path "$PSScriptRoot\..\scripts\setup_build\packages.config.template" -output_file_path "$PSScriptRoot\..\scripts\setup_build\packages.config" -version_number $wdk_version_number
+ $template_path = Join-Path $PSScriptRoot "..\scripts\setup_build\packages.config.template"
+ $output_path = Join-Path $PSScriptRoot "..\scripts\setup_build\packages.config"
+ Update-TemplateFile -template_file_path $template_path -output_file_path $output_path -version_number $wdk_version_number
+ $updated_files += $output_path
-# Print success message
-Write-Output "Updated WDK version in all files"
+ # Print success message
+ Write-Output "Updated WDK version in all files"
+} catch {
+ Write-Error "Failed to update WDK version: $_"
+ # Attempt to restore backups
+ foreach ($file in $updated_files) {
+ $backup_path = "$file.bak"
+ if (Test-Path $backup_path) {
+ Copy-Item $backup_path $file -Force
+ Remove-Item $backup_path
+ Write-Warning "Rolled back changes to $file"
+ }
+ }
+ exit 1
+}Likely invalid or redundant comment.
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
scripts/Update-WdkVersion.ps1 (3)
24-27: Consider using System.Version for version comparisonWhile the regex validation is good, using
System.Versionwould provide more robust version comparison capabilities.-if (-not ($packageVersion -match '^\d+\.\d+\.\d+\.\d+$')) { +[System.Version]$parsedVersion = $null +if (-not [System.Version]::TryParse($packageVersion, [ref]$parsedVersion)) { throw "Invalid version format: $packageVersion" }
113-117: Verify file paths before starting updatesAdd validation for all file paths before starting any updates to fail fast.
$vs_files_to_update = @( "wdk.props", "tools\bpf2c\templates\kernel_mode_bpf2c.vcxproj", "tools\bpf2c\templates\user_mode_bpf2c.vcxproj" ) +# Verify all paths exist before starting updates +foreach ($vs_file in $vs_files_to_update) { + $full_path = Join-Path $PSScriptRoot "..\$vs_file" + if (-not (Test-Path $full_path)) { + throw "Required file not found: $vs_file" + } +}
142-145: Add logging of rollback operationsEnhance rollback logging to include success/failure status.
-Write-Host "Rolling back changes in $file" +Write-Host "Rolling back changes in $file..." Copy-Item "$file.bak" $file -Force Remove-Item "$file.bak" +Write-Host "Successfully rolled back changes in $file"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
scripts/Update-WdkVersion.ps1(1 hunks)
🔇 Additional comments (4)
scripts/Update-WdkVersion.ps1 (4)
1-2: LGTM: License header is properly formatted
The copyright notice and SPDX license identifier are correctly specified.
1-154: Overall implementation is robust and production-ready
The script demonstrates good practices with:
- Comprehensive error handling
- Backup and rollback mechanisms
- Input validation
- Clear logging
The suggested improvements are minor enhancements to an already solid implementation.
95-96: 🛠️ Refactor suggestion
Add verification after template replacement
The template replacement should be verified before writing back to the file.
$template_file_content = $template_file_content -replace "\$\(WDKVersion\)", $version_number
+if ($template_file_content -notmatch [regex]::Escape($version_number)) {
+ throw "Failed to update version in template. Pattern not found."
+}Likely invalid or redundant comment.
55-56: 🛠️ Refactor suggestion
Add verification after version replacement
The version replacement should be verified before writing back to the file.
$vs_file_content = $vs_file_content -replace "<WDKVersion>.*</WDKVersion>", "<WDKVersion>$version_number</WDKVersion>"
+if ($vs_file_content -notmatch [regex]::Escape("<WDKVersion>$version_number</WDKVersion>")) {
+ throw "Failed to update version in file. Pattern not found."
+}Likely invalid or redundant comment.
Description
Testing
Do any existing tests cover this change? Are new tests needed?
Documentation
Is there any documentation impact for this change?
Installation
Is there any installer impact for this change?
Summary by CodeRabbit
New Features
Improvements