pkg/58966: mk: REQUIRES/PROVIDES analysis is wrong for self-libraries

[campbell+netbsd%mumble.net@localhost]

>Number:         58966
>Category:       pkg
>Synopsis:       mk: REQUIRES/PROVIDES analysis is wrong for self-libraries
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    pkg-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jan 07 02:40:00 +0000 2025
>Originator:     Taylor R Campbell
>Release:        current, 2024Q4, ...
>Organization:
The SelfBSD Loadation
>Environment:
>Description:

	The PROVIDES/REQUIRES analysis with ldd(1) is confused by the
	following scenario:

		- gccN has installed gccN/lib/libquadmath.so.M.
		- gccN-libs installs
		  . gccN/$GCC_TARGET_MACHINE/lib/libgfortran.so.L
		  . gccN/$GCC_TARGET_MACHINE/lib/libquadmath.so.M
		  into the destdir both having
		  . gccN/$GCC_TARGET_MACHINE/lib
		  . gccN/lib
		  in rpath.

	Under these circumstances:

	1. At _build-time_, running `ldd
	   gccN/$GCC_TARGET_MACHINE/lib/libgfortran.so.L' finds
	   libquadmath.so.M at gccN/lib/libquadmath.so.M.

	   Since the gccN-libs libraries (which are just copies of the
	   gccN libraries in a different location) only exist in the
	   destdir, they are invisible to ldd even though their
	   directories come first in the rpath.

	2. At _run-time_, when the loader actually loads
	   gccN/$GCC_TARGET_MACHINE/lib/libgfortran.so.L, it will find
	   libquadmath.so.M at
	   gccN/$GCC_TARGET_MACHINE/lib/libquadmath.so.M.

	Thus, the gccN-libs package is produced with unresolvable
	REQUIRES at run-time, and now that pkg_add(8) is strict about
	verifying REQUIRES, they are not installable.

	Worse, if you already have a gccN-libs package installed (maybe
	an earlier version) when you do this, ldd(1) will pick that up
	and produce resolvable REQUIRES in the resulting package.  So
	during development it _looks like_ everything is just fine.

	This ldd(1) business will also never work in a cross-build
	environment.

>How-To-Repeat:

	1. Build and install lang/gcc10-libs.
	2. pkg_info -B gcc10-libs | grep -e REQUIRES -e PROVIDES
	3. Build lang/gcc10-libs again.
	4. pkg_info -B $PACKAGES/All/gcc10-libs-*.tgz | \
	   grep -e REQUIRES -e PROVIDES


>Fix:

	Workaround:

CHECK_SHLIBS_SKIP+=	${LIBGCC_PREFIX}/${GCC_TARGET_MACHINE}/lib

	Fix: Adopt jperkin's REQUIRES/PROVIDES ldd-free logic.

From fc6e58a8457b3d1f5506b1a101a78b0bcb1213fa Mon Sep 17 00:00:00 2001
From: Jonathan Perkin <jperkin%smartos.org@localhost>
Date: Thu, 2 Jan 2025 16:52:52 +0000
Subject: [PATCH] mk: Move REQUIRES generation into its own file.

This also fixes an issue seen with the generation of REQUIRES for gcc
packages.  The previous script didn't consider libraries inside DESTDIR
correctly, and for gcc ended up generating REQUIRES entries for libgcc
and libstdc++ using the system paths instead of ignoring them as they
are supplied by the package itself.
---
 mk/pkgformat/pkg/metadata.mk         | 24 +------------
 mk/pkgformat/pkg/record-requires.awk | 54 ++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 23 deletions(-)
 create mode 100644 mk/pkgformat/pkg/record-requires.awk

diff --git a/mk/pkgformat/pkg/metadata.mk b/mk/pkgformat/pkg/metadata.mk
index b74dc53d62b3..57700a8b51ab 100644
--- a/mk/pkgformat/pkg/metadata.mk
+++ b/mk/pkgformat/pkg/metadata.mk
@@ -81,29 +81,7 @@ ${_BUILD_INFO_FILE}: ${_PLIST_NOKEYWORDS}
 	ELF)								\
 		libs=`${AWK} '/\/lib.*\.so(\.[0-9]+)*$$/ { print "${DESTDIR}${PREFIX}/" $$0 } END { exit 0 }' ${_PLIST_NOKEYWORDS}`; \
 		if ${TEST} -n "$$bins" -o -n "$$libs"; then		\
-			requires=`(${PKGSRC_SETENV} elfdump -d $$bins $$libs 2>/dev/null || ${TRUE}) | ${AWK} ' \
-				/NEEDED/ {									\
-					dsolibs = dsolibs (dsolibs ? ":" : "") $$NF;				\
-				}										\
-				/RPATH/ {									\
-					nrpath = split($$NF ":${DESTDIR}${PREFIX}/lib${LIBARCHSUFFIX}:${SYSTEM_DEFAULT_RPATH}", rpath, ":"); \
-					nlibs = split(dsolibs, libs, ":");					\
-					for (l = 1; l <= nlibs; l++) {						\
-						for (r = 1; r <= nrpath; r++) {					\
-							sub(/\/$$/, "", rpath[r]);				\
-							libfile = rpath[r] "/" libs[l];				\
-							if (!(libfile in libcache)) {				\
-								libcache[libfile] = system("test -f " libfile); \
-							}							\
-							if (libcache[libfile] == 0) {				\
-								print libfile;					\
-								break;						\
-							}							\
-						}								\
-					}									\
-					dsolibs = "";								\
-				}										\
-				' | ${SED} -e 's,^${DESTDIR},,' | ${SORT} -u`; \
+			requires=`(${PKGSRC_SETENV} elfdump -d $$bins $$libs 2>/dev/null || ${TRUE}) | SYSTEM_DEFAULT_RPATH=${SYSTEM_DEFAULT_RPATH} DESTDIR=${DESTDIR} ${AWK} -f ${PKGSRCDIR}/mk/pkgformat/pkg/record-requires.awk | ${SED} -e 's,^${DESTDIR},,' | ${SORT} -u`; \
 		fi;							\
 		linklibs=`${AWK} '/.*\.so(\.[0-9]+)*$$/ { print "${DESTDIR}${PREFIX}/" $$0 }' ${_PLIST_NOKEYWORDS}`; \
 		for i in $$linklibs; do					\
diff --git a/mk/pkgformat/pkg/record-requires.awk b/mk/pkgformat/pkg/record-requires.awk
new file mode 100644
index 000000000000..cb35e12309cc
--- /dev/null
+++ b/mk/pkgformat/pkg/record-requires.awk
@@ -0,0 +1,54 @@
+#
+# Record REQUIRES entries based on illumos elfdump(1) or GNU readelf output.
+#
+
+BEGIN {
+	system_rpath = ENVIRON["SYSTEM_DEFAULT_RPATH"]
+	destdir = ENVIRON["DESTDIR"]
+}
+
+/NEEDED/ {
+	# readelf wraps the filename in "[libfoo.so]"
+	gsub(/[\[\]]/, "", $NF);
+	dsolibs = dsolibs (dsolibs ? ":" : "") $NF
+}
+
+/RPATH/ {
+	# readelf wraps the path in "[/lib/foo:/lib/bar]"
+	gsub(/[\[\]]/, "", $NF);
+
+	# Add the default system paths as they won't be included in RPATH
+	nrpath = split($NF ":" system_rpath, rpath, ":")
+	nlibs = split(dsolibs, libs, ":")
+
+	for (l = 1; l <= nlibs; l++) {
+		for (r = 1; r <= nrpath; r++) {
+			sub(/\/$/, "", rpath[r])
+
+			# Look first under DESTDIR for any libraries that the
+			# package ships, and ignore them if found.
+			libfile = destdir rpath[r] "/" libs[l]
+			if (!(libfile in libcache)) {
+				libcache[libfile] = system("test -f " libfile)
+			}
+			if (libcache[libfile] == 0) {
+				break
+			}
+
+			# Now search for installed libraries and record as
+			# dependencies.
+			libfile = rpath[r] "/" libs[l]
+			if (!(libfile in libcache)) {
+				libcache[libfile] = system("test -f " libfile)
+			}
+			if (libcache[libfile] == 0) {
+				print libfile
+				break
+			}
+		}
+	}
+
+	# Reset dsolibs as multiple elfdump outputs are combined together and
+	# parsed in one go to improve efficiency.
+	dsolibs = ""
+}