Upgrade to Avalonia 12 - #549
Merged
Merged
Conversation
Nine package lines move together, because none of them builds without the others: the App's Avalonia core, Desktop, Themes.Fluent, Fonts.Inter and Controls.DataGrid to 12.1.2, AvaloniaEdit and AvaloniaEdit.TextMate to 12.0.0 (no 12.1.x exists), ScottPlot.Avalonia to 5.1.59, and the test project's Avalonia.Headless to 12.1.2. Nothing loud enforces that atomicity: every nuspec floor is minimum-only, so a partial bump restores and compiles green and then dies at the first HeadlessUnitTestSession.StartNew with an Avalonia-internals TypeLoadException. If that signature ever appears, check the two csprojs before suspecting anything else. TopLevel.PlatformSettings is removed in 12, and it is the only hard compile break in the whole project. The replacement is the VisualExtensions method, whose using was already present; it returns null when detached exactly as the old null-conditional chain did, so the Control fallback keeps its meaning. Baseline before the bump, for the zero-deleted gate: 713 tests total, 712 passed, 1 skipped (the pre-existing non-Windows Entra contract), 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
TextBox.Watermark is obsolete in 12 in favour of PlaceholderText, across 12 XAML sites and one code-behind constructor. The literal Watermark= rename is only safe because the app has no UseFloatingWatermark site; that one renames to UseFloatingPlaceholder, so nobody should generalize this recipe. The proxy input's parameter and the two prose comments that talk about "the watermark" move with it, because a control named one thing and described as another is how the next reader loses an afternoon. The CS0618 pragma in MainWindow wrapped nothing but a string array: the drag/drop migration moved the deprecated calls out from under it some time ago, and in 12 the APIs its comment names do not exist at all. Three comments stated mechanisms that 12 changed: - The ScottPlot line carried a do-not-bump-yet gate pointing at this very migration. It is executed, so the gate goes rather than being reworded. - The SkiaSharp pin's comment claimed it prevents issue #139. It never could: the collision is ScottPlot's NoDependencies package shipping its own libSkiaSharp.so, a different package id that a PackageReference on this one cannot constrain. Under 12 the pin is inert on top of that, since Avalonia.Skia already floors the same id at the same version. The comment now says what is true and why the pin stays anyway. - DataGrid 12.0.1 (PR #241) moved AllowAutoHide propagation out of OnApplyTemplate and into a template binding on each PART_ bar. The binding reads the same attached property off the same object, so App.axaml's rule still reaches the grids, but the reason it works is no longer the one the comment gave. The one-line assertion added to the DataGrid scrollbar test is the only falsifiable guard on the GetPlatformSettings migration: a manual Ctrl+C on Windows cannot tell a dead lookup from a live one, because the fallback the guard uses is Control and Control is what Windows reports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
Three things in HeadlessUi were written against 11 and are no longer true of the package it wraps. The reason for hand-rolling instead of using Avalonia.Headless.XUnit has expired without the conclusion changing: the package asked for xunit v2 when this was written and asks for xunit.v3.extensibility.core 3.2.2 now, which unifies upward against this project's 4.0.1. Adopting it is possible and still wrong, because [AvaloniaFact] offers neither the #474 drain nor the canary. Leaving the dead objection in place would have let the next reader check it, find it false, and switch on that basis. The #474 note claimed a mechanism 12 half-fixed. The FontManager is still disposed one line before ResetForUnitTests runs the queue, so the throw is exactly as available as it was and the drain is still the only thing that prevents it. What 12 changed is the aftermath: scope disposal moved into a finally and the failure is routed to the dispatch's task, so one test dies instead of every test after it. Stated that way round because the original prescription, read quickly, invites deleting the drain. Which leads to the guard on GetResult(). 12 reports a teardown failure through the dispatch's own task, awaited outside both catches, so it arrived as a throw from Dispatch rather than as a returned failure. Probed under 12.1.2 rather than reasoned about: a queued job that throws during teardown comes out of GetResult(), and when the body had already failed its assertion, the teardown exception is what the caller sees and the assertion message is gone. That inverts the precedence Run documents, so the wrap keeps the body's failure first. The metrics paragraph is new and is the one place this branch explains why layout numbers across six test files moved: 12 drops headless's stub shaper for real HarfBuzz over its own embedded font, which makes a character one em wide instead of a flat 10 DIP and a line 1.0898 em tall instead of 0.8. Every re-measured threshold carries its number and points here for the reason. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
Two assertions and four comments, all on the width axis, all re-measured rather than relaxed. At a 1520px viewport the session toolbar's overflow menu now holds four entries instead of three. The fourth is QS Overview, which is the next name in CollapseOrder, and no protected command moved: Connect, Execute and Execute-with-estimate are all still on the row, and every button in front of the tail is still at the same X. That is a wider row, not a different one. The numbers behind it, measured at 12.1.2: the row's natural width is 2116px where it was 1910, and the uncollapsible floor -- Connect, the server label, the database picker and the two plan verbs -- is 746px where it was 708. Both are about 1.1x, which is what FontSize / 10 comes to at the toolbar's 11px text. The chevron's 28px height did not move, because nothing about it is text. The DataGrid scrollbar comment is corrected on both halves, since both were wrong in different directions. The mechanism half described DataGrid assigning AllowAutoHide in code, which PR #241 replaced with a template binding in 12.0.1 -- the app's rule still reaches the bars, for a different reason than the comment gave. The premise half said the rows measure zero-high for want of a font. Under 12 there is a real font with real metrics and the rows still do not realize, so the comment now says what actually stops them, which is that the grid generates no columns at all. That is pre-existing and left alone here; this commit corrects the explanation, not the test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
The rewritten pin comment asserted that the NoDependencies package overwrites the right .so at publish time. That is the shape of issue #139, but it is not what this graph does, and a comment that states a failure mode as current when the branch's own publish check disproves it is the same defect the rest of this commit series is removing. Measured instead of assumed: a linux-x64 self-contained publish puts a libSkiaSharp.so that is SHA256-identical to SkiaSharp.NativeAssets.Linux 3.119.4's copy and different from the NoDependencies 3.119.0 one, alongside managed SkiaSharp 3.119.4. The deps file names only the 3.119.4 package as the native provider, though NoDependencies is still in the restore graph underneath ScottPlot. The point the comment exists to make is unchanged: the pin is not what decides that, because it cannot reach the other package id. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
The rewritten header said the row is narrower than either harness figure on real fonts. Nothing here measures that. The 1536-logical story is what the file is about and stands on its own; the two numbers are harness thresholds, so say that instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
The metrics paragraph gave 1.0898 em and then four whole-DIP measurements, which do not multiply out unless you know layout rounds the desired height up. Say that, so the next reader checking 1.0898 x 12 against the 14 in a test does not conclude one of them is wrong. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
Avalonia 12 hosts a Window inside a TopLevelHost, so a Window is no longer the root of its own visual tree. GetParentWindow read VisualRoot and cast it, which now always misses, and it threw "No parent window" for every dialog a query session owns: the connection dialog, the format dialog, the plan dialogs and the open-in-editor overwrite prompt. Three tests caught it; opening any of those four in the app would have. This is the second half of the same breaking change the DataGridBehaviors fix handled, and the more dangerous half. TopLevel.PlatformSettings was deleted, so the compiler stopped the build and the migration probe saw it. VisualRoot still exists, still compiles, and still returns an object -- just not the one the cast wants -- so nothing reports it until something asks for a window at runtime. Worth remembering the next time a green build is mistaken for a migrated one. TopLevel.GetTopLevel is what the rest of the app already uses for this, at thirteen sites including QueryStore.cs and QueryStoreHistoryControl, and it still resolves to the Window under the new hosting. Measured rather than assumed on an attached session: the visual chain now ends MainWindow < TopLevelHost, while GetTopLevel and FindAncestorOfType<Window> both return MainWindow and the clipboard still resolves through it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
…ctor Two comment strengthenings the numbers alone do not carry. The overflow assertion now records what is invariant versus what is merely current. How many commands sit in the menu at a given width follows the harness's text metrics and is expected to move with them. Which commands may leave at all does not: Connect, Execute and Execute-with-estimate are absent from CollapseOrder and must never appear in that menu at any width. The exact-collection assertion is what enforces it, which is the reason it stays an exact collection rather than a count. The metrics paragraph now says why the height factor is 1.36x and not the 1.25x someone will reach for: 1.25 is the em box alone, and this face carries a line gap on top of it. The font manager reports ascent 819, descent 205 and gap 92 over an em of 1024, which are the OS/2 typographic values rather than the hhea pair that would have meant 0.854 em. So 1.36x is the tell for a height that moved, and a height that moved by anything else has a different cause. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
Avalonia 12's headless bootstrap loses a race often enough to fail one arbitrary test in roughly a quarter of full-suite runs. It throws from Dispatcher.VerifyAccess while EnsureIsolatedApplication builds the per- dispatch Application: AvaloniaHeadlessPlatform.Initialize constructs a Compositor, and DefaultRenderLoop.Add finds a different thread owning the dispatcher. No code of ours appears above HeadlessUi.Dispatch in the stack. Measured before writing anything: 2 of 6 full-suite runs on this bump, 1 of 6 more with the two classes that were red for an unrelated reason excluded, and 1 of 6 with xunit parallelization disabled outright, which is what rules out test concurrency as the cause. The same measurement on 11.3.22 was 0 of 6. A different test is the victim every time. There is no newer Avalonia to take. So the dispatch is retried once, and only when the dispatched delegate was never entered. That is the whole safety argument: this does not re-run a test, it starts one that never ran. The started flag is the first statement inside the delegate, so "the body never began" is a fact rather than an inference, and the exception match is narrow on purpose -- the type AND one of the two upstream frames -- so a thread-affinity bug in our own code names our own frames and gets reported rather than papered over. One retry only; twice in a row and the run goes red. Every occurrence writes a line to stderr, because a retry nobody can see is how a 1-in-4 flake turns into a 1-in-400 mystery. Verified against the real race rather than a simulation of it: over twenty consecutive full runs the marker appeared three times and no run failed. Ten consecutive runs through dotnet test, the way CI runs it, were green at 713 tests. Remove it when an Avalonia release fixes the race. The removal condition and the evidence rates are in the doc comment, along with the ten-run check that makes deleting it falsifiable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
The retry's stderr line now points at the opened issue (#544) instead of a placeholder, and the harness doc's three metrics paragraphs are three sibling paragraphs rather than one nested inside another. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
|
Reviewed. This is a clean, well-scoped Avalonia 11→12 migration — no changes touch plan-XML parsing, T-SQL generation, or any SQL-adjacent code, so the injection/untrusted-input concerns that usually matter most here don't apply. Checked and found no issues:
No warnings-suppression additions, no version bump needed for this change, no PlanViewer.Web linked-include implications (no Core files added). Nothing to flag. |
Avalonia 12 completes a style-applied animation when its visual detaches - a handler its changelog does not document, outside the PlaybackBehavior pause guard - and re-attaching re-applies the style without resurrecting the finished animation. Switch tabs while a fetch or capture is running and its indeterminate bar comes back frozen mid-track, indistinguishable from a hang. Avalonia 11 had no detach handling at all, so the bar just kept sweeping. ProgressBarBehaviors.RestartOnReattach deactivates and reactivates the :indeterminate style one dispatcher hop apart on reattach, which spawns a fresh animation instance. Applied to all seven bars whose hosts detach mid-run: the two execution overlays, the actual-plan loading tab, and the QS grid, wait-stats, QS history and Overview loaders. Proven against the live symptom in a driven run, both directions: five window captures spanning a full animation cycle after a tab round-trip were pixel-identical without this, and sweep again with it. The headless suite cannot see animations, so that driven pair is the regression evidence. This was the plan's pre-classified bucket (b) item for the manual pass, found exactly where the review predicted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
Owner
Author
|
Post-review addition, pushed as 12f64dc: the driven GUI pass executed the plan's spinner check and hit the pre-classified bucket-(b) item exactly where the regression review predicted — Avalonia 12 completes style animations on detach (undocumented, outside the PlaybackBehavior guard), so a loading bar frozen mid-track after any tab switch during a capture. Fixed with ProgressBarBehaviors.RestartOnReattach applied to all seven at-risk bars; proven both directions in driven runs (five frames across a full animation cycle: pixel-identical before, sweeping after). Suite re-run green at 713. The drive also verified live on this branch: connection dialog through the fixed GetParentWindow path, the #542 database handover, placeholders, Alt mnemonics with underlines, editor typing/colorization/completion, clipboard copy via Get-Clipboard, scratch restore, and the error path. Items still needing human hands are unchanged: scrollbar rail geometry, chart box-select vs pan, middle-click pan, grid stripe under row recycling, sorts/resize. One pre-existing (11-reproduced) find filed separately: #550-adjacent focus-after-Ctrl+N, issue #551. |
|
Reviewed the diff against 87d7382 (dev tip). This is a pure Avalonia 11→12 migration touching only Checked and clean:
No correctness, security, or convention issues found. |
Owner
Author
|
SignPath check from the review (regression MIN1) is resolved — no action needed. The 'App' artifact configuration signs exactly one named PE, PlanViewer.App.exe, inside the zip; the second config signs the MSI container. Neither enumerates the payload, so the new Avalonia.HarfBuzz.dll passes through unsigned exactly like every Avalonia/SkiaSharp DLL already does in every shipped release — the strongest proof being history itself: releases have always contained dozens of unlisted DLLs, so unlisted files demonstrably do not fail the request. Avalonia 12 renames neither the exe nor the MSI. Nothing on the signing side gates this merge. |
2 of 3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Avalonia 11.3.22 → 12.1.2, with the coupled third-party releases: ScottPlot.Avalonia 5.1.59, Avalonia.AvaloniaEdit + AvaloniaEdit.TextMate 12.0.0, Avalonia.Controls.DataGrid 12.1.2, Avalonia.Headless 12.1.2. Migration only — no improvements rode along; the scope test applied throughout was "does Avalonia 12 make this wrong?"
Do not merge until the manual pass below is done. The suite proves a lot here, but three things are checkable only by hand.
App changes (the whole compile-and-runtime surface)
DataGridBehaviors.csmoves off the removedTopLevel.PlatformSettingsontogrid.GetPlatformSettings()(with a new in-suiteAssert.NotNullguard, since Windows Ctrl+C cannot falsify it — the fallback is also Ctrl). AndQuerySessionControl.GetParentWindow()moves offVisualRoot as Window— 12 hosts the Window under a newTopLevelHost, so the old form compiled, ran, and returned the wrong thing silently, which would have broken every session-owned dialog. That one no compile probe could catch; the headless suite did.Watermark→PlaceholderTextrenames (12 XAML + 1 C#), plus the same file's parameter and prose stragglers. Visual parity is settled by resource-set identity (Fluent ships the identical placeholder resources in 11 and 12; only unreferencedPART_names changed).AllowAutoHidecomment updated for 12's template-binding mechanism (upstream PR Await ShowDialog in Format_Click (fix CS4014) #241).Test suite
713 tests at baseline, 713 at tip — zero deleted, zero added, no new skips, no assertion softened (both exact-collection toolbar assertions stayed exact collections). The measurement churn comes from 12 replacing the headless stub text shaper (flat 10 DIP/char) with real HarfBuzz over an embedded font: widths now scale by FontSize/10, line heights by a flat ×1.36. That cause is written once, on
HeadlessUi, and every re-measured threshold points at it. The toolbar's protected-command invariant (Connect/Execute/Est never collapse) is now stated explicitly in the assertions that guard it.Harness: kept hand-rolled (12's xunit-v3 support removes the original reason but not the conclusion — the #474 drain and the survival canary don't exist in
[AvaloniaFact]). The #474 note now says precisely what 12 fixed (the scope leak) and didn't (the throw — the drain remains the only prevention). A teardown-failure precedence guard shipped after an empirical probe confirmed 12 routes teardown throws into the dispatch task, where they were replacing assertion messages.#544 — Avalonia 12's own headless setup has a thread-affinity race (~1 run in 4, one arbitrary victim, zero repo frames on the stack, not reproduced on 11, not caused by test concurrency — parallelization-off still hit it). Mitigated with a setup-scoped single re-dispatch that structurally cannot retry a test: it fires only when the dispatched delegate provably never started AND the stack matches Avalonia's setup frames, writes to stderr on every occurrence, and documents its removal condition. Proven engaging on the real race: the marker fired 3 times across 20 test-host runs, none failed. Gate: 10 consecutive complete green
dotnet testruns, plus 2 more by the reviewing orchestrator.Verification
libSkiaSharp.soSHA256-matchesSkiaSharp.NativeAssets.Linux 3.119.4and differs from ScottPlot's NoDependencies copy — issue [BUG] Application Does not open on Linux #139 does not reproduce on this graph.Before merge (manual, ~20 minutes, installed or clean Release build only)
The twelve-point pass in the plan doc, headlined by the three things the suite structurally cannot see: loading-spinner animation (12 newly completes style animations on detach — undocumented), the #464 scrollbar rail geometry, and AvaloniaEdit highlighting across repeated tab switches (the 12.0.0-editor-on-12.1.2-core skew). Plus Alt mnemonics (12 matches layout symbols now) and the clipboard guards.
Follow-ups filed
#544 (the race, with removal condition), #545 (drop unused Grammars package), #546 (TextMate installation leak in schema viewers), #547 (pre-existing mnemonic eating the escape-brackets label's underscore), #548 (the columns-less DataGrid test). Before the first 12-based release: confirm SignPath's 'App' artifact configuration signs by glob —
Avalonia.HarfBuzz.dllis new in the publish set. After merge: dispatch nightly.yml once and eyeball the linux artifact.macOS double-click activation remains the standing open question; it rides with the next Mac session, now alongside the Option-mnemonic check.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX