Skip to content

feat(libversion): add LLAR formula - #182

Open
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/62-libversion
Open

feat(libversion): add LLAR formula#182
MeteorsLiu wants to merge 1 commit into
xgo-dev:mainfrom
MeteorsLiu:issue/62-libversion

Conversation

@MeteorsLiu

Copy link
Copy Markdown
Collaborator

Translate Conan Center libversion to LLAR.

Closes #62

@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: repology/libversion 3.0.0

Solid addition that closely mirrors the established libde265 recipe (cmake builder, relocatable pkg-config rewrite, license copy, real API smoke test). Build/test structure, the stricter filter (validates option names, not just values), and versions.json all follow repo conventions well.

Two points worth a look, neither blocking:

  1. Libs.private not propagated for static builds (inline). The static branch folds only Cflags.private into Cflags:, whereas libde265 handles both Libs.private and Cflags.private. For libversion (no external link deps) this is likely harmless, but the divergence is worth confirming or documenting.
  2. String-patch replacements silently no-op on upstream drift (inline). The install(TARGETS ...) and .pc strings.replace(..., 1) calls fail silently if the upstream text changes, shipping a wrong/non-relocatable package. Low risk since the version is pinned, but a fail-loud check would help.

Minor, non-blocking: onBuild uses target.options["shared"][0] while sibling packages often use slices.contains (more robust to empty/multi-valued options); and the test consumer omits an explicit -std that libde265/libatomic pass. No security, performance, or documentation-accuracy issues found (comments and the consumer assertions match upstream behavior).

Comment on lines +122 to +138
if !shared {
lines := pc.split("\n")
privateCflags := ""
for line in lines {
if line.hasPrefix("Cflags.private:") {
privateCflags = line.trimPrefix("Cflags.private:").trimSpace()
}
}
if privateCflags != "" {
for i, line in lines {
if line.hasPrefix("Cflags:") {
lines[i] = line + " " + privateCflags
}
}
pc = strings.join(lines, "\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.

[P2] Static build drops Libs.private (diverges from libde265)

For static builds this branch folds only Cflags.private into Cflags:, but never Libs.private into Libs:. The sibling libde265 recipe handles both. When a .pc is consumed without --static, pkg-config emits only Libs:, so any transitive static link dependencies in Libs.private: are dropped — and onTest uses the non---static flags, so it wouldn't catch a resulting link failure.

For libversion specifically (a small C lib with no external link deps) this is probably latent rather than breaking. Recommendation: either mirror libde265 and propagate Libs.private symmetrically, or add a short comment noting the omission is intentional because upstream libversion.pc has no meaningful private libs — so a future reader can tell.

if shared {
installed = "libversion"
}
libLists = strings.replace(libLists, "install(TARGETS libversion libversion_static EXPORT libversion", "install(TARGETS "+installed+" EXPORT libversion", 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] String-patch replacements no-op silently on upstream drift

This exact-string strings.replace(..., 1) (and the .pc replacements just below) silently do nothing if the upstream text ever changes format — the count-1 replace returns the input unchanged with no error. That would ship both libraries installed here, or a non-relocatable .pc below, without failing the build.

Risk is low because fromVer pins 3.0.0, and this matches the string-patching pattern used elsewhere in the repo. Consider asserting the replacement actually occurred (e.g. compare before/after) to fail loudly on upstream drift.

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

1 participant