Visitar URL original
ci: Fail when hand-written package versions drift from Directory.Packages.props by agneszitte · Pull Request #26329 · unoplatform/uno · GitHub
Skip to content

ci: Fail when hand-written package versions drift from Directory.Packages.props - #26329

Merged
agneszitte merged 6 commits into
masterfrom
dev/agzi/ci-package-version-alignment
Nov 12, 2026
Merged

agneszitte merged 6 commits into
masterfrom
dev/agzi/ci-package-version-alignment

Conversation

@agneszitte

@agneszitte agneszitte commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

GitHub Issue: related to #26165 (SkiaSharp 4.156.0) and #26325, which aligns what it missed

Note

Stacked on #26325: this check fails on master until #26325 is merged. Merge #26325 first; this PR then retargets to master.

PR Type:

🏗️ Build or CI related changes

What changed? 🚀

The nuspec dependency floors (build/nuget/*.nuspec) and the Uno.Sdk.Private manifest (src/Uno.Sdk/packages.json) repeat versions from src/Directory.Packages.props by hand. #26165 moved SkiaSharp to 4.156.0 but left 19 nuspec entries and the manifest on 4.151.1. An app whose managed and native SkiaSharp versions differ fails at startup ("The version of the native libSkiaSharp library (…) is incompatible with this version of SkiaSharp").

A new check in the Setup stage (build/ci/scripts/validate-package-version-alignment.ps1, no network, fails on every branch) now fails when:

Errors name the file and line, and the version expected. The Setup job's sparse checkout now includes src/Directory.Packages.props, and .claude/rules/build-system.md describes the rule.

The CI step lists the files it checks, with a comment on each, and passes them to the script: -Source (the versions Uno is built against), -Files (@{ Path; Kind }, Kind = NuspecFloor, SdkManifest or MSBuildArgs) and -FloorExceptions (empty). The defaults are the uno files, so a local run needs no arguments. Other repositories can reuse the script with their own list, for example uno.templates with the public Uno.Sdk manifest.

Validated locally
Tree Result
This branch (on #26325), and #26322's branch ✅ pass
origin/master today ❌ 26 errors: the 19 SkiaSharp nuspec entries and the manifest group #26165 missed, and the 6 Svg.Skia floors
origin/release/stable/7.0 (the 7.0.5 state) ❌ 6 errors: the Svg.Skia 3.0.6 floors behind #26332
One nuspec SkiaSharp entry left on 4.151.1 ❌ 1 error, that line
SkiaSharp.Skottie with a literal version + a stale manifest override ❌ caught
-p:SkiaSharpVersion= drift in the Android NativeAOT yml + a HarfBuzzSharp nuspec entry ❌ 2 errors, those lines
Microsoft.WindowsAppSDK bumped in Directory.Packages.props, nuspecs forgotten ❌ 4 errors, the 4 nuspec lines
Manifest WinAppSdk pin below the nuspec floor ❌ caught (NU1605)
Svg.Skia floor back to 3.0.6 ❌ caught
SkiaSharp.Skottie PackageVersion with reversed attributes, split over lines, literal version ❌ caught
Nuspec dependency with exclude="..." between id and version, stale version ❌ caught
-property:SkiaSharpVersion= / --property:HarfbuzzSharpVersion= drift ❌ 2 errors
Commented-out stale nuspec dependency ✅ ignored
PackageVersion without a Version ❌ fails loudly instead of being skipped
A consistent net11.0-only bump (nuspec net11.0 floor, conditional core Update, manifest net11.0 override) ✅ no false error
Manifest net11.0 override below the net11.0 floor ❌ 1 error, that override only
Commented-out stale <SkiaSharpVersion> above the real one ✅ ignored
The CI step's inline script, run as written ✅ passes
Unknown Kind, a typo in a manifest path, a missing -Source ❌ each fails with a clear message
#26322's 7.0 code + uno.templates' current 7.0 Uno.Sdk manifest as a second SdkManifest ❌ reports the SkiaSharp 4.151.1 and SvgSkia 3.0.6 pins the SR1 templates PR must move

PR Checklist ✅

  • 🧪 Added Runtime tests, UI tests, or a manual test sample: N/A, CI check (validated as above).
  • 📚 Docs have been added/updated: N/A (.claude/rules/build-system.md updated).
  • 🖼️ Validated PR Screenshots Compare Test Run results: N/A.
  • ❗ Contains NO breaking changes
  • 👀 Reviewed 2 other open pull requests (optional but appreciated!)

🤖 Generated with Claude Code

@github-actions github-actions Bot added area/build Categorizes an issue or PR as relevant to build infrastructure kind/documentation labels Nov 12, 2026
@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @agneszitte's task in 20s —— View job


Review of #26329: package version alignment check

Verdict: ship. I found no blocking issues, so I posted no inline comments.

Issue counts: 0 blocking, 0 major, 2 minor notes (below).

I read the script and wiring by inspection only. I did not run the PowerShell script or any CI job.

What I checked

  • Parsing of Directory.Packages.props: the script keeps one entry per package id, so duplicate or conditional PackageVersion entries would overwrite each other. I grepped the file and found neither today, so this is safe for now.
  • Nuspec and manifest checks: the floor-versus-build-version comparison, the documented Svg.Skia 3.0.6 exception, and the manifest-versus-nuspec NU1605 ordering all hold together. to-be-set* placeholders and non-numeric manifest versions are skipped.
  • Family check: the family match uses an exact id or a Prefix. match, so unrelated packages are not swept in. The -p:SkiaSharpVersion= scan also matches the HarfBuzz casing difference.
  • CI wiring: adding src/Directory.Packages.props to the sparse checkout is needed for the script to run. The new PowerShell@2 step passes -RepoRoot correctly, and ##vso[task.logissue] gives annotated errors on Azure DevOps.

Minor notes (non-blocking)

  1. Compare-PackageVersion splits on [-+], so build metadata (1.0.0+abc) would be treated as a prerelease label. NuGet ignores metadata when ordering. No manifest or nuspec uses metadata today.
  2. Compare-PackageVersion casts the numeric parts with [long], so a malformed version string would throw. That fails the step loudly rather than silently, which is acceptable. A clearer message naming the file would be friendlier.

Cross-cutting

@mergify

mergify Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

This pull request is merged.

@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @agneszitte's task in 15s —— View job


Review of #26329: package version alignment check

Verdict: ship. I found no blocking issues, so I posted no inline comments.

Issue counts: 0 blocking, 0 major, 2 minor notes.

I reviewed validate-package-version-alignment.ps1 by inspection only. I did not run it or any CI job.

The two minor notes from the earlier review are already handled in the current script:

  • Compare-PackageVersion ignores +metadata.
  • It throws a message naming the file and the offending versions when a version is malformed.

Minor notes (non-blocking)

  1. build/ci/scripts/validate-package-version-alignment.ps1:124 only reads a PackageVersion when Include= comes before Version= on a single line. A multi-line element, or one with the attributes reversed, is skipped silently. That pin would then go unchecked rather than fail. Today's file doesn't have this pattern. If you want it to fail loudly, count the <PackageVersion elements and compare with the number parsed.
  2. build/ci/scripts/validate-package-version-alignment.ps1:223 matches only -p: and /p:. -property:SkiaSharpVersion= or --property: would slip through. This is unlikely in the current scripts.

Cross-cutting

@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @agneszitte's task in 16s —— View job


Review of #26329: package version alignment check

Verdict: ship. I found no blocking issues, so I posted no inline comments.

Issue counts: 0 blocking, 0 major, 1 minor note.

I read build/ci/scripts/validate-package-version-alignment.ps1 by inspection only. I did not run the script or any CI job.

Earlier notes now handled

  • Get-XmlElements parses attributes in any order and across lines. It skips commented-out tags and reports unreadable PackageVersion or dependency entries as errors instead of ignoring them.
  • The -p: scan also matches /p:, -property: and --property:.
  • Compare-PackageVersion ignores +metadata and throws a message naming the context when a version is malformed.

Minor note (non-blocking)

  • Package versions in Directory.Packages.props are resolved only from the first <Prop>value</Prop> found in that file. A property redefined later or set conditionally would be read as its first value. No such property exists today, and the SkiaSharp and HarfBuzzSharp properties are looked up by name and fail loudly if missing.

Cross-cutting

@Xiaoy312 Xiaoy312 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice check. I ran it on this head: 105 references checked, all in sync.

The one change I'm asking for is the Svg.Skia exception comment (inline). It says Svg.Skia 3.0.6 is compatible, and I reproduced that it isn't on desktop. It's fine to keep the exception so master stays green, but the comment should call it a known break and link a tracking issue. The other three inline notes are minor or nits.

Process: Azure DevOps hasn't run on this PR yet because the base is #26325's branch. After the retarget to master, please check that the new Setup step actually runs.

@agneszitte
agneszitte added this pull request to stack #26330 November 12, 2026 03:51
@Xiaoy312
Xiaoy312 self-requested a review November 12, 2026 03:53
@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @agneszitte's task in 13s —— View job


Review of #26329: package version alignment check

Verdict: ship. I read the script and the setup step by inspection only. I did not run the script or any CI job. The comment you triggered me with gave no specific request, so I treated this as a review and changed nothing.

Issue counts: 0 blocking, 0 major, 1 open suggestion.

Xiaoy312's review points already handled in d64531a4

  • Svg.Skia exception: $KnownFloorExceptions is now empty. The comment says exceptions need a reason and a tracking issue, and it points to SVG images fail to load on desktop with Uno.Sdk 7.0.5 and the Svg feature #26332. The Svg.Skia floor is raised to 5.2.3 in 8b00643c.
  • succeededOrFailed(): the new Setup step has it, so it still reports when the network-based validate-packages-json step fails.
  • Per-framework manifest comparison: the commit message says manifest pins are now compared per target framework. I did not re-verify the net11.0-only bump case.

Still open, non-blocking

  • Comment-stripped property read: a commented-out <SkiaSharpVersion> above the real line would still be read as the property value. The script fails loudly in that case, so this is a nit. Reusing the comment-stripped text from Get-XmlElements would fix it.
  • Parameterised file list: the suggestion to pass the file list and exceptions in from the .yml is fine as a follow-up. It is not needed to merge.

Process

@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @agneszitte's task in 17s —— View job


Review of #26329: package version alignment check

Verdict: ship. I found no blocking issues, so I posted no inline comments. I read the diff and the script by inspection only. I did not run the script or any CI job.

Issue counts: 0 blocking, 0 major, 1 minor note.

What changed since the last review (bda2760a)

  • -Source, -Files and -FloorExceptions are now parameters. -Source is a separate argument, as Xiaoy312 suggested.
  • The Setup step in .azure-devops-setup-validate-packages.yml is now an inline pwsh: block that passes them. Each entry has a comment, and condition: succeededOrFailed() is kept.
  • The script rejects an unknown Kind, an entry without Path or Kind, and a Path that matches nothing. The one exception is an MSBuildArgs wildcard, which only prints a note.
  • The yml's own comment mentions -p:SkiaSharpVersion=, but the scan requires a digit after =, so the Setup step does not flag itself.
  • src/Directory.Packages.props is in the Setup job's sparse checkout, and build/** already was.

Minor note (non-blocking)

  • $self excludes only the script itself from the MSBuildArgs scan. It is not needed for the default file list, since the script contains no literal -p:...Version=<digit> text. It is harmless, but a short comment on why it is there would help.

Cross-cutting

@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @agneszitte's task in 10s —— View job


Review of #26329: package version alignment check

Verdict: ship. No blocking issues, so I posted no inline comments. I reviewed the changes by inspection only (the -Source/-Files/-FloorExceptions wiring in the Setup step, the sparse-checkout addition, and the build-system.md note). I did not run the script or any CI job.

Issue counts: 0 blocking, 0 major, 0 minor.

Status of earlier feedback

  • The latest commit (fec223de) adds the explanation for the self-exclusion and the duplicated file list. That covers the last open note from the previous review.
  • All of Xiaoy312's change requests are addressed: no Svg.Skia exception is left, condition: succeededOrFailed() is set, per-framework manifest comparison is in, -Source is a separate argument, and the file list comes from the pipeline.

Cross-cutting

@Xiaoy312 Xiaoy312 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

veri nice

Base automatically changed from dev/agzi/sdk-private-master-70-stables to master November 12, 2026 06:35
agneszitte and others added 6 commits November 12, 2026 01:35
…ages.props

The nuspec dependency floors and the Uno.Sdk.Private manifest repeat the
versions of Directory.Packages.props by hand, and a version bump can miss
them: the SkiaSharp 4.156.0 bump left 19 nuspec entries and the manifest
on 4.151.1, and an app whose managed and native SkiaSharp versions differ
fails at startup.

The new check, in the Setup stage, fails when:
- a nuspec dependency differs from the version Uno is built against
  (Svg.Skia 3.0.6 is a documented exception),
- a manifest pin is below the nuspec floor of the same package (NU1605),
- a SkiaSharp or HarfBuzzSharp version differs from SkiaSharpVersion /
  HarfbuzzSharpVersion in the nuspecs, the manifest or CI scripts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review notes on the version alignment check: NuGet ignores "+metadata"
when ordering versions, and a version that is not a NuGet version now
fails with the files being compared instead of a cast error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…le entries

Review notes on the version alignment check:
- <PackageVersion> and nuspec <dependency> elements are read whatever the
  attribute order or line breaks, and commented-out ones are skipped.
  Before, a dependency with exclude="..." between id and version, or a
  PackageVersion split over lines, went unchecked.
- PackageVersion Update items (conditional overrides) are read as allowed
  versions of their package, and a PackageVersion without an id or
  Version fails the check instead of being skipped.
- -property: and --property: are matched along with -p: and /p:.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…framework

Review notes on the version alignment check:
- The Svg.Skia 3.0.6 floor exception is gone: the floor now matches the
  5.2.3 build version (#26332). .NET binds by assembly version, so a
  floor below the build version fails to load at runtime even when the
  API exists; any future exception needs a tracking issue.
- A nuspec floor in a <group targetFramework="net11.0..."> is compared
  with the manifest net11.0 override when there is one, otherwise with
  the base version, so a consistent net11.0-only bump is not reported.
- Version properties are read with XML comments blanked out.
- The CI step also runs when the network-based check before it fails.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review suggestion on the version alignment check: the CI step now lists
the files it validates, with a comment on each, instead of the script
hard-coding them. The script takes:
- -Source: the file with the versions Uno is built against,
- -Files: @{ Path; Kind } entries, Kind being NuspecFloor, SdkManifest
  or MSBuildArgs (how to read the file; the script decides what to
  compare),
- -FloorExceptions: the allowed nuspec floors below the build version.

The defaults are the uno repository files, so a local run needs no
arguments. An unknown Kind, a missing -Source and a nuspec, manifest or
named file that matches nothing fail the check. Other repositories, such
as uno.templates with the public Uno.Sdk manifest, can now reuse it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review note on the version alignment check: say why the script skips
itself when it scans build/**/*.ps1, and that the -Files default (local
runs) and the list in the CI step must be kept in sync. Comments only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@agneszitte
agneszitte force-pushed the dev/agzi/ci-package-version-alignment branch from fec223d to cb84680 Compare November 12, 2026 06:35
@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @agneszitte's task in 10s —— View job


Review of #26329: package version alignment check

Verdict: ship. No blocking issues, so I posted no inline comments. I reviewed the changes by inspection only: the .claude/rules/build-system.md note and the Setup step wiring. I did not run the script or any CI job, and I did not re-read the full script in this pass.

Issue counts: 0 blocking, 0 major, 0 minor.

Status

  • The Setup step is an inline pwsh: block. It passes -Source, -Files and -FloorExceptions, with a comment on each entry.
  • $floorExceptions is empty, and the comment requires a reason and a tracking issue for any entry.
  • condition: succeededOrFailed() is set, so the step reports even when the network-based check before it fails.
  • The build-system.md addition is accurate and concise.
  • The commit messages follow Conventional Commits.

Cross-cutting

@unodevops

Copy link
Copy Markdown
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-26329/docs/index.html

@unodevops

Copy link
Copy Markdown
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here:

@agneszitte
agneszitte merged commit cca5c4f into master Nov 12, 2026
57 checks passed
@agneszitte
agneszitte deleted the dev/agzi/ci-package-version-alignment branch November 12, 2026 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/build Categorizes an issue or PR as relevant to build infrastructure kind/documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants