feat(chaiscript): add LLAR formula - #130
Conversation
There was a problem hiding this comment.
Review: ChaiScript Formula
Thanks for adding the ChaiScript Formula. The recipe follows the store's contract well in several respects: onBuild and onTest each run their own pkgconfig.use/pkgconfig.lookup so the published metadata is validated even on a cache hit, the consumer test tree (_llar_consumer) is kept separate from the build scratch tree (_build), and the error idiom (! / lastErr!) matches the style reference.
A few items to address, the first two being the most important:
Comparator does not produce a total order over the real tag set. Upstream ships both Release-X.Y.Z and vX.Y.Z tags for 21 versions (verified via git ls-remote). normalize() folds Release-X.Y.Z onto vX.Y.Z, so semver.Compare returns 0 for each of those 21 distinct pairs. The contract (references/contract-discovery.md:63, SKILL.md:98-101) requires distinct releases not to collapse to equality. See the inline note.
Test_Release maps to a version that does not exist upstream. No v5.7.2/Release-5.7.2 line exists (Release-* stops at 5.3.0, v* jumps to 6.x), so the mapping is a synthetic ordinal rather than a source-backed release line as the comment claims.
Remaining items (.pc $$ prefix — needs a verification of XGo string semantics; a comment/code mismatch on the license source; versions.json indentation) are inline.
| // build. | ||
| func normalize(version string) string { | ||
| if strings.hasPrefix(version, "Release-") { | ||
| return "v" + strings.trimPrefix(version, "Release-") |
There was a problem hiding this comment.
Distinct tags collapse to equality — comparator is not a total order.
Upstream has both Release-X.Y.Z and vX.Y.Z for 21 versions (e.g. Release-1.0.0 and v1.0.0, up through Release-5.2.0/v5.2.0). Stripping the Release- prefix maps each Release-X.Y.Z onto the identical string vX.Y.Z, so semver.Compare(normalize(a), normalize(b)) returns 0 for each of those 21 pairs.
Per references/contract-discovery.md:63 ("distinct ordered releases do not collapse to equality") and SKILL.md:98-101, the comparator must impose a total order over the complete tag set. Ties here make selection at any threshold landing on such a pair (including fromVer "v5.0.0", which collides with Release-5.0.0) nondeterministic. Consider a stable tie-break so the two spellings order consistently rather than comparing equal, and validate against fromVer and the version immediately below it.
| return "v" + strings.trimPrefix(version, "Release-") | ||
| } | ||
| if version == "Test_Release" { | ||
| return "v5.7.2" |
There was a problem hiding this comment.
Test_Release normalizes to a non-existent version. v5.7.2 (and Release-5.7.2) does not exist in the upstream tag set — Release-* stops at 5.3.0 and the v* line jumps from 5.8.x to 6.x. So this is a synthetic ordinal wedged between the 5.x and 6.x lines, not the "source-tree release line" the file comment (lines 4-5) claims. Please either back the mapping with evidence of the source line this snapshot actually points at, or handle/exclude the snapshot explicitly rather than assigning a fabricated semver.
| // before using the installed pkg-config contract for Formula metadata. | ||
| pcPath := filepath.join(installDir, "lib", "pkgconfig", "chaiscript.pc") | ||
| pc := string(os.readFile(pcPath)!) | ||
| pc = strings.replace(pc, "prefix="+installDir, `prefix=$${pcfiledir}/../..`, 1) |
There was a problem hiding this comment.
Verify the $$ in the .pc prefix produces a single literal $. pkg-config expects single-dollar ${pcfiledir} in the .pc file, and it treats $$ in a .pc as an escape for a literal $ — so if the bytes written to disk are $${pcfiledir}, pkg-config collapses them to the literal text ${pcfiledir} (never expanded), leaving a broken, non-relocatable prefix.
Whether the written bytes are $$ or $ depends on how XGo/ixgo treats $ inside a backtick raw string (the style reference documents $NAME env-expansion in gsh contexts, so $$ may be the intended escape to emit one $). Please confirm by inspecting the installed chaiscript.pc after a build and running the pkg-config lookup: if the file contains prefix=${pcfiledir}/../.. and resolves correctly, this is fine; if it contains prefix=$${pcfiledir}/../.., drop one $. (The ../.. depth is correct for lib/pkgconfig/.)
| stdlib := os.readFile(filepath.join(ctx.SourceDir, "_build", "libchaiscript_stdlib.so"))! | ||
| os.writeFile(filepath.join(installDir, "lib", "chaiscript", "libchaiscript_stdlib.so"), stdlib, 0o755)! | ||
|
|
||
| // Keep the license published by the Conan package. |
There was a problem hiding this comment.
Comment/code mismatch: this says "published by the Conan package," but the code reads license.txt from the upstream source tree (ctx.SourceDir) — nothing here involves Conan. Please reword to reflect the actual source.
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "path": "ChaiScript/ChaiScript", | |||
There was a problem hiding this comment.
Indentation nit: existing versions.json files in the store use tabs (see recp/cglm/versions.json, madler/zlib/versions.json); this one uses 2 spaces. Convert to tabs for consistency.
Add an LLAR Formula for
ChaiScript/ChaiScriptfromv5.0.0throughv6.1.0.The implementation includes:
fromVerat the first no-Boost release after verifying the older Boost-dependent CMake contract.wasm-latest.pkgconfig.lookup("chaiscript")metadata in both the Formula and independent C++ consumer test.This completes the closed ChaiScript translation with explicit version ordering and relocatable consumer metadata.
Closes #108