Skip to content

formula: dmtx/libdmtx - #197

Open
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/26-libdmtx
Open

formula: dmtx/libdmtx#197
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/26-libdmtx

Conversation

@MeteorsLiu

Copy link
Copy Markdown
Collaborator

Closes #26

@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: dmtx/libdmtx v0.7.8 recipe

Solid recipe that closely follows established repo conventions (neco, streamvbyte, libatomic_ops). Verified against upstream v0.7.8: project(DMTX VERSION 0.7.8 LANGUAGES C) parses correctly, the shared/static option wiring is right, BUILD_TESTING is correctly forced off, and the synthesized .pc fields (URL/Description/Cflags) match the upstream Autotools template. Security and performance passes found nothing.

Two minor notes are inline below — neither is blocking.

}

libs := "-L$${libdir} -ldmtx"
for osName in target.require["os"] {

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.

-lm is only appended when target.require["os"] explicitly contains linux/freebsd. Upstream links libm PUBLIC on all non-MSVC platforms, so if require["os"] is unset/empty for a UNIX-like target, this loop runs zero times and -lm is dropped from the .pc Libs: — static consumers calling math symbols would then hit undefined references. This mirrors the sibling neco recipe's require["os"]-gated pattern, so it matches convention, but it's worth confirming the harness always populates require["os"] for your target platforms. Minor style: the sibling recipes use slices.contains(target.require["os"], "linux") for this exact check, which is shorter and drops the manual break.

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

// CMake installs the library and dmtx.h but not libdmtx.pc. Keep the
// Autotools template's fields and make the published metadata relocatable.

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 comment says "Keep the Autotools template's fields," but the .pc is hand-authored inline rather than read/transformed from any template, and the Libs field intentionally diverges from the upstream template (-ldmtx-ldmtx -lm on linux/freebsd). Consider rewording to match what the code does, e.g. "libdmtx ships no pkg-config file, so synthesize one with the standard fields (adding -lm where libm is needed) and make it relocatable."

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

1 participant