unibilium: add new package - #27951
betonmischer86 wants to merge 1 commit into
Conversation
GeorgeSapkin
left a comment
GeorgeSapkin
left a comment
There was a problem hiding this comment.
Unless something is going to depend on this library, and nothing indicates this in either the PR or commit message, there's no point adding it.
As with the other PRs, you need to set yourself as the maintainer and don't need to include the license file.
57fbad3 to
8b53edb
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new libunibilium package to the OpenWrt packages feed, providing the Unibilium terminfo parsing library for potential downstream consumers.
Changes:
- Adds package metadata, source download information, license, maintainer, and description.
- Defines build/install behavior for staging headers, pkg-config metadata, static library, and shared library files.
- Sets terminfo search paths via upstream Makefile flags.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ncurses-style terminfo files, and it can interpret terminfo format strings. | ||
| endef | ||
|
|
||
| MAKE_FLAGS+= TERMINFO_DIRS='"/etc/terminfo:/lib/terminfo:/usr/share/terminfo:/usr/lib/terminfo:/usr/local/share/terminfo:/usr/local/lib/terminfo"' |
|
|
||
| PKG_BUILD_PARALLEL:=1 | ||
| PKG_INSTALL:=1 | ||
| PKG_FIXUP:=autoreconf |
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Commit checks
- The commit message body contains a stray junk token: "It can read and write sdfkj ncurses-style terminfo files, ...". Please remove
sdfkjso the message matches the (clean)Package/libunibilium/descriptiontext in the Makefile.
Generated by Claude Code
0abed55 to
73938e9
Compare
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Commit checks
- 73938e9 "unibilium: add new package" — the commit body still carries a stray junk token: "It can read and write sdfkj ncurses-style terminfo files, ...". The
Package/libunibilium/descriptiontext in the Makefile is clean, so please dropsdfkjto make the two agree. (Same finding as my review on 0abed55; the rebase to 73938e9 carried it over unchanged.)
libs/unibilium/Makefile itself is byte-identical to the version I last reviewed — the only change on this push is the rebase onto a newer master. The two inline notes re-raise unresolved build-system questions that GitHub now hides as outdated; neither is a confirmed defect, since I could not fetch the upstream tarball from this session and no build job has run on this head.
Generated by Claude Code
|
|
||
| PKG_BUILD_PARALLEL:=1 | ||
| PKG_INSTALL:=1 | ||
| PKG_FIXUP:=autoreconf |
There was a problem hiding this comment.
PKG_FIXUP:=autoreconf is inconsistent with how the rest of this Makefile drives the build. Line 33 passes the terminfo search path as a make variable (MAKE_FLAGS+= TERMINFO_DIRS=...), which only makes sense if upstream ships a hand-written Makefile — an autotools tree would take that as a configure argument. If the 2.1.2 tarball has no configure.ac/Makefile.am, this fixup runs autoreconf on a non-autotools tree and fails before anything compiles.
The same premise decides whether the install paths line up: with PKG_INSTALL:=1 and no --prefix=/usr configure step, a plain upstream Makefile installs into $(PKG_INSTALL_DIR)/usr/local/..., while lines 38-41 and 46 copy from $(PKG_INSTALL_DIR)/usr/... — the header, unibilium.pc and both library variants would then be missing. In that case MAKE_FLAGS also needs PREFIX=/usr (or matching libdir/includedir).
Posting this as a question rather than an assertion: I could not fetch the upstream tarball from this session to confirm which case applies, and no build job has run on this head (only the three FormalityCheck runs). Which build system does unibilium 2.1.2 actually use? Both points were raised on an earlier push and are still unresolved — GitHub just hides those threads as outdated after the rebase.
Generated by Claude Code
|
|
||
| define Package/libunibilium/install | ||
| $(INSTALL_DIR) $(1)/usr/lib | ||
| $(CP) $(PKG_INSTALL_DIR)/usr/lib/libunibilium.so* $(1)/usr/lib |
There was a problem hiding this comment.
The package ships the versioned libunibilium.so.* files but sets no ABI_VERSION, which defaults to empty in include/package-defaults.mk — so no ABI suffix is recorded for libunibilium. When a later PKG_VERSION bump changes the SONAME, packages already installed against the old libunibilium.so.N still resolve the dependency and break at runtime instead of being pulled in for a rebuild.
In-tree libraries that ship a versioned SONAME declare it inside the Package/... block, e.g. libs/lzo/Makefile:32 and libs/libnatpmp/Makefile:44. Adding ABI_VERSION:=<soname major of libunibilium.so> to define Package/libunibilium (lines 21-26) would cover it — no suggestion block since I could not fetch the tarball to read the actual major.
Generated by Claude Code
Unibilium is a very basic terminfo library. It can read and write sdfkj ncurses-style terminfo files, and it can interpret terminfo format strings. Signed-off-by: Valeriy Kosikhin <vkosikhin@gmail.com>
73938e9 to
f0a61fb
Compare
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Commit checks
- f0a61fb "unibilium: add new package" — body still contains the stray token
sdfkj("It can read and write sdfkj ncurses-style ..."); drop it so the text matches the Makefile description.
Generated by Claude Code
📦 Package Details
Maintainer: @betonmischer86
(You can find this by checking the history of the package
Makefile.)Description:
Unibilium is a very basic terminfo library. It can read and write ncurses-style terminfo files, and it can interpret terminfo format strings.
🧪 Run Testing Details
✅ Formalities
If your PR contains a patch:
git am(e.g., subject line, commit description, etc.)
We must try to upstream patches to reduce maintenance burden.