Skip to content

feat(heatshrink): add LLAR formula - #131

Open
MeteorsLiu wants to merge 2 commits into
xgo-dev:mainfrom
MeteorsLiu:issue/107-heatshrink
Open

feat(heatshrink): add LLAR formula#131
MeteorsLiu wants to merge 2 commits into
xgo-dev:mainfrom
MeteorsLiu:issue/107-heatshrink

Conversation

@MeteorsLiu

Copy link
Copy Markdown
Collaborator

Summary

  • add atomicobject/heatshrink Formula from v0.3.0 through v0.4.1
  • translate CCI CMake build and Conan options
  • publish relocatable heatshrink.pc metadata through pkgconfig.lookup
  • add consumer verification and license installation

Closes #107

Validated locally with LLAR main b4164750da04f08c179965eb2f7b5087b1fc260c across all upstream tags, default/cache-hit paths, and pkg-config relocation.

@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: 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.gox filename — the repo convention (formula-semantics.md: "Start every _llar.gox filename stem with a lowercase ASCII letter") wants zlib_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 new heatshrink_llar.gox.

version = line.trimSpace.trimPrefix("/* Version ").trimSuffix(" */")
break
}
}

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.

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

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.

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.

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 heatshrink recipe to LLAR

1 participant