Skip to content

feat(miniz): add LLAR formula - #167

Open
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/37-miniz
Open

feat(miniz): add LLAR formula#167
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/37-miniz

Conversation

@MeteorsLiu

Copy link
Copy Markdown
Collaborator

Closes #37

Add an LLAR formula for richgel999/miniz from the Conan Center recipe at ffe30df101afd4dc95aac2f14b25bf345e64d7be.

  • fromVer is 3.0.1, the lowest Conan recipe version that shares the 3.x CMake, header, and pkg-config contract used by 3.1.1 (includedir=${prefix}/include/miniz).
  • Build with upstream CMake: examples/fuzzers/amalgamate/header-only/tests off, INSTALL_PROJECT=ON, BUILD_SHARED_LIBS/fPIC from options, and CMAKE_INSTALL_LIBDIR=lib.
  • Keep the installed miniz.pc instead of removing lib/pkgconfig. Rewrite only prefix to relocatable ${pcfiledir}/../.. and set metadata from pkgconfig.lookup("miniz").
  • Consumer test is Conan's test_package.c compress/uncompress sample, compiled with the lookup flags via @flagsFile.

@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: richgel999/miniz 3.0.1 recipe

Solid recipe that closely follows the established sibling patterns (streamvbyte, semver.c): idiomatic defaults/filter/onBuild/onTest, valid embedded C consumer using public miniz symbols, and a correct relocatable pkg-config rewrite. Security and documentation-accuracy passes found nothing.

One inline finding worth addressing (position-independent-code handling), plus a minor cosmetic note below.

Minor — versions.json indentation: the new richgel999/miniz/versions.json uses 2-space indentation, whereas sibling versions.json files (fast-pack/streamvbyte, h2non/semver.c) use tabs. Content is correct; align with repo convention if a formatter is enforced.

Positive: the miniz.pc prefix rewrite is correct and minimal — only prefix= bakes in the absolute install dir; exec_prefix/libdir/includedir all derive from ${prefix}, so the single strings.replace(..., 1) makes the whole file relocatable, and its comment accurately describes this.

c.defineBool "INSTALL_PROJECT", true
c.defineBool "BUILD_TESTS", false
c.defineBool "BUILD_SHARED_LIBS", shared
c.defineBool "CMAKE_POSITION_INDEPENDENT_CODE", fPIC

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.

CMAKE_POSITION_INDEPENDENT_CODE is set unconditionally, which diverges from the sibling fPIC recipe and is contradictory for shared builds.

h2non/semver.c uses an identical shared+fPIC filter but guards this define with if !shared:

c.defineBool "BUILD_SHARED_LIBS", shared
if !shared {
	c.defineBool "CMAKE_POSITION_INDEPENDENT_CODE", fPIC
}

Two consequences:

  • PIC only matters for static libraries — shared libs are always position-independent. So (shared=ON, fPIC=ON) and (shared=ON, fPIC=OFF) produce identical output: a redundant build variant and a misleading toggle.
  • (shared=ON, fPIC=OFF) explicitly passes CMAKE_POSITION_INDEPENDENT_CODE=OFF into a shared build, which is contradictory and, depending on toolchain, can fail to link.

Suggest guarding with if !shared to match the sibling idiom, or add a comment if the unconditional set is intentional.

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

1 participant