feat(libipt): add LLAR formula - #149
Conversation
There was a problem hiding this comment.
Review: add intel/libipt Formula (v2.0.1)
The recipe is well-structured and follows the LLAR Formula contract: id/fromVer, defaults, filter, onBuild, onTest. versions.json matches the sibling format. onTest correctly re-derives paths from installDir (ctx.outputDir) so it survives a cache hit where onBuild is skipped. Nice.
I checked several things that turned out to be fine, noting them so they aren't re-raised:
-liptis correct. Upstreamlibipt/CMakeLists.txtnames the targetlibiptwithPREFIX "", so the installed artifact islibipt.so/libipt.a.-liptmakes the linker searchlibipt.*→ matches. (-llibiptwould searchliblibipt.*and fail.)- The lowercase filename
libipt_llar.goxis correct. The repo'swrite-formulaskill mandates a lowercase filename stem ("Do not capitalize it"). The capitalized siblings (Cglm_llar.gox, etc.) predate/diverge from that rule. - No pkg-config path is expected. libipt v2.0.1 installs no
.pcfile, so hand-built-I/-L/-lflags are the correct choice here.
One low-severity robustness note is inline. A couple of optional polish items:
- Comments (optional): Unlike the
cglm/json-csiblings, this recipe has no explanatory comments (build contract,shared/fPIC→BUILD_SHARED_LIBS/CMAKE_POSITION_INDEPENDENT_CODE, ON/OFF values). A short header would aid maintenance, thoughzlibis also comment-free so this isn't universal. consumer.cppcompiled withc++: The consumer uses only the C API; compiling it as C++ works but unnecessarily requires a C++ toolchain.cc+.cwould match thecglmsibling.
No blocking issues found.
|
|
||
| if slices.contains(target.options["shared"], "ON") { | ||
| 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.
os.setenv overwrites LD_LIBRARY_PATH/DYLD_LIBRARY_PATH rather than prepending to any existing value. If the test environment already relies on these paths (e.g. for the C++ runtime or another dependency), overwriting them could break loader resolution for the consumer binary. Prepending the install lib dir to the existing value is the safer idiom, e.g. os.getenv("LD_LIBRARY_PATH") joined with :. Low severity, but worth considering.
Closes #89
intel/libiptFormula using the upstream CMake build and consumer check.sharedandfPICoptions, installsLICENSE, and publishes-liptmetadata.v2.0.1as the lowest compatible tag because earlier releases hard-code a shared target.Tests were not run per request.