Skip to content

formula: mutouyun/cpp-ipc - #193

Open
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/50-cpp-ipc
Open

formula: mutouyun/cpp-ipc#193
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/50-cpp-ipc

Conversation

@MeteorsLiu

Copy link
Copy Markdown
Collaborator

Closes #50

Adds an idiomatic LLAR formula for mutouyun/cpp-ipc at fromVer "v1.4.1".

Darwin limitation: v1.4.1 still does not define IPC_OS_* for __APPLE__. src/libipc/platform/platform.cpp and the Linux-only sync backends fail with #error "Unsupported platform." (Conan also marks Apple unsupported). POSIX shm_open exists, but the library does not compile or run on Darwin. Linux remains the intended consumer platform (-lrt -lpthread -lm).

@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: add mutouyun/cpp-ipc v1.4.1 recipe

Solid recipe that closely follows the established double-conversion convention (CMake + pkgconfig + shared/fPIC options + filter + relocatable .pc). Error propagation (!), the license copy, and the onTest consumer all match repo idioms. No security concerns found.

A few robustness/consistency notes below — none are blockers. The most notable is the unguarded target.require["os"][0] access (this recipe is the only one in the repo not using slices.contains).

}

libs := "-L$${libdir} -lipc"
if target.require["os"][0] == "linux" {

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.

[P2] Unguarded target.require["os"][0] can panic; use slices.contains

This is the only recipe in the repo that indexes target.require["os"][0] directly — microtar, neco use slices.contains(target.require["os"], ...) and poshlib guards with a length check. If os is empty/unset, [0] panics with index-out-of-range. Prefer slices.contains(target.require["os"], "linux") for consistency and safety.

os.writeFile(filepath.join(licenseDir, "LICENSE"), os.readFile(filepath.join(ctx.SourceDir, "LICENSE"))!, 0o644)!

version := ""
for line in strings.split(string(os.readFile(cmakeLists)!), "\n") {

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] Redundant second read of CMakeLists.txt

The file is read into source at line 47, then read from disk again at line 66 for version parsing. The PIC replace at line 48 doesn't touch the project(...) line, so you can parse the version from the in-memory source (strings.split(source, "\n")) and drop the redundant read.


libs := "-L$${libdir} -lipc"
if target.require["os"][0] == "linux" {
libs += " -lrt -lpthread -lm"

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] -lm is not linked by upstream cpp-ipc

Upstream src/CMakeLists.txt links only pthread and rt (PUBLIC pthread rt) on Linux — the math library m is never linked. The added -lm overstates the library's dependencies in the generated .pc. It is likely harmless (no-op on most linkers), but consider dropping it to match upstream, or add a comment if there is a real reason.

// controls CMAKE_POSITION_INDEPENDENT_CODE for static builds.
cmakeLists := filepath.join(ctx.SourceDir, "CMakeLists.txt")
source := string(os.readFile(cmakeLists)!)
source = strings.replace(source, "set(CMAKE_POSITION_INDEPENDENT_CODE ON)\n", "", 1)

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] Unguarded string.replace can silently no-op on upstream drift

The PIC-line removal at line 48 matches the exact literal set(CMAKE_POSITION_INDEPENDENT_CODE ON)\n (verified present in v1.4.1 today, so it works now). But if upstream ever changes spacing/casing/line-ending, replace silently matches nothing and the hardcoded PIC stays — quietly breaking the fPIC=OFF static build with no error. The double-conversion reference guards its replace with a .contains check. Consider a guard (and ideally panic on mismatch) so future drift surfaces loudly.

version := ""
for line in strings.split(string(os.readFile(cmakeLists)!), "\n") {
if line.hasPrefix("project(cpp-ipc VERSION ") {
version = line.trimPrefix("project(cpp-ipc VERSION ").trimSuffix(")")

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 via trimSuffix is fragile vs .split(")")[0]

trimSuffix(")") yields the wrong value if upstream adds tokens after the version, e.g. project(cpp-ipc VERSION 1.4.1 LANGUAGES CXX)1.4.1 LANGUAGES CXX. The double-conversion reference uses .split(")")[0], which is resilient to trailing tokens. Consider matching that pattern.

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 cpp-ipc recipe to LLAR

1 participant