Update MAUI dogfooding scripts to work with new Azure DevOps instance#33198
Update MAUI dogfooding scripts to work with new Azure DevOps instance#33198
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the MAUI dogfooding scripts to work with the new Azure DevOps instance (dnceng-public instead of xamarin), enabling users to more easily test PR builds. The changes include infrastructure updates for the new build system, improved error handling with timeouts, better version detection logic, and enhanced user experience with the -Yes parameter and more helpful revert instructions.
Key changes:
- Updates Azure DevOps organization from
xamarintodnceng-public - Changes artifact name from
nugettoPackageArtifactsand build check pattern fromMAUI-publictomaui-pr* - Adds
-Yesparameter to PowerShell script for automated workflows and timeout parameters to prevent hanging on network calls
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| eng/scripts/get-maui-pr.sh | Updates Azure DevOps instance name, artifact naming, build check pattern, removes backup file cleanup code, improves version compatibility logic, and enhances revert instructions with actual stable version fetching |
| eng/scripts/get-maui-pr.ps1 | Updates Azure DevOps instance name, adds -Yes parameter for automation, updates artifact naming and build check pattern, adds timeout parameters to HTTP requests, changes Get-BuildInfo return type, improves version sorting, and duplicates stable version fetching logic |
Comments suppressed due to low confidence (1)
eng/scripts/get-maui-pr.ps1:456
- The PowerShell script is missing the extraction of
$packageDotNetVersionthat the bash script has (lines 469-472 in get-maui-pr.sh). This variable is later referenced on line 505 and 522 but never defined, which will cause those conditions to always fail. You need to add the major version extraction logic here, similar to the bash script.
Write-Step "Extracting package information"
$version = Get-PackageVersion -PackagesDir $packagesDir
Write-Success "Found package version: $version"
|
/rebase |
18bb5ad to
deb95ad
Compare
Added a -Yes switch to auto-accept confirmations and updated Azure DevOps organization.
Changed the dogfood-comment workflow to trigger on check_run completion instead of pull_request_target. This ensures the comment is only posted after the maui-pr build successfully completes, so users can actually use the artifacts. The workflow now: - Triggers on check_run completed events - Only runs for the check named exactly 'maui-pr' - Only runs when the check conclusion is 'success' - Still supports manual triggering via workflow_dispatch
deb95ad to
e5f4afa
Compare
| # Get latest stable version for revert instructions | ||
| $stableVersion = "X.Y.Z" | ||
| try { | ||
| $nugetResponse = Invoke-RestMethod -Uri "https://api.nuget.org/v3-flatcontainer/$($PackageName.ToLower())/index.json" -Method Get -ErrorAction SilentlyContinue | ||
| if ($nugetResponse -and $nugetResponse.versions) { | ||
| $stableVersions = $nugetResponse.versions | Where-Object { $_ -notmatch '-' } | ||
| if ($stableVersions) { | ||
| $stableVersion = $stableVersions | Select-Object -Last 1 | ||
| } | ||
| } | ||
| } catch { | ||
| # If we can't fetch, just use placeholder | ||
| } |
There was a problem hiding this comment.
This code block for fetching the latest stable version is duplicated - the same logic appears earlier at lines 534-544. The first occurrence fetches into $latestStable (which is unused), while this second occurrence at lines 565-577 fetches into $stableVersion (which is actually used at line 585). Consider removing the first occurrence at lines 534-544 to eliminate the duplicate API call and unused variable.
| if ($willUpdateTfm) { | ||
| $targetVersionForDisplay = if ($packageDotNetVersion) { "$packageDotNetVersion.0" } else { "$packageNetVersion.0" } | ||
| $targetVersionForDisplay = if ($packageDotNetVersion) { "$packageDotNetVersion.0" } else { "10.0" } | ||
| Write-Host " • Target framework: Will be updated to .NET $targetVersionForDisplay" -ForegroundColor Gray | ||
| } |
There was a problem hiding this comment.
The code removed the extraction of $packageDotNetVersion from the version string (the old lines 487-495) but this variable is still used at line 505 (visible in the diff) and line 522 (outside diff but nearby). Since $packageDotNetVersion is undefined, the condition will always be false and the fallback "10.0" will always be used.
To fix this, either:
- Re-add code to extract
$packageDotNetVersionfrom$versionusing regex similar to the shell script (lines 469-472), or - Use
$packageNetVersiondirectly instead of the conditional, since it already contains the correct .NET version.
| github.event_name == 'workflow_dispatch' || | ||
| ( | ||
| github.event_name == 'check_run' && | ||
| github.event.check_run.name == 'maui-pr' && |
There was a problem hiding this comment.
The workflow condition checks for an exact match github.event.check_run.name == 'maui-pr', but the scripts (both bash and PowerShell) look for check names that start with maui-pr (using wildcard patterns like maui-pr*).
If the actual check name from the new Azure DevOps instance is something like maui-pr-12345 or maui-pr-build (with a suffix), this workflow will never trigger because the exact match will fail, even though the scripts would correctly find the build.
Consider using a pattern match in the workflow condition as well, such as:
startsWith(github.event.check_run.name, 'maui-pr')
to be consistent with how the scripts search for builds.
| github.event.check_run.name == 'maui-pr' && | |
| startsWith(github.event.check_run.name, 'maui-pr') && |
PureWeen
left a comment
There was a problem hiding this comment.
local copilot agrees with cloud copilot. Should we resolve the comments before merge>?
Address PR review comments (PureWeen/Copilot): - Fix undefined $packageDotNetVersion in PS1 (use $packageNetVersion) - Remove duplicate NuGet API calls in both scripts - Add -TimeoutSec 30 to remaining NuGet call in PS1 - Use proper Sort-Object version sorting instead of Select-Object -Last - Fix workflow check_run condition to use startsWith() for maui-pr* - Add fallback to workflow concurrency group for non-PR check_runs Fix bugs found during testing: - Fix version extraction matching wrong package (Controls.Core before Controls) by requiring version starts with a digit - Fix bash exit codes: split local var=$(func) to prevent masking errors - Fix PS1 exit code: add exit 1 to catch block - Fix PS1 downloads hanging on macOS: use curl on non-Windows platforms (Invoke-WebRequest is extremely slow for large files in pwsh on macOS) - Fix macOS BSD sed compatibility: use -E extended regex - Fix NuGet source name mismatch: use maui-pr-<N> (unique per PR, matches revert instructions) - Rename Write-Error/Write-Warning to avoid shadowing PS1 built-in cmdlets - Redirect bash output functions to stderr to prevent stdout pollution Improvements: - Add -y/--yes flag to bash script (parity with PS1 -Yes) - Add GITHUB_TOKEN / gh CLI auth support (5000 req/hr vs 60 unauthenticated) - Clean up zip file after extraction to save disk space - Escape regex in PS1 package name matching Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PureWeen
left a comment
There was a problem hiding this comment.
NuGet.config has an accidental local path committed that should be reverted before merge:
<add key="maui-pr-34155" value="/Users/jfversluis/.maui/hives/pr-34155/packages/PackageArtifacts" />This references a local filesystem path from testing and won't exist on anyone else's machine.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The startsWith('maui-pr') filter matched ~30+ check runs per PR (every
build, pack, helix, and integration test job), causing duplicate workflow
triggers. Scope to the exact 'Pack .NET MAUI Pack Windows' check run
which fires exactly once when PackageArtifacts become available.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ss handling (#34259) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Description Three fixes for the dogfooding infrastructure that was merged in #33198 but never worked correctly: ### 1. Workflow trigger fix (`dogfood-comment.yml`) The workflow was changed from `pull_request_target` to `check_run` trigger in #33198, but Azure DevOps check runs **never populate the `pull_requests[]` array**. This means `github.event.check_run.pull_requests[0] != null` was **always false**, and the workflow never ran. **Fix**: Reverted to `pull_request_target` trigger with a comment explaining why `check_run` does not work. ### 2. AzDO direct fallback (both scripts) When the GitHub Checks API does not return build info (which can happen for merge commits or in certain timing windows), the scripts now fall back to querying the Azure DevOps API directly using `refs/pull/{PR}/merge` branch name. This approach is modeled after the working implementation in [maui-version](https://github.com/jfversluis/maui-version). ### 3. In-progress build detection (both scripts) When no completed build is found, the scripts now query AzDO for builds with `status in ("inProgress", "notStarted", "postponed")`. Instead of the generic "no build found" error, users now see: - **Build in progress**: "A build is currently in progress. Please wait for it to complete." - **Stale artifacts**: When an older build has artifacts but a newer build is running, warns users ### Additional improvements - Build ID numeric validation to prevent URL injection - PR number validation in bash script - Protection against `set -e` crashes on invalid `jq` input - Better error messages for all failure scenarios ## Testing Tested locally with multiple real PRs across both scripts: - ✅ PR #34216 (completed, successful) - both scripts find build and apply artifacts - ✅ PR #34250 (in-progress build) - correctly finds existing artifacts with warning - ✅ PR #99999999 (non-existent) - proper error handling - ✅ Invalid PR number ("abc") - input validation works - ✅ Multi-model code review (Claude Sonnet 4.5, GPT-5.1, Gemini 3 Pro) - all findings addressed Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ss handling (#34259) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Description Three fixes for the dogfooding infrastructure that was merged in #33198 but never worked correctly: ### 1. Workflow trigger fix (`dogfood-comment.yml`) The workflow was changed from `pull_request_target` to `check_run` trigger in #33198, but Azure DevOps check runs **never populate the `pull_requests[]` array**. This means `github.event.check_run.pull_requests[0] != null` was **always false**, and the workflow never ran. **Fix**: Reverted to `pull_request_target` trigger with a comment explaining why `check_run` does not work. ### 2. AzDO direct fallback (both scripts) When the GitHub Checks API does not return build info (which can happen for merge commits or in certain timing windows), the scripts now fall back to querying the Azure DevOps API directly using `refs/pull/{PR}/merge` branch name. This approach is modeled after the working implementation in [maui-version](https://github.com/jfversluis/maui-version). ### 3. In-progress build detection (both scripts) When no completed build is found, the scripts now query AzDO for builds with `status in ("inProgress", "notStarted", "postponed")`. Instead of the generic "no build found" error, users now see: - **Build in progress**: "A build is currently in progress. Please wait for it to complete." - **Stale artifacts**: When an older build has artifacts but a newer build is running, warns users ### Additional improvements - Build ID numeric validation to prevent URL injection - PR number validation in bash script - Protection against `set -e` crashes on invalid `jq` input - Better error messages for all failure scenarios ## Testing Tested locally with multiple real PRs across both scripts: - ✅ PR #34216 (completed, successful) - both scripts find build and apply artifacts - ✅ PR #34250 (in-progress build) - correctly finds existing artifacts with warning - ✅ PR #99999999 (non-existent) - proper error handling - ✅ Invalid PR number ("abc") - input validation works - ✅ Multi-model code review (Claude Sonnet 4.5, GPT-5.1, Gemini 3 Pro) - all findings addressed Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…dotnet#33198) Updates the scripts to use the new Azure DevOps instance. After this we can enable the dogfooding comment again so people can test PRs easier. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Updates the scripts to use the new Azure DevOps instance.
After this we can enable the dogfooding comment again so people can test PRs easier.