Skip to content

feat(chaiscript): add LLAR formula - #130

Open
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/108-chaiscript
Open

feat(chaiscript): add LLAR formula#130
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/108-chaiscript

Conversation

@MeteorsLiu

Copy link
Copy Markdown
Collaborator

Add an LLAR Formula for ChaiScript/ChaiScript from v5.0.0 through v6.1.0.

The implementation includes:

  • Set fromVer at the first no-Boost release after verifying the older Boost-dependent CMake contract.
  • Add an exhaustive comparator for the repository aliases and named snapshot tags, including wasm-latest.
  • Build and install ChaiScript modules, preserve the license, and make the installed pkg-config prefix relocatable.
  • Use the complete pkgconfig.lookup("chaiscript") metadata in both the Formula and independent C++ consumer test.
  • Validate v5.0.0, v6.1.0, default selection, fresh builds, and cache-hit consumers on Darwin arm64.

This completes the closed ChaiScript translation with explicit version ordering and relocatable consumer metadata.

Closes #108

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

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-")

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.

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"

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.

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)

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.

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.

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.

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",

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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Translate Conan Center chaiscript recipe to LLAR

1 participant