treewide: rework libatomic dependencies with ATOMIC64_DEPENDS - #30517
Conversation
Formality Check: Suggestions AvailableWe checked this pull request against the contribution guidelines. Here is what needs your attention:
|
6ed65f7 to
6009e7f
Compare
|
CI note: the failing build jobs on the 32-bit targets (mips, mipsel, powerpc, ARMv5) are expected until the snapshot SDKs include openwrt/openwrt@e23f575. The current SDKs (r36237 and older) predate that commit, so |
openwrt-ai
left a comment
There was a problem hiding this comment.
Commit checks
- 6009e7f "ustreamer: use ATOMIC64_DEPENDS" — also re-indents the whole
Package/ustreamerblock from tabs to two spaces; the message mentions only the dependency change.
Generated by Claude Code
| TITLE:=BitTorrent client for ncurses | ||
| URL:=https://github.com/rakshasa/rtorrent | ||
| DEPENDS:=+libcurl +libncurses +libatomic +libopenssl +libstdcpp +zlib | ||
| DEPENDS:=+libcurl +libncurses $(ATOMIC64_DEPENDS) +libopenssl +libstdcpp +zlib |
There was a problem hiding this comment.
Package rtorrent is missing dependencies for the following libraries:
libatomic.so.1
make[2]: *** [Makefile:99: .../i386_pentium-mmx/packages_ci/rtorrent-0.15.5-r2.apk] Error 1
ERROR: package/feeds/packages_ci/rtorrent failed to build (build variant: norpc).
i386_pentium-mmx and riscv64_generic (which also stops at package/rtorrent/compile) both select HAS_ATOMIC64, so $(ATOMIC64_DEPENDS) expands to nothing there with or without the core commit — the stale-SDK explanation in the PR comment does not cover these two jobs. rtorrent's link still records DT_NEEDED libatomic.so.1, and the packaging check keys on DT_NEEDED in gen-dependencies.sh:24 rather than on imported symbols, so this package needs the same TARGET_LDFLAGS += -Wl,--as-needed as nanomq/sysrepo/ustreamer. Both jobs aborted here, so the packages built after rtorrent (srt, libopen62541, gnutls, openal-soft, php8, ruby, python3, ustreamer, gstreamer1) were never checked on those two architectures.
Generated by Claude Code
There was a problem hiding this comment.
fixed, thanks — solved by the rebase onto d2a2e7b8 rather than by --as-needed: i386_pentium-mmx now gets past package/rtorrent/compile and through packaging.
Generated by Claude Code
| $(call Package/gstreamer1/Default) | ||
| TITLE+= library (core) | ||
| DEPENDS+= +glib2 +libpthread +libxml2 +(armeb||arm||powerpc||mips||mipsel):libatomic | ||
| DEPENDS+= +glib2 +libpthread +libxml2 $(ATOMIC64_DEPENDS) |
There was a problem hiding this comment.
nit: the PR body says this reworks every package in this feed that depends on libatomic, but roughly 30 Makefiles still carry a plain +libatomic, and net/samba4/Makefile:66 still has the identical open-coded architecture list being replaced here. Worth narrowing the claim or calling the remainder out as follow-up.
Generated by Claude Code
Btw this link does not work. |
|
Fixed. The full hash in the note was wrong; the commit is openwrt/openwrt@e23f575. The same note on the video and telephony PRs is corrected too. |
|
bind changes look good to me. |
libatomic is only needed where the compiler does not inline 64-bit atomics, but the package depends on it on every target. ATOMIC64_DEPENDS restricts the dependency to those targets. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
libatomic is only needed where the compiler does not inline 64-bit atomics, but the package depends on it on every target. ATOMIC64_DEPENDS restricts the dependency to those targets. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
The libraries link nothing from libatomic on any architecture, so the dependency is unnecessary. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
The open-coded architecture list pulls libatomic onto ARMv6K and ARMv7 targets although they inline 64-bit atomics. ATOMIC64_DEPENDS covers exactly the targets that need it. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
None of the binaries links libatomic on any architecture, so the dependency is unnecessary. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
The library links nothing from libatomic on any architecture, so the dependency is unnecessary. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
The daemon imports 64-bit atomics from libatomic where the compiler cannot inline them, but the CMake build links the library on every architecture. ATOMIC64_DEPENDS restricts the dependency to the targets that need it; --as-needed drops the unused library elsewhere. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
libatomic is only needed where the compiler does not inline 64-bit atomics, but the package depends on it on every target. ATOMIC64_DEPENDS restricts the dependency to those targets. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
libatomic is only needed where the compiler does not inline 64-bit atomics, but the package depends on it on every target. ATOMIC64_DEPENDS restricts the dependency to those targets. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
The CMake build links libatomic on targets without inline 64-bit atomics, but libopenal imports no symbol from it. Link with --as-needed so the unused library is dropped and remove the dependency. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
PHP's configure script probes __atomic_exchange_1 as an external function on riscv64, which only resolves against libatomic, and so links the library although no binary references an atomic symbol. The riscv64-only dependency and linker flag paper over that. Link with --as-needed so the unused library is dropped and remove both; mips and riscv64 builds import no atomic symbols. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
libatomic is only needed where the compiler does not inline 64-bit atomics, but the package depends on it on every target. ATOMIC64_DEPENDS restricts the dependency to those targets. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
libatomic is only needed where the compiler does not inline 64-bit atomics, but the package depends on it on every target. ATOMIC64_DEPENDS restricts the dependency to those targets. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
The open-coded architecture list pulls libatomic onto ARMv6K and ARMv7 targets although they inline 64-bit atomics, and still names the removed arc architecture. ATOMIC64_DEPENDS covers exactly the targets that need it. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
libatomic is only needed where the compiler does not inline 64-bit atomics, but the package depends on it on every target. ATOMIC64_DEPENDS restricts the dependency to those targets. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
sysrepo-plugind imports 64-bit atomics from libatomic where the compiler cannot inline them, but the CMake build links the library on every architecture. ATOMIC64_DEPENDS restricts the dependency to the targets that need it; --as-needed drops the unused library elsewhere. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
The binary links nothing from libatomic on any architecture, so the dependency is unnecessary. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
ustreamer and ustreamer-dump import 64-bit atomics from libatomic where the compiler cannot inline them, but the build links the library on every architecture. ATOMIC64_DEPENDS restricts the dependency to the targets that need it; --as-needed drops the unused library elsewhere. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
6009e7f to
9eb970e
Compare
| TITLE:=BitTorrent client for ncurses | ||
| URL:=https://github.com/rakshasa/rtorrent | ||
| DEPENDS:=+libcurl +libncurses +libatomic +libopenssl +libstdcpp +zlib | ||
| DEPENDS:=+libcurl +libncurses $(ATOMIC64_DEPENDS) +libopenssl +libstdcpp +zlib |
There was a problem hiding this comment.
PKG_RELEASE stays at 2: the rebase dropped this commit's bump, and master already shipped r2 with d2a2e7b8. A DEPENDS change on top of that needs PKG_RELEASE:=3 — it is the only commit in the series without a bump.
Generated by Claude Code
|
CI will never complete this, I'll merge and take care of the fallout once it surfaces on buildbot. |
📦 Package Details
Maintainers: @nmeyerhans , @dangowrt , @nmav, @flyn-org , @salzmdan , @mhei , @luizluca , @val-kulkov , @commodo, @neheb and others:
Description:
Follow-up to openwrt/openwrt#25112, which added
HAS_ATOMIC64andATOMIC64_DEPENDS:=+!HAS_ATOMIC64:libatomic: the compiler calls libatomic for 64-bit atomics only on mips, mipsel, powerpc and ARM below ARMv6K, so a package needs the dependency exactly there and nowhere else. This PR reworks every package in this feed that depends on libatomic, one commit per package, according to what its built binaries actually import (checked withreadelf -DWson the packaged files):+libatomicor an open-coded architecture list becomes$(ATOMIC64_DEPENDS). nanomq, sysrepo and ustreamer additionally link with--as-neededbecause their build systems link libatomic on every architecture, which would otherwise leave aNEEDED libatomic.so.1without a dependency on x86_64 and fail the packaging check.--as-needed. php8's+riscv64:libatomiccame from PHP's own configure probe, which references__atomic_exchange_1as a plain function and therefore always resolves to-latomicon riscv64.The old lists also named the removed
arcarchitecture (ruby) and missed ARMv5 (comrade, not touched here since its current upstream version has no libatomic dependency).🧪 Run Testing Details
HAS_ATOMIC64) and x86/64 (HAS_ATOMIC64), SDK builds of every package: on mt7621 the packages that import 64-bit atomics depend on and linklibatomic1, the others carry noNEEDED libatomic.so.1any more; on x86/64 no package links libatomic and every package passes the packaging dependency check✅ 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.