feat(heatshrink): add LLAR formula - #131
Conversation
There was a problem hiding this comment.
Review: add heatshrink formula + migrate zlib to new API
Nice, well-structured formula. I verified the new formula API (ctx.outputDir, ctx.Proj.readFile, ctx.setMetadata, pkgconfig.use/lookup) against goplus/llar@main, and the config-header/version parsing against upstream heatshrink v0.3.0 — the exact #define lines, the /* Version 0.3.0 */ marker, and the source file set all match. The .pc is correctly relocatable via ${pcfiledir}, versions.json is well-formed, and the onTest consumer probe cleverly branches on HEATSHRINK_DYNAMIC_ALLOC so it compiles under either alloc mode. Security and performance passes found nothing material.
A few non-blocking notes below (see inline comments), plus one file-level nit:
madler/zlib/1.0.0/Zlib_llar.goxfilename — the repo convention (formula-semantics.md: "Start every_llar.goxfilename stem with a lowercase ASCII letter") wantszlib_llar.gox. Since this migration already rewrites the file, it's a natural moment to rename it, and it would be consistent with the correctly-lowercased newheatshrink_llar.gox.
| version = line.trimSpace.trimPrefix("/* Version ").trimSuffix(" */") | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
If no line matches the /* Version prefix, version stays "" and the generated heatshrink.pc silently emits Version: with an empty value. For the pinned v0.3.0 tag the marker is guaranteed, so this is defensive, but a silently-malformed .pc is hard to diagnose if a future tag reformats the comment. Consider failing loudly (or falling back to the formula version) when the marker isn't found.
| lastErr! | ||
|
|
||
| os.setenv("LD_LIBRARY_PATH", filepath.join(installDir, "lib"))! | ||
| os.setenv("DYLD_LIBRARY_PATH", filepath.join(installDir, "lib"))! |
There was a problem hiding this comment.
Minor: filepath.join(installDir, "lib") is computed twice — extract a libDir local to keep both in sync. Also, these setenv calls overwrite rather than prepend, discarding any pre-existing LD_LIBRARY_PATH/DYLD_LIBRARY_PATH. Prepending (libDir + ":" + old when set) is safer for the test environment.
Summary
Closes #107
Validated locally with LLAR main b4164750da04f08c179965eb2f7b5087b1fc260c across all upstream tags, default/cache-hit paths, and pkg-config relocation.