Skip to content

feat(foxi): add LLAR formula - #128

Open
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/110-foxi
Open

feat(foxi): add LLAR formula#128
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/110-foxi

Conversation

@MeteorsLiu

Copy link
Copy Markdown
Collaborator

Add an LLAR Formula for houseroad/foxi at Conan source commit bd6feb6d0d3fc903df42b4feb82a602a5fcb1fd5.

The implementation includes:

  • Carry the verified Conan CMake patch and install the foxi headers and libraries into the LLAR output.
  • Preserve the source-backed static, shared, and module targets without adding unverified dependencies.
  • Publish installed link metadata and compile an independent C consumer against the result.
  • Validate fresh and cache-hit builds with the exact source commit on Darwin arm64.

The upstream repository has no release tags, so the Formula uses the immutable Conan source commit explicitly and avoids inventing a version comparator.

Closes #110

@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.

FennoAI Review

New LLAR recipe for houseroad/foxi. The overall structure is solid — commit pinning with a rationale comment, _llar_-prefixed scratch patch files, --forward --batch patch application, cache-hit-safe onTest flag derivation, and appropriate file permissions. Nice cross-platform touch setting both LD_LIBRARY_PATH and DYLD_LIBRARY_PATH.

One correctness issue is worth fixing (inline). The rest are consistency/documentation notes.

Consistency with existing recipes

  • Filename: existing recipes use PascalCase (Cglm_llar.gox, Jsonc_llar.gox, Zlib_llar.gox); this file is lowercase foxi_llar.gox. Consider renaming to Foxi_llar.gox, and confirm recipe discovery does not depend on the capitalized name.
  • Indentation: this recipe uses 4-space indentation while all existing recipes use tabs (gofmt style). Consider converting to tabs.
  • versions.json uses 2-space indentation; siblings use tabs.

Documentation (relative to the well-commented cglm/json-c recipes)

  • No comment explains the two patches (fix-conflicting-types.patch alters the public onnxInitGraph/onnxWaitEventFor ABI; cmake.patch changes install layout) or the CMAKE_POLICY_VERSION_MINIMUM=3.5 define. A short rationale for each would help future maintainers.
  • The onTest flags are deliberately re-derived from installDir so the test works on a cache hit (onBuild skipped). The cglm/json-c recipes document this explicitly; a one-line comment here would match convention.

Minor

  • os.setenv "LD_LIBRARY_PATH" / "DYLD_LIBRARY_PATH" overwrite any inherited value rather than prepending installDir/lib:$LD_LIBRARY_PATH. Low impact since it runs only in onTest, but prepending is safer.

int ret = onnxifi_load(ONNXIFI_LOADER_FLAG_VERSION_1_0, NULL, &onnx);
if (!ret) {
printf("Cannot load onnxifi lib\n");
return 0;

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.

The failure branch prints an error but returns 0 (success), so onTest (which checks the binary's exit code via lastErr!) can never fail even when onnxifi_load fails — the test becomes a no-op. onnxifi_load returns non-zero on success per upstream onnxifi_loader.h, so !ret is correctly the failure path; only the return code is wrong.

Suggested change
return 0;
if (!ret) {
fprintf(stderr, "Cannot load onnxifi lib\n");
return 1;
}

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

1 participant