Skip to content

fix: build with Rust 1.99, which deprecates the legacy f64 constants and fetch_update - #6507

Merged
andygrove merged 1 commit into
apache:mainfrom
andygrove:fix/rust-1-99-deprecations
Oct 1, 2026
Merged

andygrove merged 1 commit into
apache:mainfrom
andygrove:fix/rust-1-99-deprecations

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

No issue is filed. Rust 1.99.0 was released on 2026-10-01, and the rust-test job has failed on
every pull request and merge queue run since about 12:41 UTC, for example in
run 36862898765.

Rationale for this change

CI installs the latest stable toolchain (RUST_VERSION: stable in pr_build_linux.yml), and
rust-test runs cargo clippy --all-targets --workspace -- -D warnings. Rust 1.99.0 fully
deprecates the legacy numeric module constants and deprecates Atomic*::fetch_update, so the
warnings fail the job:

  • 29 errors in the test module of spark-expr/src/conversion_funcs/numeric.rs, such as
    use of deprecated constant std::f64::NAN: replaced by the NAN associated constant on f64
  • 4 errors in core/src/execution/memory_pools/:
    use of deprecated method std::sync::atomic::Atomic::<usize>::fetch_update: renamed to try_update for consistency

What changes are included in this PR?

  • The test module in numeric.rs imported core::f64, so its f64::NAN, f64::INFINITY,
    f64::MAX and the other constants named the deprecated module constants. Without the import they
    are the associated constants on f64. The tests are otherwise unchanged.
  • CometUnifiedMemoryPool::{grow, shrink, try_grow} and SparkMemory::repay keep fetch_update
    under #[allow(deprecated)]. Its replacement, try_update, was stabilized in Rust 1.95, which is
    newer than the workspace rust-version of 1.94, so switching would raise the minimum Rust version,
    and clippy's incompatible_msrv lint would reject it. The attribute goes on each method because
    three of the calls sit in if let conditions, where it can't go on the expression.

How are these changes tested?

This changes no behavior, so there are no new tests.

  • I could not install Rust 1.99 locally, so this PR's own rust-test run is the check on 1.99. Cargo
    stops at the first crates that fail, so the failing runs did not lint every target. If 1.99 flags
    anything else, it shows up here.
  • On Rust 1.97, cargo clippy --all-targets --workspace -- -D warnings and cargo fmt --check are
    clean, and the memory_pools and conversion_funcs::numeric tests pass.
  • A grep of native/ and contrib/ finds no other legacy numeric module constants and no other
    fetch_update calls.

…and fetch_update

Rust 1.99.0, released 2026-10-01, deprecates the legacy numeric module constants and
`Atomic*::fetch_update`. CI installs the latest stable toolchain, so `cargo clippy -- -D warnings`
in the rust-test job has failed on every pull request and merge queue run since.

- A test module in `numeric.rs` imported `core::f64`, so its `f64::NAN`, `f64::INFINITY` and the
  other constants named the deprecated module constants. Without the import they are the
  associated constants on `f64`.
- The four `fetch_update` calls in the memory pools keep it under `#[allow(deprecated)]`. Its
  replacement, `try_update`, was stabilized in Rust 1.95, newer than the workspace `rust-version`
  of 1.94.
@github-actions github-actions Bot added bug Something isn't working area:expressions Expression evaluation area:memory Memory pools, reservations, OOM handling labels Oct 1, 2026

@coderfender coderfender 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.

LGTM

@andygrove
andygrove enabled auto-merge October 1, 2026 14:32
@andygrove
andygrove added this pull request to the merge queue Oct 1, 2026
Merged via the queue into apache:main with commit 75d7c7a Oct 1, 2026
41 checks passed
Owie6789 added a commit to better-Shape/app that referenced this pull request Oct 24, 2026
Rust Lints (fmt, clippy) has been red on this branch since Rust 1.99.0 reached
the runner images, on a line no commit here touched:

  error: use of deprecated method `Atomic::<u64>::fetch_update`:
         renamed to `try_update` for consistency
    --> src/core/turn_order.rs:13:10

The line is byte-identical at the branch point ae262b9. The cause is a
floating toolchain: no rust-toolchain.toml, `toolchain: stable` in
.github/actions/setup/action.yml, and `-D warnings` in ci.yml. ci.yml does not
run on pushes to mega/absorb-all, so this PR is the first CI run ever to lint
that branch's Rust and it surfaced a failure that was already sitting there.

The rename clippy suggests is not available to this crate. try_update is stable
only from 1.95.0, while src-tauri/Cargo.toml declares rust-version = "1.89.0"
and engineering/rust-toolchain.md:69-70 records the vendored sandbox toolchain
as 1.89.0 to match, as is [email protected]'s own rust-version. Renaming would
break the declared MSRV, break the vendored toolchain agents fetch in blocked
sandboxes, and be rejected by clippy's own incompatible_msrv lint. It trades a
deprecation error for a compile error on 1.89 through 1.94.

So the attribute goes on the one function that needs it, scoped to that
function rather than the module or the crate, with the removal condition in the
doc comment. In std, fetch_update is an alias whose body is
self.try_update(set_order, fetch_order, f), so the two are semantically
identical and behaviour does not change. This is what sub-1.95 projects did
upstream: valkey-glide#7253, apache/datafusion-comet#6507,
cheerc/agend-terminal#4, sturnus-dev/sturnus#80 and google-cloud-rust#7108 all
suppressed and deferred rather than raised their floor.

Raising the MSRV and pinning the toolchain were both considered and rejected
here. Raising it invalidates the 4 GB vendor branch that cannot be regenerated
from a sandbox. Pinning is the real root-cause fix, because a pinned toolchain
cannot drift under a -D warnings gate, but it is a repo-wide policy change that
interacts with dtolnay/rust-toolchain's explicit `toolchain: stable` and needs
someone who can run cargo to verify it. Both are recorded with their tradeoffs
in .dump/global/floating-rust-stable-breaks-ci-with-no-commit.md so the exposure
is not re-diagnosed by the next person, and so the pinning decision gets made
deliberately instead of by accident.

Co-authored-by: arena-agent <[email protected]>
Owie6789 added a commit to better-Shape/app that referenced this pull request Oct 24, 2026
* installer: replace the animated NSIS sidebar with the static image

The welcome page sidebar is now the plain 164x314 BMP that
bundle.windows.nsis.sidebarImage already names. Tauri maps it to
MUI_WELCOMEFINISHPAGE_BITMAP on its own (installer.nsi:42 and 127-129), so no
hook and no plug-in are involved and both the welcome and finish pages keep
showing it.

Removed: hooks.nsh, sidebar.gif, the vendored AnimGif plug-in and its license,
and the installerHooks key. The animation cost a 3.9 MB committed asset and a
3.9 MB extraction into $PLUGINSDIR on every visit to the welcome page, carried
an unsigned 2010 third-party binary into the installer, and could not be made
safe without forking that binary. AnimGif ends its worker with TerminateThread
and then deletes GDI resources the killed thread may still be inside
(winimage.cpp:236-240), and its header check compares two bytes against "GIF"
under a comment claiming GIF87a and GIF89a (winimage.cpp:301), so a payload
that decodes to no frames made play return silently after the hook had already
cleared MUI's bitmap.

tests/release/installer-art.test.ts now parses the BMP header, pins its
checksum, checks the config path resolves, and asserts that no hook, GIF or
plugins directory comes back. The directory listing is what stops a vendored
binary being re-added without a recorded decision.

This does not sign the installer and it is not a fix for an unsigned build
being refused by Windows. The research behind that, the diagnostic that would
confirm which mechanism is involved, and the signing options with their
eligibility constraints are in
.dump/app/research/windows-installer-static-sidebar-2026-10-24.md.

 

* docs: record the PR base lesson and the hook removal's compile safety

Three records from taking PR #52 through review.

The PR opened against draft/mega-absorb-all, copied from a sibling PR, when
this branch was cut from ae262b9 at the tip of mega/absorb-all. The two bases
are 547 commits apart, so the same head reported 2539 changed files, was
unmergeable, and invalidated four automated reviews: CodeAnt skipped over the
100-file threshold, Sourcery refused past the API's 300-file diff limit,
DeepSource reviewed the whole divergence, and SonarCloud's new-code period
covered files this PR never touched, so its Quality Gate failure looked
introduced here when none of it was. Recorded in
.dump/global/pr-base-is-a-property-of-the-branch.md with the two commands that
settle a base before opening, and with the finding that retargeting a base
fires only `edited`, which bare `pull_request:` triggers do not listen to.

No makensis compile was available for the hook removal, so it was proved from
source instead and the proof is recorded in the research file. MUI dispatches a
page callback inside `!ifdef MUI_PAGE_CUSTOMFUNCTION_${TYPE}`
(Pages.nsh:105-112), the template includes a hook only under
`{{#if installer_hooks}}` (installer.nsi:28-29), and nothing else consumes what
the hook defined, so removing it emits no dangling reference.

The README gains the one fact that changes how the art is maintained: MUI runs
`!pragma verifyloadimage` on the sidebar (Welcome.nsh:45-46), so makensis
validates the BMP at compile time and a corrupt image fails the build rather
than installing blank.

 

* fix: allow the fetch_update deprecation the MSRV cannot adopt yet

Rust Lints (fmt, clippy) has been red on this branch since Rust 1.99.0 reached
the runner images, on a line no commit here touched:

  error: use of deprecated method `Atomic::<u64>::fetch_update`:
         renamed to `try_update` for consistency
    --> src/core/turn_order.rs:13:10

The line is byte-identical at the branch point ae262b9. The cause is a
floating toolchain: no rust-toolchain.toml, `toolchain: stable` in
.github/actions/setup/action.yml, and `-D warnings` in ci.yml. ci.yml does not
run on pushes to mega/absorb-all, so this PR is the first CI run ever to lint
that branch's Rust and it surfaced a failure that was already sitting there.

The rename clippy suggests is not available to this crate. try_update is stable
only from 1.95.0, while src-tauri/Cargo.toml declares rust-version = "1.89.0"
and engineering/rust-toolchain.md:69-70 records the vendored sandbox toolchain
as 1.89.0 to match, as is [email protected]'s own rust-version. Renaming would
break the declared MSRV, break the vendored toolchain agents fetch in blocked
sandboxes, and be rejected by clippy's own incompatible_msrv lint. It trades a
deprecation error for a compile error on 1.89 through 1.94.

So the attribute goes on the one function that needs it, scoped to that
function rather than the module or the crate, with the removal condition in the
doc comment. In std, fetch_update is an alias whose body is
self.try_update(set_order, fetch_order, f), so the two are semantically
identical and behaviour does not change. This is what sub-1.95 projects did
upstream: valkey-glide#7253, apache/datafusion-comet#6507,
cheerc/agend-terminal#4, sturnus-dev/sturnus#80 and google-cloud-rust#7108 all
suppressed and deferred rather than raised their floor.

Raising the MSRV and pinning the toolchain were both considered and rejected
here. Raising it invalidates the 4 GB vendor branch that cannot be regenerated
from a sandbox. Pinning is the real root-cause fix, because a pinned toolchain
cannot drift under a -D warnings gate, but it is a repo-wide policy change that
interacts with dtolnay/rust-toolchain's explicit `toolchain: stable` and needs
someone who can run cargo to verify it. Both are recorded with their tradeoffs
in .dump/global/floating-rust-stable-breaks-ci-with-no-commit.md so the exposure
is not re-diagnosed by the next person, and so the pinning decision gets made
deliberately instead of by accident.
 

* docs: record the aggregate-gate and macOS DMG bundling lessons

Three markdown records under .dump/, no code change.

global/a-red-aggregate-check-does-not-tell-you-what-it-aggregates.md
records that CI Success is reddened only by the jobs in its needs list
(ci.yml:399), which excludes the browser jobs, so the Playwright and
Browser Preview failures cannot be its cause. Head a1d0076 is the
control: three checks red, aggregate green. The root cause of the
mistake was describing an aggregate gate by counting the red checks
beside it instead of reading the job definition.

app/research/macos-dmg-bundle-fails-after-a-successful-compile-2026-10-24.md
records the macos-x86_64 Build Installer failure at bundle_dmg.sh, the
four discriminators that make it a packaging flake rather than a
regression, that a flaked job cannot be re-run with this token because
both rerun endpoints return 403, and that check-run annotations carry
the tail of the failing step when the job log blob host is unreachable.

global/floating-rust-stable-breaks-ci-with-no-commit.md gains the
resolution section: option 1 was applied at a1d0076 and is verified by
CI's Rust Lints job, because the 1.88 toolchain from the @rustbin npm
scope predates the 1.99 deprecation and cannot reproduce it locally.

The gate is green on this tree: lint, typecheck for both projects,
skills:verify 53 of 53, and vitest at 213 files and 2367 tests.

 

* fix(a11y): restore the /git stage checkbox safe target size

On /git, axe reported two target-size nodes where e2e/a11y-baseline.json
allows one, which is the only thing failing Test Browser (Playwright) and
Browser Preview. The regression is the stage checkbox in the Source file
list, not the generate-commit button that the baseline pins.

The checkbox has always been 11.38px and has always passed WCAG 2.2
SC 2.5.8 on the spacing branch rather than on size. Commit 0bb0559
replaced the file row's single clickable wrapper div with a
role="presentation" wrapper plus a real sibling button for the open-diff
action, which made the row keyboard accessible and put an axe target
immediately to the checkbox's right. Its siblings had not been targets
before: the icon wrapper is pointer-events-none and the path and status
labels are plain text.

The safe clickable diameter is the checkbox plus twice the gap to that
button. Centred in a w-5 wrapper it has 2.435px either side, and mr-1
adds 3.25px, so 11.38 + 2 x 5.685 = 22.75px, the 22.8px axe reported.
mr-1.5 adds 4.875px instead, giving 7.31px and 26.0px. The icon wrapper
two lines below already uses mr-1.5, so the row keeps one spacing
vocabulary, and the content shifts 1.625px right.

Enlarging the checkbox is not available: the row is h-7, 22.75px.
Raising the baseline to 2 would be altering a test to pass, and the
baseline file says to lower numbers as violations are fixed.

Verified locally against the static bundle with the CI-identical command,
playwright test --grep-invert @visual with E2E_STATIC=1: 22 passed,
0 failed, and /git now reports exactly one target-size node, .size-7,
which is the node the baseline recorded at d49de66. No visual baseline
covers /git; e2e/visual.spec.ts-snapshots holds welcome, settings and
workbench only. The full gate is green on this tree: lint, typecheck for
both projects, skills:verify 53 of 53, and vitest at 213 files and 2367
tests.

 

* docs: record the target-size regression and the sandbox Playwright harness

Two research records under .dump/, no code change.

app/research/git-stage-checkbox-target-size-regression-2026-10-24.md
records the root cause, that adding an interactive element next to a
small one violates target-size without touching the small one, the four
hypotheses eliminated by comparing the tree at the baseline commit
against HEAD, the worktree method that identified which node the
baseline actually pinned, and the two wrong diagnoses that preceded it,
including why a diff between two divergent branches is not a
before-and-after.

app/research/playwright-in-a-cdn-blocked-sandbox-2026-10-24.md records
that the e2e suite is runnable here after all. The @sparticuz/chromium
npm tarball carries a 209 MB Chromium 153 ELF plus the NSS/NSPR
libraries in al2023.tar.br, Node decompresses brotli with no extra
binary, and playwright.config.ts already resolves both through
sandboxChromium() and PLAYWRIGHT_SANDBOX_LIBS. It also records that the
automatic path is required over PLAYWRIGHT_CHROMIUM_PATH because only
the automatic one supplies --no-sandbox.

 

* docs: record the design gates and the capture attachment gap for the UI fix

One decision record under .dump/app/decisions/, no code change.

ui-change-design-gates-and-capture-gap-2026-10-24.md reports the seven
delivery gates of engineering/design-skills.md for the one-margin UI
change in the previous commit, names the skills loaded and the ones
declined with reasons, records the baseline mirror check including the
mr-sm deviation considered and not taken, and records the capture:
330 frames at 30fps, validate verdict PASS_WITH_WARNINGS, watched frame
by frame, cursor verified on the control.

It also records the one gap. AGENTS.md requires attaching the recording
to the pull request while ui-capture requires keeping the artifacts out
of Git, which .gitignore:56 enforces. No upload channel satisfies both
from this sandbox: gh gist create refuses binary files and GitHub's
comment attachment endpoint is the web UI's. The clip stays at
.qa-artifacts/capture/out.gif and the body describes the frames read,
so the claim does not rest on a file a reviewer cannot open.

 

* docs: address CodeRabbit findings in engineering records
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:expressions Expression evaluation area:memory Memory pools, reservations, OOM handling bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants