Skip to content

feat(miniscript): add LLAR formula - #184

Open
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/77-miniscript
Open

feat(miniscript): add LLAR formula#184
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/77-miniscript

Conversation

@MeteorsLiu

Copy link
Copy Markdown
Collaborator

Translate Conan Center miniscript to LLAR.

Closes #77

@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: JoeStrout/miniscript v1.6.2 formula

Reviewed the full diff (new package formula miniscript_llar.gox + versions.json) against sibling packages (atomicobject/heatshrink, rxi/microtar, pantor/ruckig, bdwgc/libatomic_ops) and the upstream MiniScript repo.

Overall: looks good. The formula is well-structured and follows established repo conventions — the defaults/filter option shape, license copy, manual header install, pkg-config .pc generation, and consumer test harness all match sibling patterns. Verified the load-bearing facts: the library target is miniscript-cpp (so -lminiscript-cpp is correct), upstream CMake does not install headers (so the manual header copy is required), MINISCRIPT_BUILD_TESTING/MINISCRIPT_BUILD_CSHARP are real options, the header source path is correct, and C++14 matches upstream.

No security or performance concerns. The target.options["shared"][0] direct indexing is a sanctioned repo convention (matches bdwgc/libatomic_ops and strukturag/libde265) since defaults guarantees a value.

Two minor, non-blocking notes are inline below.


if target.options["shared"][0] == "ON" {
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 (only if Windows shared builds are in scope): for a shared build, this sets LD_LIBRARY_PATH/DYLD_LIBRARY_PATH but does not prepend installDir/bin to PATH on Windows, where the DLL search relies on PATH. The rxi/microtar sibling handles this case (see microtar_llar.gox:113-116). If Windows shared builds aren't targeted for this package, this is fine and matches heatshrink/libatomic_ops, which also omit it.


Name: miniscript
Description: MiniScript embedded scripting language
Version: 1.6.2

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 maintainability note: the .pc Version is hardcoded 1.6.2, which is a second place to update alongside fromVer "v1.6.2" on a version bump and could silently drift. This is consistent with the rxi/microtar sibling (which also hardcodes), so it's acceptable; atomicobject/heatshrink instead derives the version dynamically from a source header if you prefer that pattern. No change required.

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

1 participant