Skip to content

ci: bump mordant to d0dca00 - #43805

Merged
alii merged 3 commits into
mainfrom
robobun/5c0dd972/bump-mordant-d0dca00
Sep 22, 2026
Merged

alii merged 3 commits into
mainfrom
robobun/5c0dd972/bump-mordant-d0dca00

Conversation

@robobun

@robobun robobun commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • The mordant job pins a1effd3. rename the cfg to mordant scarletindustries/mordant#28 renames the cfg that the driver sets while it lints: --cfg mordant now, --cfg dylint_lib="mordant" before.
  • mordant-baseline.toml holds one entry with no finding behind it: unused_pub:src/event_loop/ConcurrentTask.rs.

Fix

  • MORDANT_REV moves to d0dca0026f0662a94e929a8a697d711be5eafd9b. MORDANT_TOOLCHAIN stays nightly-2026-09-01. Nothing in this repository names dylint_lib or cfg(mordant), so no source file changes.
  • The regenerated baseline drops the stale entry and changes nothing else. The entry was ContextId::raw, which src/jsc/VirtualMachine.rs calls since event loop: remove ManagedTask; each callback is its own task type #43675.
  • Verified: with no baseline, a1effd3 and d0dca00 print the same 485 findings over the same tree (all lints, line by line). A cold run of the job's steps is clean locally.

Background

  • The baseline is a ratchet with a count per lint and file. A count above the real number is spare allowance: the job does not fail on it, and only a regeneration removes it.
  • A crate can allow a mordant lint only while mordant runs, with #[cfg_attr(mordant, allow(..))]. That is what the renamed cfg is for. This workspace does not use it, and --cfg mordant raises no unexpected_cfgs warning.
Notes

The mordant check can fail on this PR, and on any other, for a reason that is not in the diff. cargo mordant reports three associated constants of bun_sys::Tag (WriteFile, SetEndOfFile, fchownat) two or three times each. The unused_pub count for src/sys/lib.rs is 65 or 68 over the same tree, and the baseline records 65. The first run here counted 68.

The cause is in mordant. The definition path that identifies an item contains the index of its impl block. That index is {impl#9} on Linux and macOS and {impl#10} on Windows, because a cfg(windows) impl block sits above. cargo mordant sorts the findings of the three targets by position and drops a duplicate only when it is next to its twin. The order of equal positions is the order in which cargo finishes the targets, so the two equal keys are adjacent on some runs and not on others.

A commit on this branch recorded 68 by hand and was reverted: the baseline comes from a regeneration only. The fix is a mordant change that matches and deduplicates findings by position, and a later bump picks it up.


no test proof · iteration 0 · no src or test change; test-proof not applicable

From this revision the driver sets `--cfg mordant` and no longer sets
`--cfg dylint_lib="mordant"`. Nothing in this repository names either
cfg, so only the pin moves. Over the same tree the new revision prints
the same 485 findings as a1effd3, all lints compared line by line.

The regenerated baseline drops one stale entry that has nothing to do
with the new revision: `ContextId::raw` in
src/event_loop/ConcurrentTask.rs has a caller now
(src/jsc/VirtualMachine.rs), so it is no longer an unused_pub finding.
@robobun

robobun commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

  • Requested in chat by @alii after rename the cfg to mordant scarletindustries/mordant#28 merged.
  • Verified locally on this branch: with no baseline, a1effd3 and d0dca00 print the same 485 findings over the same tree (all lints, compared line by line).
  • bun scripts/rust-mordant.ts from an empty target/mordant is clean at the new revision (three targets).
  • bun run rust:mordant:baseline drops one entry, unused_pub:src/event_loop/ConcurrentTask.rs, which is stale on main because ContextId::raw has a caller now. Nothing else changes.
  • The mordant check on this PR runs the job at the new revision. It is the CI proof.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: cf2ac8f6-1eb1-4a58-850b-2f562b1ec0f0

📥 Commits

Reviewing files that changed from the base of the PR and between bf80d21 and 83b928a.

📒 Files selected for processing (2)
  • .github/workflows/rust-lints.yml
  • mordant-baseline.toml
💤 Files with no reviewable changes (1)
  • mordant-baseline.toml

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.


Walkthrough

The Mordant lint workflow now uses a new pinned revision. The Mordant baseline removes the unused_pub entry for src/event_loop/ConcurrentTask.rs.

Changes

Mordant lint maintenance

Layer / File(s) Summary
Update lint revision and baseline
.github/workflows/rust-lints.yml, mordant-baseline.toml
The workflow pin changes to revision d0dca0026f0662a94e929a8a697d711be5eafd9b. The baseline removes the [bun_event_loop] unused_pub entry.

Suggested reviewers: eastlondoner

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 83b92

The lint dependency update is ready to merge with no identified production or CI risk.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely identifies the main change: updating the Mordant CI dependency to revision d0dca00.
Description check ✅ Passed The description explains the problem, fix, background, affected files, and verification results. It does not use the template headings exactly, but it provides the required information and is sufficie…

Comment @coderabbitai help to get the list of available commands.

…han once

Three associated constants of `bun_sys::Tag` have one key on Linux and
macOS and another on Windows, because a `cfg(windows)` impl block shifts
the impl index. `cargo mordant` drops a duplicate finding only when it
sits next to its twin, so it reports each of the three items two or
three times, depending on the order in which cargo finishes the targets.
The count for src/sys/lib.rs is 65 or 68 over the same tree, and the job
failed when it came out as 68.

Record 68 until the pinned mordant matches findings by position.

@claude claude Bot 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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding on the bun_sys count, I also checked the dropped [bun_event_loop] baseline entry: ContextId::raw (src/event_loop/ConcurrentTask.rs:179) now has a caller at src/jsc/VirtualMachine.rs:1300, so removing it does not hide a finding. I could not confirm from this checkout that mordant's rust-toolchain at d0dca00 is still nightly-2026-09-01, which the workflow comment requires to be kept in sync with MORDANT_REV.

Extended reasoning...

The diff is CI-only: it bumps the pinned mordant revision in .github/workflows/rust-lints.yml and edits two entries in mordant-baseline.toml (one removed, one raised from 65 to 68). No runtime source or security-sensitive surface is touched. The removed entry is backed by a real caller in the tree; the raised count is covered by the inline finding, and the toolchain-sync claim could not be verified locally.

Comment thread mordant-baseline.toml Outdated

@claude claude Bot 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 — the hand-edited 68 count from my earlier comment is gone with the revert, and what remains is a plain tool-pin bump plus a baseline regeneration.

What was reviewed:

  • The dropped unused_pub:src/event_loop/ConcurrentTask.rs entry: ContextId::raw has a caller at src/jsc/VirtualMachine.rs:1300, so the finding is genuinely gone; the baseline only shrinks, which cannot loosen the ratchet.
  • MORDANT_REV change flows into the mordant-bin-<os>-<toolchain>-<rev> cache key, so no stale binary can be restored. I could not fetch mordant's rust-toolchain at d0dca00 from here, so the unchanged MORDANT_TOOLCHAIN pin rests on the PR's claim — a mismatch would surface as a visible cargo install failure on the advisory job, not a silent pass.
  • No CODEOWNERS entry covers either file; the reverted middle commit leaves no residue in the tree.
Extended reasoning...

The net diff is two CI-only lines: the pinned mordant commit in .github/workflows/rust-lints.yml and the removal of one stale [bun_event_loop] baseline entry in mordant-baseline.toml; no Rust source, scripts, or mordant.toml change. It touches no security-sensitive surface. The earlier inline finding (hand-raising src/sys/lib.rs to 68) was addressed by the revert commit, the removed baseline entry is backed by a real caller in src/jsc/VirtualMachine.rs, and the mordant job is continue-on-error so a wrong toolchain pin would fail loudly without blocking merges. The remaining 65-vs-68 nondeterminism is pre-existing in mordant, documented in the PR, and unchanged by this diff.

@alii
alii merged commit ce4d569 into main Sep 22, 2026
9 checks passed
@alii
alii deleted the robobun/5c0dd972/bump-mordant-d0dca00 branch September 22, 2026 19:31
alii pushed a commit that referenced this pull request Sep 22, 2026
…43810)

### Problem

- The `mordant` job passes or fails at random at the pinned `d0dca00`.
`cargo mordant` reports three associated constants of `bun_sys::Tag` two
or three times each, so the `unused_pub` count for `src/sys/lib.rs` is
65 or 68 over the same code. The baseline records 65, and the first run
on #43805 counted 68.
- The cause is in mordant. It identified an item by a key that contains
the index of its impl block. A `cfg(windows)` impl block shifts that
index on Windows only, and mordant dropped a duplicate finding only when
it sat next to its twin. The order of the three targets' records is the
order in which cargo finishes them.

### Fix

- `MORDANT_REV` moves to `00778d35391dc1a75381008754fd40726c1682f0`
(scarletindustries/mordant#29). It matches and reports an item by its
position: crate, file, offset of the name. `MORDANT_TOOLCHAIN` stays
`nightly-2026-09-01`.
- `mordant-baseline.toml` is regenerated and only goes down:
`src/sys/lib.rs` 65 to 56, and the entries for `src/io/lib.rs` (1) and
`src/spawn_sys/spawn_process.rs` (5) are gone.
- Verified: `bun scripts/rust-mordant.ts` is clean from an empty
`target/mordant`, and clean again on a warm run. The `mordant` check on
this PR is the CI proof.

### Background

- The job checks three targets in one run, so each `pub` item is
recorded up to three times. `cargo mordant` decides `unused_pub` after
the build, from all of those records.
- The baseline holds a count per lint and file. A run fails when a count
goes over.

<details><summary>Notes</summary>

What changes in the findings, measured over this tree with no baseline:

- 18 items are no longer reported. Each has a use under one target, and
the other targets' records of the same item had a different key, so the
use did not count for them. Examples: `Tag::epoll_ctl` (Linux),
`Tag::uv_spawn` (Windows), `FilePollRef::set_flag`, `ExtraPipe::fd`.
- The three `Tag` constants that were reported two or three times are
reported once.
- Six more reports appear, all in `src/sys/lib.rs`, for items that share
a name under different `cfg`s, such as `O::NOATIME`, which is defined at
three places. Each is a separate unused item. The old key was the same
for all of them, so they were merged into one finding.
- In total 188 reports become 173.

</details>

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 0 · no src or test change; test-proof not
applicable

<!-- robobun:evidence:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants