feat(libyaml): add LLAR formula - #169
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: libyaml 0.2.5 recipe
This recipe is well-constructed and closely follows the established repo conventions (streamvbyte, libde265, semver.c). The pkg-config .pc generation, license copy, and the CMake variables (YAML_STATIC_LIB_NAME, INSTALL_LIB_DIR, INSTALL_CMAKE_DIR) are the correct upstream libyaml settings, and the filter is if anything stricter/safer than the reference recipes. The -lyaml in the generated .pc is correct for both static and shared builds. No blocking issues.
A few low-severity / nit-level items are noted inline. The sprintf in the embedded C test is not a real bug — values come only from the hardcoded data[] table and a 32-bit int fits easily in the 64-byte buffer — so it's flagged only as optional hardening.
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "path": "yaml/libyaml", | |||
| "deps": {} | |||
There was a problem hiding this comment.
Nit — indentation. Other versions.json files in the repo (fast-pack/streamvbyte, madler/zlib, json-c/json-c) use tab indentation; this file uses two spaces. Worth normalizing to tabs for consistency.
| cc! consumer, "@${flagsFile}", "-o", binary | ||
|
|
||
| if slices.contains(target.options["shared"], "ON") { | ||
| os.setenv("LD_LIBRARY_PATH", filepath.join(installDir, "lib"))! |
There was a problem hiding this comment.
Low — loader path is overwritten, not prepended. os.setenv("LD_LIBRARY_PATH", ...) (and DYLD_LIBRARY_PATH on the next line) replace any inherited value rather than prepending the install lib dir. If the test env relies on an inherited loader path, the shared-build consumer could fail to resolve unrelated libraries. Consider prepending the existing value, e.g. filepath.join(installDir, "lib") + ":" + os.getenv("LD_LIBRARY_PATH"). Test-reliability only, not a security issue.
| libdir=$${prefix}/lib | ||
| includedir=$${prefix}/include | ||
|
|
||
| Name: yaml |
There was a problem hiding this comment.
Low — .pc Name/Description diverge from upstream. Upstream's own template (yaml-0.1.pc.in) uses Name: LibYAML and Description: Library to parse and emit YAML. Functionally harmless — pkgconfig.lookup("yaml") resolves by filename, not the Name field — but consumers inspecting pkg-config --description yaml will see different text. Consider aligning with upstream for accuracy. (Version: 0.2.5 is correct.)
| (yaml_char_t *)"count", strlen("count"), 1, 0, YAML_PLAIN_SCALAR_STYLE); | ||
| if (!yaml_emitter_emit(&emitter, &event)) goto error; | ||
|
|
||
| sprintf(buffer, "%d", f->count); |
There was a problem hiding this comment.
Info — optional hardening. sprintf(buffer, "%d", f->count) into the fixed 64-byte buffer is safe as written (values come only from the hardcoded data[]; a 32-bit int is ≤11 chars). No bug. If you want to future-proof against copy-paste reuse, snprintf(buffer, sizeof(buffer), "%d", f->count) is the trivial change.
1 participant
Closes #29
yaml/libyamlFormula for upstream tag0.2.5from the Conan Center snapshot.shared/fPICdefaults, CMake install layout, and theLicensefile.yaml.pcand compile the Conan yaml_emitter consumer with pkg-config flags.Validation:
git diff --checkpassed. Per task instruction, localllar testwas not blocked on.