Skip to content

feat(cpu_features): add LLAR formula - #168

Open
MeteorsLiu wants to merge 2 commits into
xgo-dev:mainfrom
MeteorsLiu:issue/36-cpu-features
Open

feat(cpu_features): add LLAR formula#168
MeteorsLiu wants to merge 2 commits into
xgo-dev:mainfrom
MeteorsLiu:issue/36-cpu-features

Conversation

@MeteorsLiu

Copy link
Copy Markdown
Collaborator

Closes #36

Translate the Conan Center cpu_features recipe (ffe30df101afd4dc95aac2f14b25bf345e64d7be) into an LLAR Formula for google/cpu_features.

Formula

  • Module: google/cpu_features
  • fromVer: v0.9.0 (Conan recipe covers 0.9.0 / 0.10.1; CMake install layout, public headers under include/cpu_features, and consumer contract match)
  • Options: shared (default OFF), fPIC (default ON)
  • CMake: BUILD_TESTING=OFF, CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=ON, CMAKE_INSTALL_LIBDIR=lib, BUILD_SHARED_LIBS / CMAKE_POSITION_INDEPENDENT_CODE from options
  • LICENSE copied to licenses/

pkg-config

Upstream does not install a .pc file. The Formula writes relocatable lib/pkgconfig/cpu_features.pc:

  • prefix=${pcfiledir}/../..
  • Cflags: -I${includedir}/cpu_features so the Conan test can #include <cpu_features_macros.h>
  • Libs: -L${libdir} -lcpu_features plus -ldl on Linux/FreeBSD (Conan system_libs = ["dl"]; placed in Libs so default static pkgconfig.lookup is enough)
  • metadata is the complete pkg-config --cflags --libs lookup
  • onTest compiles the Conan test_package.c consumer with a flags file from that lookup

Apple CMAKE_SYSTEM_PROCESSOR=aarch64 is not set: v0.9.0+ already matches Darwin's arm64, and CI is linux/darwin only. Android ndk_compat is not exposed.

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

This adds a new LLAR build formula for google/cpu_features v0.9.0. It closely and correctly follows the established repo conventions (e.g. fast-pack/streamvbyte, intel/libipt): the filter/defaults blocks, CMake option handling, license copy, relocatable pkgconfig generation, defensive version panic, and shared-build LD_LIBRARY_PATH/DYLD_LIBRARY_PATH handling all match existing patterns. versions.json is minimal and correct.

One correctness bug worth fixing before merge (the PPC test include typo). A couple of minor, non-blocking notes are inline.

#elif defined(CPU_FEATURES_ARCH_MIPS)
#include <cpuinfo_mips.h>
#elif defined(CPU_FEATURES_ARCH_PPC)
#include <ccpuinfo_ppc.h>

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.

[P1] PPC include header has a typo (ccpuinfo_ppc.h)

The PPC branch of the embedded test consumer includes <ccpuinfo_ppc.h> (leading double c). The upstream cpu_features header is cpuinfo_ppc.h. On a PPC target, onTest compiles this consumer and the include will fail with a file-not-found error, breaking the test. The other arch branches (cpuinfo_x86.h, cpuinfo_arm.h, cpuinfo_aarch64.h, cpuinfo_mips.h) are correct.

Suggested change
#include <ccpuinfo_ppc.h>
#include <cpuinfo_ppc.h>

Comment on lines +93 to +94
if line.hasPrefix("project(CpuFeatures VERSION ") {
version = line.trimPrefix("project(CpuFeatures VERSION ").split(" ")[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.

[P3] Version parse is whitespace-brittle for future tags

Version extraction relies on the exact literal prefix project(CpuFeatures VERSION plus .split(" ")[0]. This is correct for v0.9.0 (project(CpuFeatures VERSION 0.9.0 LANGUAGES C)). But since fromVer "v0.9.0" means this formula may be reused for later tags, any whitespace variation (extra space, tab, or a line-wrapped project(...)) would miss the prefix and hit the panic. Consider a whitespace-tolerant parse (e.g. tokenizing) as done in intel/libipt. Non-blocking.

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

1 participant