feat(google/crc32c): add LLAR formula for crc32c 1.1.1+ - #117
feat(google/crc32c): add LLAR formula for crc32c 1.1.1+#117fennoai[bot] wants to merge 2 commits into
Conversation
Translate the Conan Center crc32c recipe into an idiomatic LLAR Formula. - fromVer "1.1.1": both 1.1.1 and 1.1.2 share the same upstream build contract (CMake option set, single installed header crc32c/crc32c.h, target crc32c, cmake_minimum_required 3.1), so one Formula serves both. - onBuild: CMake with CRC32C_BUILD_TESTS/BENCHMARKS/USE_GLOG=OFF and CRC32C_INSTALL=ON, matching the recipe. CMAKE_POLICY_VERSION_MINIMUM=3.5 keeps the CMake 3.1 project buildable under CMake 4. Metadata: -lcrc32c. - No dependencies: glog/googletest/benchmark are vendored subprojects used only by the disabled tests/benchmarks; there are no find_package deps. - onTest: compiles and runs a consumer mirroring the Conan test_package via the exported Crc32c::crc32c package target, in a build tree independent of onBuild so it also passes on a cache hit. Verified with `llar test` for 1.1.2 and 1.1.1 on linux/amd64, including a cache-hit re-run to exercise onTest without onBuild. Closes #47 Co-authored-by: fennoai[bot] <231223108+fennoai[bot]@users.noreply.github.com> Co-authored-by: MeteorsLiu <17515813+MeteorsLiu@users.noreply.github.com>
There was a problem hiding this comment.
Review: google/crc32c 1.1.1 formula
Solid, well-commented formula. The build recipe, CMake option handling (CRC32C_BUILD_TESTS/BENCHMARKS/USE_GLOG=OFF, CRC32C_INSTALL=ON), -lcrc32c metadata, id/versions.json.path/directory alignment, fromVer "1.1.1" literal, and the Crc32c_llar.gox → Crc32c class-name stem all check out against the LLAR spec and the upstream crc32c 1.1.1 source. Inline comments were verified accurate against upstream (cmake 3.1 minimum, exported Crc32c::crc32c target, lib/cmake/Crc32c, no pkg-config file). No security concerns.
Two findings inline. The onTest one is worth addressing before merge; the other is minor cleanup.
Note: reviewed against the checked-out branch after fetching the PR head.
| s.resize(4); | ||
| std::uint32_t b = crc32c::Crc32c(s); | ||
|
|
||
| return (a == b) ? 0 : 1; |
There was a problem hiding this comment.
onTest is effectively a tautology and does not validate CRC correctness. a = crc32c::Crc32c(buffer, 4) runs over 4 zero bytes, and b = crc32c::Crc32c(s) runs over a std::string of size 4 (default-filled with '\0'). The std::string overload just forwards the same bytes to the same routine, so a == b is guaranteed true by construction — the check passes even if Crc32c were broken (e.g. returned a constant), since it only proves the two overloads agree, not that the CRC is computed correctly.
Assert against a known-answer constant instead. CRC32C over 4 zero bytes is 0x48674BC7; e.g. return (a == 0x48674BC7u) ? 0 : 1; (optionally still checking a == b) turns this into a real smoke test that would catch a miscompiled/misconfigured library. A second vector such as "123456789" → 0xE3069283 would strengthen it further.
|
|
||
| tc := cmake.new(testDir, testBuild, "") | ||
| tc.buildType "Release" | ||
| tc.define "CMAKE_POLICY_VERSION_MINIMUM", "3.5" |
There was a problem hiding this comment.
Minor: CMAKE_POLICY_VERSION_MINIMUM=3.5 is redundant here. The generated consumer CMakeLists.txt already declares cmake_minimum_required(VERSION 3.5), and CMake config packages (loaded via find_package(Crc32c CONFIG)) don't call cmake_minimum_required, so this shim is dead in the test path. It is justified in onBuild (line 13) because upstream's own top-level CMakeLists.txt requests VERSION 3.1. Consider dropping it from onTest, or add a note explaining which downstream file it guards.
Translates the Conan Center
crc32crecipe into an idiomatic LLAR Formula forgoogle/crc32c. Closes #47.What was added
google/crc32c/versions.json— module metadata, no static deps.google/crc32c/1.1.1/Crc32c_llar.gox— the build formula.Translation decisions (upstream-verified)
fromVer "1.1.1"— the earliest version the Conan recipe folder serves. Upstream1.1.1and1.1.2share an identical build contract (sameoption()set, single installed headercrc32c/crc32c.h, targetcrc32c,cmake_minimum_required(VERSION 3.1)), so one Formula covers both without an extra threshold.onRequire— glog, googletest and benchmark are vendored subprojects used only by the tests/benchmarks, which are disabled. There are nofind_package/requiresdeps.onBuild— CMake withCRC32C_BUILD_TESTS=OFF,CRC32C_BUILD_BENCHMARKS=OFF,CRC32C_USE_GLOG=OFF,CRC32C_INSTALL=ON(matching the recipe'sgenerateblock).CMAKE_POLICY_VERSION_MINIMUM=3.5keeps the CMake-3.1 project buildable under CMake 4, mirroring the recipe's<= 1.1.2handling. Metadata is-lcrc32c.onTest— compiles and runs a small consumer that mirrors the Conantest_package(raw-buffer andstd::stringoverloads ofcrc32c::Crc32c) against the installed exported package targetCrc32c::crc32c. Its build tree is independent of theonBuild_builddir, so it also passes on a cache hit.Validation
Ran locally on linux/amd64 (only host arch available in the sandbox):
llar test ./google/crc32c@1.1.2 --os linux --arch amd64(exact Conan tag)-lcrc32cllar test ./google/crc32c@1.1.1 --os linux --arch amd64(fromVerboundary)1.1.2re-run (cache hit)onBuildskipped,onTeststill passes