feat(cpp-optparse): add LLAR formula - #159
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: weisslj/cpp-optparse formula
Solid addition that follows the repo's formula conventions closely (defaults/filter/onBuild/onTest, LICENSE vendoring, -lm fallback parity, shared-build LD_LIBRARY_PATH/DYLD_LIBRARY_PATH handling). The full-SHA pin is the correct supply-chain posture, and the untagged-commit comment is accurate. A few points below, one of which can cause real link failures on some platforms.
Verified non-issues: the consumer program's use of cout/endl is fine — OptionParser.h transitively includes <iostream>; versions.json matches the schema of existing formulas; error handling (! / lastErr!) is consistent with the reference formulas.
|
|
||
| install(TARGETS OptionParser | ||
| RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} | ||
| LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} |
There was a problem hiding this comment.
[P1] lib vs lib64 install-layout mismatch can break linking
CMakeLists.txt installs via include(GNUInstallDirs) using ${CMAKE_INSTALL_LIBDIR} (lines 4, 11-12), which resolves to lib64 on many 64-bit distros (RHEL/Fedora/openSUSE). But the build metadata (-L.../lib, line 80) and the test link flags (-L.../lib, line 101) — plus the shared-build LD_LIBRARY_PATH/DYLD_LIBRARY_PATH (lines 113-114) — hardcode lib. On a lib64 platform the archive/library installs to lib64 while consumers search lib, producing link/runtime failures.
The json-c formula avoids exactly this by forcing c.define "CMAKE_INSTALL_LIBDIR", "lib" with an explicit comment. Since this formula ships its own CMakeLists.txt, the simplest fix is to set CMAKE_INSTALL_LIBDIR=lib in the CMake invocation (or drop GNUInstallDirs and hardcode lib/include/bin destinations).
| if slices.contains(target.require["os"], "linux") || slices.contains(target.require["os"], "freebsd") { | ||
| metadata <- "-lm" | ||
| } |
There was a problem hiding this comment.
[P2] -lm is appended but cpp-optparse has no libm dependency
-lm is added on Linux/FreeBSD in both onBuild (lines 83-85) and onTest (lines 104-106), but cpp-optparse uses no libm functions — OptionParser.{h,cpp} at this commit only pull in <string>, <vector>, <iostream>, <sstream>, etc., with no pow/sqrt/floor/log. This is harmless (the linker ignores an unneeded -lm) but misleading, and it diverges from repo convention: the cglm and json-c formulas justify their -lm against a real math dependency in comments. Consider dropping -lm, or adding a short comment explaining why it is retained for parity.
| "fPIC": "ON", | ||
| } | ||
|
|
||
| filter => { |
There was a problem hiding this comment.
[P2] filter accepts contradictory fPIC=OFF + shared=ON combination
defaults declares both shared and fPIC (lines 39-42) and filter accepts any independent ON/OFF combination. In the Conan model fPIC is dropped when shared=True (shared implies PIC). Here fPIC=OFF + shared=ON is accepted and passes CMAKE_POSITION_INDEPENDENT_CODE=OFF to a shared build, producing a redundant/contradictory variant that either fails or is silently overridden. Consider normalizing (force fPIC when shared) or documenting that the combination is intentionally allowed.
| onBuild ctx => { | ||
| installDir := ctx.outputDir | ||
|
|
||
| cmakeLists := ctx.Proj.readFile("2ec0b7aca9a692ff93017ed44ca9d13a8e7d4d00/CMakeLists.txt")! |
There was a problem hiding this comment.
[P3] Comment density below repo convention
The reference formulas (cglm, json-c) document their build contract, option semantics, and why flags/build files are derived a certain way. This formula has a single comment. Two behaviors in particular would benefit from a short note consistent with convention: why a CMakeLists.txt is injected into the source tree (upstream ships no CMake build — the reason for vendoring one, line 61-62), and the shared/fPIC defaults reasoning (lines 39-42). Non-blocking.
1 participant
Summary
weisslj/cpp-optparseat Conan Center's pinned commit2ec0b7aca9a692ff93017ed44ca9d13a8e7d4d00shared/fPICoptionslibmmetadataValidation
git diff --checkllar testnot run per request