tech-pkg archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: suggestion: pkg_add should stat all libs in REQUIRES, at least base libs



On Tue, Jul 23, 2024 at 07:19:21AM -0400, Greg Troxel wrote:
> I saw a report of a core dump apparently from pkg_add, which I think was
> a program run by INSTALL, which failed to find a base system lib,
> because it was an X lib, and the person had not installed X sets.  While
> it's an error to install binary packages with a base system not matching
> the bulk build environment, catching it and having a better error
> message would be better.
> 
> A semi-random snippet from the middle of:
> 
> $ find . -name \+BUILD_INFO | xargs cat | egrep REQUIRES= | egrep -v /usr/pkg |sort | uniq -c
> 
>   66 REQUIRES=/usr/X11R7/lib/libxcb-render.so.0
>    5 REQUIRES=/usr/X11R7/lib/libxcb-shape.so.0
>   72 REQUIRES=/usr/X11R7/lib/libxcb-shm.so.0
>    8 REQUIRES=/usr/X11R7/lib/libxcb-sync.so.1
>   12 REQUIRES=/usr/X11R7/lib/libxcb-xfixes.so.0
>    1 REQUIRES=/usr/X11R7/lib/libxcb-xinerama.so.0
>    1 REQUIRES=/usr/X11R7/lib/libxcb-xinput.so.0
>    2 REQUIRES=/usr/X11R7/lib/libxcb-xkb.so.1
>  166 REQUIRES=/usr/X11R7/lib/libxcb.so.2
>    3 REQUIRES=/usr/X11R7/lib/libxkbfile.so.2
>    8 REQUIRES=/usr/X11R7/lib/libxshmfence.so.1
>    9 REQUIRES=/usr/lib/libarchive.so.5
>   69 REQUIRES=/usr/lib/libasn1.so.10
>    2 REQUIRES=/usr/lib/libblocklist.so.0
>  177 REQUIRES=/usr/lib/libbz2.so.1
>  617 REQUIRES=/usr/lib/libc.so.12
>   69 REQUIRES=/usr/lib/libcom_err.so.8
>   29 REQUIRES=/usr/lib/libcrypt.so.1
>  113 REQUIRES=/usr/lib/libcrypto.so.15
>   14 REQUIRES=/usr/lib/libcurses.so.9
> 
> The concrete suggestion is for pkg_add, before commmiting to add a
> package, to:
> 
>   - extract the REQUIRES list of libs
>   - for each lib that is not in $LOCALBASE
>       stat the lib
>       throw an exception if missing
> 
> where the exception looks to the user something like:
> 
>   Can't install foo-1.2.3: This package needs /usr/X11R7/lib/libxcb.so.2
>   be present, but it is not present on this system.
> 
> And perhaps extend that to all libs.  Allow -f to install anyway after
> printing the error message as a warning.
> 
> On one system I checked, I have 1296 packages and 14076 total REQUIRES
> lines.  I would suggest that 14076 stat calls is tiny compared to the
> total work to install 1296 packages, and that an average of say 12 stats
> per pkg_add will not be noticed.  But, it would add a lot of safety, and
> help people that didn't install X sets.
> 
> 
> Does anybody think this is a bad idea, or other comments?
> 
> Does anybody want to implement it?

Here's an implementation of it. I tested it with a package I faked to
add two REQUIRES lines, one for /usr/X11R7/lib/libXau.so.7 (exists)
and one for /usr/X11R7/lib/libXau.so.8 (does not exist).

# ./work/pkg_install-20240821/add/pkg_add /tmp/dummy-1.0.tgz
pkg_add: Missing required library: /usr/X11R7/lib/libXau.so.8
pkg_add: Please make sure to install the X sets
pkg_add: 1 package addition failed

The "X sets" line is only printed if the path starts with
"/usr/X11R7":

# ./work/pkg_install-20240821/add/pkg_add /tmp/dummy-1.0.tgz
pkg_add: Missing required library: /usr/not/lib/libXau.so.8
pkg_add: 1 package addition failed

Please try this out in your environments!
 Thomas
Index: files/add/perform.c
===================================================================
RCS file: /cvsroot/pkgsrc/pkgtools/pkg_install/files/add/perform.c,v
retrieving revision 1.122
diff -u -r1.122 perform.c
--- files/add/perform.c	26 Jan 2024 03:24:49 -0000	1.122
+++ files/add/perform.c	21 Aug 2024 17:58:37 -0000
@@ -1115,6 +1115,39 @@
 	return status;
 }
 
+/* check if all REQUIRES files (usually libraries) are installed */
+static int
+check_requires(struct pkg_task *pkg)
+{
+	const char *data, *eol, *next_line;
+
+	data = pkg->meta_data.meta_build_info;
+
+	for (; data != NULL && *data != '\0'; data = next_line) {
+		if ((eol = strchr(data, '\n')) == NULL) {
+			eol = data + strlen(data);
+			next_line = eol;
+		} else
+			next_line = eol + 1;
+
+		if (strncmp(data, "REQUIRES=", 9) == 0) {
+			char *library_name = dup_value(data, eol);
+			struct stat sb;
+                        if (stat(library_name, &sb) != 0
+                            || (!S_ISREG(sb.st_mode) && !S_ISLNK(sb.st_mode))) {
+				warnx("Missing required library: %s", library_name);
+				if (strncmp(library_name, "/usr/X11R7", 10) == 0) {
+					warnx("Please make sure to install the X sets");
+				}
+				return 1;
+			}
+			free(library_name);
+		}
+	}
+
+	return 0;
+}
+
 /*
  * Install a required dependency and verify its installation.
  */
@@ -1124,7 +1157,7 @@
 	/* XXX check cyclic dependencies? */
 	if (Fake || NoRecord) {
 		if (!Force) {
-			warnx("Missing dependency %s\n", dep);
+			warnx("Missing dependency %s", dep);
 			return 1;
 		}
 		warnx("Missing dependency %s, continuing", dep);
@@ -1513,6 +1546,9 @@
 	if (check_implicit_conflict(pkg))
 		goto clean_memory;
 
+	if (check_requires(pkg))
+		goto clean_memory;
+
 	if (pkg->other_version != NULL) {
 		/*
 		 * Replacing an existing package.
Index: files/lib/version.h
===================================================================
RCS file: /cvsroot/pkgsrc/pkgtools/pkg_install/files/lib/version.h,v
retrieving revision 1.193
diff -u -r1.193 version.h
--- files/lib/version.h	15 Aug 2024 02:43:58 -0000	1.193
+++ files/lib/version.h	21 Aug 2024 17:58:37 -0000
@@ -27,6 +27,6 @@
 #ifndef _INST_LIB_VERSION_H_
 #define _INST_LIB_VERSION_H_
 
-#define PKGTOOLS_VERSION 20240810
+#define PKGTOOLS_VERSION 20240821
 
 #endif /* _INST_LIB_VERSION_H_ */


Home | Main Index | Thread Index | Old Index