Add json-c/json-c LLAR Formula (Conan 0.18 translation) - #122
Add json-c/json-c LLAR Formula (Conan 0.18 translation)#122fennoai[bot] wants to merge 1 commit into
Conversation
Translate the Conan Center json-c recipe into an idiomatic LLAR Formula serving the upstream tag json-c-0.18-20240915. - CMake build mirroring the Conan CMakeToolchain variables for json-c >= 0.17: BUILD_SHARED_LIBS/BUILD_STATIC_LIBS from the `shared` option, DISABLE_STATIC_FPIC from `fPIC`, plus BUILD_TESTING=OFF and BUILD_APPS=OFF. - No dependencies (verified against the upstream CMakeLists; threading is optional and off by default). - Metadata derived from the installed json-c.pc via pkg-config. - onTest compiles and runs the Conan test_package consumer in an independent tree so it also passes on a build cache hit. - fromVer pinned to the verified 0.18 tag; earlier tags in the same Conan folder use a different build contract and are intentionally not claimed. Closes #39 Co-authored-by: fennoai[bot] <231223108+fennoai[bot]@users.noreply.github.com> Co-authored-by: MeteorsLiu <17515813+MeteorsLiu@users.noreply.github.com>
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
FennoAI Review — json-c formula
Thanks for the well-documented formula — the Conan-derived option mapping, the honest fromVer rationale, and deriving link flags from the installed json-c.pc via pkg-config are all solid.
My main concern is a cluster of constructs that don't appear in the documented formula API (.claude/skills/write-formula/SKILL.md) or in the only existing example (madler/zlib/1.0.0/Zlib_llar.gox):
onBuild ctx => {...}uses a single lambda parameter; the documented signature isonBuild (ctx, proj, out) => {}and every example routes output throughout(out.setMetadata,out.addErr).ctx.outputDiris read as a bare property; the API definesctx.outputDir()returning(string, error).ctx.setMetadata— metadata is documented onBuildResult(out.setMetadata), not onContext.defaults { ... },target.options[...], andonTesthave no entry in the "Complete API Reference".
I could not find the formula runtime source in this repo to confirm these either way. If they rely on a newer runtime API than SKILL.md documents, please point to it and the API-mismatch inline comments can be dismissed — but note the c.configure/build/install error-handling issue below holds regardless, and if SKILL.md is stale it should be updated so it no longer claims to be complete.
See inline comments for specifics. Severity ranges from likely-blocking (API mismatch, dropped build errors) to low (comment accuracy, shell hardening).
| // forced CMP0042/policy-min before 0.17). Only the 0.18 revision was verified, | ||
| // so this is the honest fromVer boundary. Add a lower threshold with its own | ||
| // formula when an earlier range is actually verified. | ||
| fromVer "json-c-0.18-20240915" |
There was a problem hiding this comment.
fromVer "json-c-0.18-20240915" uses a full upstream tag string as the version boundary. SKILL.md describes fromVer as the minimum version served and the existing example uses a plain 1.0.0. json-c-0.18-20240915 is not semver/dpkg-comparable, so the default gnu.Compare may not order it as intended. Consider adding a json-c_cmp.gox version comparator (see SKILL.md "Two Classfiles") and confirm how the resolver compares this tag.
| // for the static library, whether position-independent code is emitted. The | ||
| // upstream CMakeLists defaults both libraries ON; Conan builds exactly one, | ||
| // defaulting to the static library. | ||
| defaults { |
There was a problem hiding this comment.
defaults { ... } is not a documented ModuleF top-level DSL method (documented: id, fromVer, matrix, onRequire, onBuild). SKILL.md's "XGo Pitfalls" #1 also warns that a bare {...} block at statement level is ambiguous with a lambda. If defaults is a supported runtime API, please confirm; otherwise this needs reworking.
| "fPIC": "ON", | ||
| } | ||
|
|
||
| onBuild ctx => { |
There was a problem hiding this comment.
onBuild ctx => {...} uses a single-parameter lambda. The documented signature is onBuild (ctx, proj, out) => {} → OnBuild(func(*Context, *Project, *BuildResult)), and the zlib example uses all three params. Without out in scope there's no documented way to call out.setMetadata / out.addErr (see lines 53 and the panics below). Please restore the three-parameter signature and route output through out.
| } | ||
|
|
||
| onBuild ctx => { | ||
| installDir := ctx.outputDir |
There was a problem hiding this comment.
ctx.outputDir is used as a bare property returning a single value. The API defines ctx.outputDir() as OutputDir__0() (string, error) — call it with parentheses and handle the error, e.g. installDir, err := ctx.outputDir(); if err != nil { out.addErr err; return } (cf. zlib lines 6-10). Same at line 57.
| onBuild ctx => { | ||
| installDir := ctx.outputDir | ||
|
|
||
| shared := slices.contains(target.options["shared"], "ON") |
There was a problem hiding this comment.
target.options["shared"] / target.options["fPIC"] (line 29) reference a global target that isn't in the documented API (ModuleF/Project/Context/BuildResult/ModuleDeps). If reading build-option values this way is supported by the current runtime, please confirm; otherwise there's no documented mechanism for it.
| c.defineBool "BUILD_TESTING", false | ||
| c.defineBool "BUILD_APPS", false | ||
|
|
||
| c.configure |
There was a problem hiding this comment.
c.configure / c.build / c.install are invoked as bare command-style statements, silently discarding their error returns. The zlib example error-checks every one (lines 14-28). As written, a failed configure/build/install would proceed silently and produce a broken or empty install. Capture and handle each error (e.g. err = c.configure(); if err != nil { out.addErr err; return }). This holds regardless of the API-signature question.
| if lastErr != nil { | ||
| panic lastErr | ||
| } | ||
| ctx.setMetadata output.trimSpace |
There was a problem hiding this comment.
ctx.setMetadata — setMetadata is documented on BuildResult (out.setMetadata), not on Context. Use the out parameter (see line 23). Also note panic lastErr (lines 50-52) diverges from the repo convention of out.addErr err; return used by zlib and every SKILL example for recoverable build errors.
| os.writeFile(testDir+"/consumer.c", []byte(src), 0o644)! | ||
|
|
||
| // Point pkg-config at the installed result. This env change is all the | ||
| // test needs, so the consumer builds in its own tree and works identically |
There was a problem hiding this comment.
Comment accuracy: this says "the consumer builds in its own tree", but the consumer is compiled by a raw cc call at line 104, not cmake. cmake.new(ctx.SourceDir, testDir, installDir) (line 84) is constructed only so c.use installDir (line 85) sets PKG_CONFIG_PATH/CPPFLAGS/LDFLAGS; the testDir build-dir arg is never used and no cmake configure/build runs. The cache-hit conclusion is correct (onTest reads only installDir), but reword to reflect that there's no cmake build tree, or drop the unused cmake.new/testDir argument.
| libs := output.trimSpace | ||
|
|
||
| bin := testDir + "/consumer" | ||
| exec "sh", "-c", "cc "+cflags+" "+testDir+"/consumer.c -o "+bin+" "+libs |
There was a problem hiding this comment.
This assembles a shell command by concatenating unquoted values into sh -c: cflags/libs (pkg-config output) and testDir/bin (derived from ctx.SourceDir). A space or shell metacharacter in the workspace path (or .pc contents) would break the compile or be interpreted by the shell. Low severity under the build-tooling trust model (inputs aren't remote-attacker-controlled), but it's the lone deviation from the safe argv-form exec used elsewhere (lines 48, 88, 96, 109). Prefer argv form: exec "cc", <cflags split...>, testDir+"/consumer.c", "-o", bin, <libs split...>, or at least shell-quote the interpolated paths.
1 participant
Summary
Translates the Conan Center
json-crecipe into an idiomatic LLAR Formula, addingjson-c/json-cto llarhub. Serves the upstream tagjson-c-0.18-20240915(Conan version0.18).Closes #39
Files
json-c/json-c/versions.json— module metadata, no static deps.json-c/json-c/json-c-0.18-20240915/JsonC_llar.gox— the Formula.Translation notes
CMakeToolchainvariables for json-c ≥ 0.17 —BUILD_SHARED_LIBS/BUILD_STATIC_LIBSfrom thesharedoption,DISABLE_STATIC_FPICfromfPIC, plusBUILD_TESTING=OFFandBUILD_APPS=OFF. Callsconfigure/build/installdirectly (they panic on failure).CMakeLists.txtat the tag — threading is optional and off by default; nofind_packagefor external libraries. So noonRequire.shared(defaultOFF) andfPIC(defaultON), matching the Conandefault_options. Package-owned, declared viadefaults{}.json-c.pcviapkg-config --libs(-L<lib> -ljson-c) rather than guessing from the Conanpackage_infoblock.test_packageconsumer (json_object_*+ serialize) in an independent tree underctx.SourceDir, resolving flags fromjson-c.pc, so it passes identically on a build cache hit (whereonBuildnever runs).fromVerboundary: pinned to the verifiedjson-c-0.18-20240915tag. The Conan folder also covers 0.14–0.17, but those revisions use a different build contract (noBUILD_STATIC_LIBS/DISABLE_STATIC_FPICbefore 0.15; noBUILD_APPSand a forcedCMP0042/policy-min before 0.17). Only 0.18 was inspected, so earlier tags are intentionally not claimed.Validation
Run with
llarv0.3.0. All succeed:llar test ./json-c/json-c@json-c-0.18-20240915onTest)--os linux --arch amd64--os linux --arch amd64 --option shared=ON--os linux --arch amd64 --option fPIC=OFFllar test ./json-c/json-c@json-c-0.17-20230812no formula found) — confirms thefromVerboundaryonTest output:
json object created: { "Categories": [ "c", "c++" ] }