docs: name the forge-std prerequisite in the README Install section - #117
docs: name the forge-std prerequisite in the README Install section#117thedavidmeister wants to merge 4 commits into
Conversation
The published package ships no foundry.toml, soldeer.lock or remappings.txt, and soldeer writes remappings only for a project's own direct dependencies, so a consumer gets an unresolvable `forge-std-1.16.1/src/Vm.sol` import the moment they import anything from this package. State the version and the prefix as an install prerequisite, and pin the claim to foundry.toml with a test. Closes #82 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 59 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Delete `test/README.t.sol` and revert the `{ access = "read", path =
"README.md" }` `fs_permissions` grant it needed. The README Install section
edit — the actual fix for #82 — is unchanged.
The deleted test's subject was a TOML file and a markdown file, not any code in
`src/`. `vm.contains` on prose passes if the substring appears anywhere in the
file, so it could not establish that the Install section is correct; it only
stood in the way of the next README edit. The check is rehomed to
rainlanguage/rainix#317 as a `rainix-static` subcommand.
`foundry.toml` is now byte-identical to `main`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Closing: this documents how soldeer works, which is not this README's subject. Every fact the added section carries is a property of soldeer, identical for
None of it is specific to It is also unnecessary, by the mechanism this PR's own text quotes. Soldeer's The prose is the copy that rots. #126 bumps forge-std to #82 is closed with the same reasoning. The branch is left in place as the audit trail. |
Closes #82
What a consumer actually needs
1.16.1— the pin infoundry.toml[dependencies],matching
soldeer.lock. Stale soldeer pin: foundry.toml pins forge-std 1.16.1 with 1.16.2 published, and the version is baked into every import prefix #90 owns whether that pin is stale; this PR documentswhat is pinned today and does not move it.
forge-std-1.16.1/—src/lib/LibCodeGen.sol:5,src/lib/LibFs.sol:5andsrc/lib/LibHexString.sol:5each importforge-std-1.16.1/src/Vm.sol.forge soldeer install forge-std~1.16.1in theconsuming project writes exactly
forge-std-1.16.1/=dependencies/forge-std-1.16.1/toremappings.txt, whichis what makes the prefix resolve.
The issue's conclusion, re-derived
The issue concluded the README is the place to fix this, on the grounds that a
sibling package ships no
foundry.tomleither, so shipping none is convention.I checked that and then checked whether the package route could work at all.
rain-math-float~0.1.7does ship nofoundry.toml,soldeer.lockorremappings.txt— confirmed against the published zip. But "sibling does thesame" is a weaker argument than the mechanism, so I measured the mechanism.
Reproduced the consumer failure against the published package in a scratch
foundry project (
rain-sol-codegen~0.1.12, 21 zip entries, none of themfoundry.toml/soldeer.lock/remappings.txt):Then confirmed the documented prerequisite is sufficient:
Then tested the package route directly, by hand-building the exact tree a
recursive soldeer install would produce — forge-std under
dependencies/rain-sol-codegen-0.1.12/dependencies/forge-std-1.16.1/, plus aremappings.txtinside the dependency directory naming the prefix:A dependency's own
remappings.txtis not consulted; only the consumingproject's root remappings count, and soldeer writes those only for direct
dependencies.
forge soldeer install --helpalso confirms--recursive-deps/[soldeer] recursive_depsis a flag in the consuming project, off bydefault (this repo sets it
falseexplicitly) — so a package-side manifestcould not guarantee resolution even if it were shipped.
So the conclusion holds, on a stronger basis than convention: there is no
package-side artifact that can supply this prefix. The README is the only place
it can live, and the README is shipped in the published zip, so the
prerequisite reaches the consumer either way.
Change
README.md(+16) — the Install section now names the version, the prefix, theinstall command, the remapping it generates, and the compiler error you get
without it.
That is the entire diff. No test, and
foundry.tomlis byte-identical tomain.No test guards this, and it can regress silently
This change is guarded by nothing. If someone bumps
[dependencies] forge-stdinfoundry.toml— which #90 is open to do — the README will keepnaming
1.16.1andforge-std-1.16.1/, the suite will stay green, and everyconsumer following the Install section will install the wrong forge-std and hit
exactly the
Source "forge-std-1.16.1/src/Vm.sol" not foundcompile error thissection exists to prevent. The same is true of any later README edit that drops
or rewords the paragraph. The regression is silent in CI and only surfaces in a
downstream project.
That is deliberate, and it replaces an earlier version of this PR. This PR
originally shipped
test/README.t.sol(37 lines) plus a{ access = "read", path = "README.md" }fs_permissionsgrant. Both are removed here. Thecheck is rehomed to rainlanguage/rainix#317, which collects the six
hand-rolled Solidity static-analysis harnesses this fleet produced and asks for
them as
rainix-staticsubcommands in Rust, with bats coverage, inherited byevery consumer through the rainix reusables instead of copied per repo.
Cause, stated plainly: the fixer brief driving this fleet said "every fix ships
a test, verify-by-hand is never acceptable", with no exemption for changes that
have no behaviour. That instruction was wrong and has been corrected. A README
paragraph has no behaviour, and the test it forced was a bad check on its own
terms — its subject was a TOML file and a markdown file, not any code in
src/,and
vm.containson prose passes if the string appears anywhere in the file.It could not establish that the Install section is correct, only that two
substrings exist somewhere in the README. What it actually did was fight the
next README edit.
Note for #90 supersedes the one this PR carried before: bumping the forge-std
pin no longer fails anything here. Whoever does it has to update this README
paragraph by hand, or wait for the rainix#317 check to land.
QA
origin/mainin (merge commit, not a rebase); the branch predated Unlink a symlink with no target before writing the generated file #110and Declare every tooling builder
viewso an implementation can read state #124.stated as an install prerequisite; both are covered, plus the install command,
the remapping it generates, and the compiler error without it. The issue's own
conclusion (README not package) was re-derived from measurement rather than
assumed — see above. The forge-std version being stale is Stale soldeer pin: foundry.toml pins forge-std 1.16.1 with 1.16.2 published, and the version is baked into every import prefix #90 and is
deliberately not touched here.
Full suite and formatting from the flake shell (
nix develop -c), on the mergecommit before and after the strip:
(Wall/CPU differ between the two runs only because the machine was running
other builds concurrently; the pass/fail counts are the measurement.)
144 → 142 is exactly the two functions in the deleted
test/README.t.sol(
testReadmeNamesPinnedForgeStdVersion,testReadmeNamesForgeStdImportPrefix);19 → 18 suites is that file's
ReadmeTestcontract. Nothing else moved.Post-
Build.sol-removal sweep (2026-08-17)main(959d527) merged in. Unaffected by #138's removal ofscript/Build.sol— this PR is one paragraph in the Install section and touchesnothing the removal deleted. Nothing cut.
Re-verified against
mainrather than restated:src/lib/LibCodeGen.sol:5,src/lib/LibFs.sol:5,src/lib/LibHexString.sol:5each still import the literal
forge-std-1.16.1/src/Vm.sol— 3 of 3..soldeerignoreonmainstill excludes/foundry.toml,/remappings.txtand
/soldeer.lock, so the published package still ships no remapping thatcould supply that prefix. Issue The published rain-sol-codegen package ships no dependency metadata, and the README never names the forge-std version its imports require #82's premise holds.
remappings.txtonmainis exactly the one lineforge-std-1.16.1/=dependencies/forge-std-1.16.1/, which is the line thisparagraph tells a consumer they will get.
Coupled to #126. #126 bumps forge-std
1.16.1→1.16.2and rewrites thatimport prefix. Whichever of the two lands second has to carry the other's
version: if #126 lands first, the three
1.16.1strings added here become1.16.2. Stated rather than pre-applied, becausemainis on1.16.1today anda README that names a version the source does not import would be wrong now
instead of later.
Suite on the merge commit:
Ran 19 test suites: 145 tests passed, 0 failed, 0 skipped (145 total tests).forge fmt --checkclean,git statusclean afterthe run.