Add LLAR Formula for coin-or/qpOASES (releases/3.2.1) - #119
Conversation
Translate the Conan Center `qpoases` recipe into an idiomatic LLAR Formula. - fromVer "releases/3.2.1": exact upstream tag for Conan version 3.2.1 (all qpOASES tags are `releases/X.Y.Z`; spelling preserved). Only this version is served by the recipe, so no version range is claimed. - No dependencies: the Conan recipe declares no requirements/tool_requires/ test_requires, confirmed against the upstream CMakeLists. - onBuild: CMake build of the static library. Sets QPOASES_BUILD_EXAMPLES=OFF (matching the recipe), CMAKE_POLICY_VERSION_MINIMUM=3.5 (upstream declares cmake_minimum_required(VERSION 2.6)), and carries over the Conan `fPIC` option via CMAKE_POSITION_INDEPENDENT_CODE. Installs lib/libqpOASES.a and the include/ headers; metadata "-lqpOASES". - fPIC modeled as target.options (default ON); filter rejects fPIC=ON on Windows, where the recipe deletes the option. - onTest: builds a minimal QProblem consumer (derived from the Conan test_package) in an independent tree so it also passes on a cache hit, linking the installed library and asserting a successful solve. Verified end-to-end against the real `releases/3.2.1` source (configure/ build/install produce the expected layout; the consumer compiles, links, and solves returning SUCCESSFUL_RETURN). `llar test` itself cannot run this formula because llar v0.3.0 rejects version tags containing `/` when naming the source temp dir (os.MkdirTemp pattern) -- an upstream toolchain limitation, not a formula defect. Co-authored-by: fennoai[bot] <231223108+fennoai[bot]@users.noreply.github.com> Co-authored-by: MeteorsLiu <17515813+MeteorsLiu@users.noreply.github.com>
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Add LLAR Formula for coin-or/qpOASES
Reviewed the two files in this PR (coin-or/qpOASES/releases/3.2.1/QpOASES_llar.gox, coin-or/qpOASES/versions.json) with quality, performance, security, and documentation passes.
The formula is well-structured and correctly adopts the context-hook API: onTest builds the consumer in an independent tree so it passes on both cache-miss and cache-hit, examples are disabled to minimize build work, and versions.json is correctly dependency-free. Performance and security passes found no issues.
The main finding is a build-blocking correctness bug in the Windows filter branch, plus a few maintainability/robustness points, left inline.
Summary of findings
- [High]
filterrejects the default Windows matrix — see inline at thefilterblock. The defaultfPIC=ONmakes every default Windows build fail the filter, which contradicts the stated Conan "delete the option on Windows" intent. - [Medium]
fPICoption value is not validated; non-ON/OFFspellings are silently treated as OFF. - [Low] Consumer metadata
-lqpOASESomits the-I/-Linclude/lib paths a consumer needs; the passingonTestdoes not prove metadata completeness because it discovers paths viafind_path/find_library. - [Low]
onTestexecs a POSIXconsumerpath, which won't run on the very platform thefilterspecial-cases.
One scoping note worth confirming: fromVer "releases/3.2.1" should match the exact upstream tag spelling — please verify the qpOASES upstream ref is literally releases/3.2.1 and not 3.2.1, since it drives source checkout and Formula selection.
| // the Conan recipe deletes the fPIC option there. Reject fPIC=ON on Windows. | ||
| filter => { | ||
| if slices.contains(target.require["os"], "windows") { | ||
| return slices.contains(target.options["fPIC"], "OFF") |
There was a problem hiding this comment.
[High] The default Windows matrix is rejected — this contradicts the stated intent and blocks the default Windows build.
defaults sets fPIC: "ON", so a plain Windows build has target.options["fPIC"] == ["ON"]. This branch then returns slices.contains(["ON"], "OFF") → false, which rejects the entire effective matrix (a false filter stops before dependency discovery). So llar make on Windows with default options cannot build at all — a consumer must explicitly pass fPIC=OFF.
The comment says this "matches the Conan recipe," but Conan's del self.options.fPIC on Windows removes the option so the build proceeds normally without any fPIC input — it does not require fPIC=OFF. The reference guidance is to use filter only to reject a selection "proved unsupported by the source"; fPIC=ON is not unsupported (CMake simply ignores CMAKE_POSITION_INDEPENDENT_CODE on Windows).
Suggested fix: drop the OS special-case in filter (return true on Windows) and, if desired, gate only the c.defineBool "CMAKE_POSITION_INDEPENDENT_CODE" call on non-Windows — rather than failing the matrix on its own default value.
| c.defineBool "QPOASES_BUILD_EXAMPLES", false | ||
|
|
||
| // Carry over the fPIC option for the static library. | ||
| c.defineBool "CMAKE_POSITION_INDEPENDENT_CODE", slices.contains(target.options["fPIC"], "ON") |
There was a problem hiding this comment.
[Medium] The fPIC option value isn't validated. On non-Windows the filter accepts any value unconditionally, and this line maps fPIC via slices.contains(target.options["fPIC"], "ON"). If a user passes e.g. fPIC=true, the check is false and the build silently produces a non-PIC library instead of rejecting the invalid request. Consider validating the fPIC value in filter so unsupported spellings are rejected rather than silently reinterpreted.
|
|
||
| // Installed interface: static lib libqpOASES.a in lib/, headers in | ||
| // include/ (qpOASES.hpp + the qpOASES/ directory). | ||
| ctx.setMetadata "-lqpOASES" |
There was a problem hiding this comment.
[Low] Consumer metadata likely omits paths a consumer needs. setMetadata "-lqpOASES" provides only the link name — a consumer compiling against qpOASES also needs the header include path and library search path (e.g. -I{install}/include -L{install}/lib). Per the metadata guidance, include install-directory paths when consumers need them (LLAR rewrites the prefix to its portable placeholder at install time). Note the onTest below passing does not prove this metadata is complete: it locates the headers/lib via tc.use installDir + find_path/find_library, not via this metadata string. Please confirm a bare consumer using only -lqpOASES can actually compile and link.
| tc.build | ||
|
|
||
| // Run the freshly built consumer. A non-zero exit surfaces via lastErr. | ||
| exec testBuild + "/consumer" |
There was a problem hiding this comment.
[Low] Platform-assuming executable path. exec testBuild + "/consumer" assumes a POSIX executable name; on Windows the artifact is consumer.exe. This is currently masked because the default Windows matrix is rejected by filter, but if that filter issue is fixed to allow Windows, this test path will break. Worth making the consumer path platform-aware or documenting the test as non-Windows only.
Closes #14.
Translates the Conan Center
qpoasesrecipe into an idiomatic LLAR Formula.What was added
coin-or/qpOASES/versions.json— module metadata, no static deps.coin-or/qpOASES/releases/3.2.1/QpOASES_llar.gox— the build formula.Translation decisions
fromVer "releases/3.2.1"— the exact upstream tag for Conan3.2.1(every qpOASES tag isreleases/X.Y.Z; spelling preserved). The recipe serves only3.2.1, so no version range is claimed.requirements/tool_requires/test_requires, confirmed against the upstreamCMakeLists.txt.onBuild): CMake build of the static library.QPOASES_BUILD_EXAMPLES=OFF(matches the recipe'sgenerate).CMAKE_POLICY_VERSION_MINIMUM=3.5— upstream declarescmake_minimum_required(VERSION 2.6).fPICoption carried over viaCMAKE_POSITION_INDEPENDENT_CODE.lib/libqpOASES.aandinclude/headers; metadata-lqpOASES.fPICmodeled astarget.options(defaultON, matching Conan).filterrejectsfPIC=ONon Windows, where the recipe deletes the option.onTest): builds a minimalQProblemconsumer (derived from the Conantest_package) in an independent build tree so it also passes on a cache hit, links the installed library, and asserts a successful solve.Verification
Verified end-to-end against the real
releases/3.2.1source with the exact defines the formula uses:lib/libqpOASES.a,include/qpOASES.hpp,include/qpOASES/.onTestconsumer compiles, links-lqpOASES, andQProblem::init(...)returnsSUCCESSFUL_RETURN(exit 0)..goxclassfile (includingonBuild/onTestclosures) compiles under llar v0.3.0.