Skip to content

onigumura: fix library installation #29397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cotequeiroz merged 1 commit into from
May 11, 2026
Merged

onigumura: fix library installation #29397

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions libs/oniguruma/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ include $(TOPDIR)/rules.mk

PKG_NAME:=oniguruma
PKG_VERSION:=6.9.10
PKG_RELEASE:=2
PKG_RELEASE:=3

PKG_SOURCE:=onig-v$(subst _,-,$(PKG_VERSION)).tar.gz
PKG_SOURCE_URL:=https://codeload.github.com/kkos/oniguruma/tar.gz/v$(PKG_VERSION)?
Expand Down Expand Up @@ -43,7 +43,7 @@ CONFIGURE_ARGS += --enable-posix-api

define Package/oniguruma/install
$(INSTALL_DIR) $(1)/usr/lib
$(CP) $(PKG_INSTALL_DIR)/usr/lib/libonig.so.$(ABI_VERSION) $(1)/usr/lib/
$(INSTALL_BIN) $(PKG_INSTALL_DIR)/usr/lib/libonig.so.$(ABI_VERSION) $(1)/usr/lib/

@BKPepe BKPepe May 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I figured out what went wrong. I pushed the commit almost completely without the commit description. It was heavily inspired by openwrt/openwrt@38b22b1 and #9233

You are right, my change is not sufficient and breaks the library, but here is what is going on. To understand this issue, we need to look at two use-cases. One with ABI_VERSION, which I broke, and the other one without ABI_VERSION, which was fixed.

With ABI versioning:

Before:

tar -Oxzf bin/packages/arm_cortex-a9_vfpv3-d16/packages/oniguruma_6.9.9-r1_arm_cortex-a9_vfpv3-d16.ipk ./data.tar.gz | tar -tzvf -
drwxr-xr-x  0 0      0           0 Feb  5  2024 ./
drwxr-xr-x  0 0      0           0 Feb  5  2024 ./usr/
drwxr-xr-x  0 0      0           0 Feb  5  2024 ./usr/lib/
-rwxr-xr-x  0 0      0      526493 Feb  5  2024 ./usr/lib/libonig.so.5

Current master - basically broken:

tar -Oxzf bin/packages/arm_cortex-a9_vfpv3-d16/packages/oniguruma_6.9.9-r1_arm_cortex-a9_vfpv3-d16.ipk ./data.tar.gz | tar -tzvf -
drwxr-xr-x  0 0      0           0 Feb  5  2024 ./
drwxr-xr-x  0 0      0           0 Feb  5  2024 ./usr/
drwxr-xr-x  0 0      0           0 Feb  5  2024 ./usr/lib/
lrwxrwxrwx  0 0      0           0 Feb  5  2024 ./usr/lib/libonig.so.5 -> libonig.so.5.4.0

After applying this fix (just adding *):

Suggested change
$(INSTALL_BIN) $(PKG_INSTALL_DIR)/usr/lib/libonig.so.$(ABI_VERSION) $(1)/usr/lib/
$(CP) $(PKG_INSTALL_DIR)/usr/lib/libonig.so.$(ABI_VERSION)* $(1)/usr/lib/

It looks like this:

tar -Oxzf bin/packages/arm_cortex-a9_vfpv3-d16/packages/oniguruma_6.9.9-r1_arm_cortex-a9_vfpv3-d16.ipk ./data.tar.gz | tar -tzvf -
drwxr-xr-x  0 0      0           0 Feb  5  2024 ./
drwxr-xr-x  0 0      0           0 Feb  5  2024 ./usr/
drwxr-xr-x  0 0      0           0 Feb  5  2024 ./usr/lib/
lrwxrwxrwx  0 0      0           0 Feb  5  2024 ./usr/lib/libonig.so.5 -> libonig.so.5.4.0
-rwxr-xr-x  0 0      0      526493 Feb  5  2024 ./usr/lib/libonig.so.5.4.0

Without ABI versioning:

  • libre2 package
    Before:
tar -Oxzf bin/packages/arm_cortex-a9_vfpv3-d16/packages/re2_2023.02.01\~b025c6a3-r1_arm_cortex-a9_vfpv3-d16.ipk ./data.tar.gz | tar -tzvf - 
drwxr-xr-x  0 0      0           0 Aug 18  2024 ./
drwxr-xr-x  0 0      0           0 Aug 18  2024 ./usr/
drwxr-xr-x  0 0      0           0 Aug 18  2024 ./usr/lib/
-rw-r--r--  0 0      0      331875 Aug 18  2024 ./usr/lib/libre2.so
-rw-r--r--  0 0      0      331875 Aug 18  2024 ./usr/lib/libre2.so.10
-rw-r--r--  0 0      0      331875 Aug 18  2024 ./usr/lib/libre2.so.10.0.0

After applying my commit:

tar -Oxzf bin/packages/arm_cortex-a9_vfpv3-d16/packages/re2_2023.02.01\~b025c6a3-r1_arm_cortex-a9_vfpv3-d16.ipk ./data.tar.gz | tar -tzvf -
drwxr-xr-x  0 0      0           0 Aug 18  2024 ./
drwxr-xr-x  0 0      0           0 Aug 18  2024 ./usr/
drwxr-xr-x  0 0      0           0 Aug 18  2024 ./usr/lib/
lrwxrwxrwx  0 0      0           0 Aug 18  2024 ./usr/lib/libre2.so -> libre2.so.10
lrwxrwxrwx  0 0      0           0 Aug 18  2024 ./usr/lib/libre2.so.10 -> libre2.so.10.0.0
-rwxr-xr-x  0 0      0      331875 Aug 18  2024 ./usr/lib/libre2.so.10.0.0

To sum it up:

  • With ABI_VERSION, there was no symlink. Was this intentional?
  • Without ABI_VERSION, we were shipping three (!) files in case of libre2, which makes it huge and no symlinks at all.

I am not entirely sure how this pattern originated in the packages feed. If I look at the main openwrt repository, ABI_VERSION is not typically used this way in the Package/foo/install definitions. For instance, I checked the jansson package in the main repo, and it correctly ships the symlink when using ABI.

TL,DR: If I am not mistaken, even packages with ABI_VERSION should preserve the correct symlink structure pointing to the actual shared object. Can we agree on this one? If yes, I will prepare a new PR to fix other packages, which I broke. 😢

@cotequeiroz cotequeiroz May 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't say for other packages, but my installing of libonig.so.5 only is intentional. That's the only file the OS needs to load the library. The unversioned libonig.so would be used to link the binary when building a binary. It could arguably be used if one were to use an openwrt host for building binaries. However, our package system does not handle multiple packages owning the symlink well, so I never tried to head in that direction.
Why do we need the libonig.so.5.4.0 file? To me it is a needless symlink that will mostly clutter /usr/lib.

TLDR: I would leave the package as I'm doing here.

@BKPepe BKPepe May 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, and I see your point about saving an inode and keeping the package layout minimal. My concern is mainly consistency across OpenWrt and easier debugging in practice.

When I look at /usr/lib on a running router, the symlinked layout is what I see for the large majority of shared libraries, including core OpenWrt ones. From my point of view, the bigger problem is having several different packaging styles for essentially the same kind of library, because that makes the feed harder to maintain and makes it less obvious which approach is considered correct.

In my view, it is more useful to know the exact library version that is present, not just the major version. Keeping only the major version visible does not seem like a good trade-off to me, especially when the symlink itself uses very little space. With ls -l, you can immediately see which exact library version is present. You can also inspect package contents through apk info -L oniguruma or opkg files oniguruma, but that still does not make the full version visible in the same straightforward way if only the major-version name is kept. It becomes even more confusing when the package version does not directly match the library version, because then it is no longer obvious which exact library build is actually installed unless the full version is visible in /usr/lib.

What I found is that The TLDP Program Library HOWTO explicitly describes the usual layout as SONAME -> Real Name, stating that a fully qualified soname is a symbolic link to the shared library’s real name, and that ldconfig creates sonames as symbolic links to the real names. The GNU Libtool manual’s Release numbers page also shows the same general idea of using a symlinked library name that points to the actual versioned file, although it is less explicit about SONAME terminology there. For that reason, I do not think omitting the library’s full release number is a good idea.

That being said, you are the maintainer of this package, so I will respect your decision here. I would just prefer that we keep one consistent shared-library layout across the feed, because mixing both approaches makes maintenance and debugging harder than necessary, and then we end up having to debate what is actually correct, why it should be done that way, and what the expected layout is.

@BKPepe BKPepe May 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And it looks like CI/CD agrees with me. :)
Screenshot_20260510_170127_Brave

endef

define Build/InstallDev
Expand Down
Loading