Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 113 additions & 0 deletions json-c/json-c/json-c-0.18-20240915/JsonC_llar.gox
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import "os"
import "slices"

id "json-c/json-c"

// Serves the json-c 0.18 upstream tag family. The Conan recipe folder also
// covers 0.14-0.17, but those revisions use a different CMake build contract
// (no BUILD_STATIC_LIBS/DISABLE_STATIC_FPIC before 0.15, no BUILD_APPS and a
// 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"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// Package-owned choices from the Conan recipe: shared/static selection and,
// 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 {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"shared": "OFF",
"fPIC": "ON",
}

onBuild ctx => {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

installDir := ctx.outputDir

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


shared := slices.contains(target.options["shared"], "ON")

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// Conan removes fPIC when building the shared library; PIC is implicit
// for shared objects, so it only affects the static archive.
fpic := shared || slices.contains(target.options["fPIC"], "ON")

c := cmake.new(ctx.SourceDir, ctx.SourceDir+"/_build", installDir)
c.buildType "Release"

// Mirror the Conan CMakeToolchain variables for json-c >= 0.17.
c.defineBool "BUILD_SHARED_LIBS", shared
c.defineBool "BUILD_STATIC_LIBS", !shared
c.defineBool "DISABLE_STATIC_FPIC", !fpic
c.defineBool "BUILD_TESTING", false
c.defineBool "BUILD_APPS", false

c.configure

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

c.build
c.install

// Derive metadata from the installed json-c.pc rather than guessing flags.
c.use installDir
capout => {
exec "pkg-config", "--libs", "json-c"
}
if lastErr != nil {
panic lastErr
}
ctx.setMetadata output.trimSpace

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctx.setMetadatasetMetadata 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.

}

onTest ctx => {
installDir := ctx.outputDir

// Consumer mirrored from the Conan test_package: build a json object and
// serialize it, exercising the installed public headers and library.
testDir := ctx.SourceDir + "/_llartest"
os.mkdirAll(testDir, 0o755)!

src := `#include <json-c/json.h>
#include <stdio.h>
#include <stdlib.h>

int main(void) {
json_object *jobj = json_object_new_object();
json_object *jarray = json_object_new_array();
json_object_array_add(jarray, json_object_new_string("c"));
json_object_array_add(jarray, json_object_new_string("c++"));
json_object_object_add(jobj, "Categories", jarray);
printf("json object created: %s\n", json_object_to_json_string(jobj));
json_object_put(jobj);
return EXIT_SUCCESS;
}
`
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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// on a cache hit, where onBuild (and its _build tree) never ran.
c := cmake.new(ctx.SourceDir, testDir, installDir)
c.use installDir

capout => {
exec "pkg-config", "--cflags", "json-c"
}
if lastErr != nil {
panic lastErr
}
cflags := output.trimSpace

capout => {
exec "pkg-config", "--libs", "json-c"
}
if lastErr != nil {
panic lastErr
}
libs := output.trimSpace

bin := testDir + "/consumer"
exec "sh", "-c", "cc "+cflags+" "+testDir+"/consumer.c -o "+bin+" "+libs

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if lastErr != nil {
panic lastErr
}

exec bin
if lastErr != nil {
panic lastErr
}
}
4 changes: 4 additions & 0 deletions json-c/json-c/versions.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"path": "json-c/json-c",
"deps": {}
}
Loading