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