pkg_admin check fixes

README.md

The current implementation of pkg_admin check has a few issues:

• It always exits 0 regardless of whether there were errors
• Errors are printed to stdout
• By default progress dots are printed, making the output rather unreadable.
• Even in quiet mode (-q) it prints a summary at the end.

Example:

$ pkg_admin check; echo $?
./opt/pkg/lib/ruby/gems/3.1.0/doc/i18n-1.14.1/ri/I18n/Gettext/Helpers/N_-i.ri fails MD5 checksum
......../opt/pkg/lib/ruby/gems/3.1.0/doc/em-websocket-0.5.3/ri/EventMachine/WebSocket/cdesc-WebSocket.ri fails MD5 checksum
.........................................................../opt/pkg/lib/ruby/gems/3.1.0/doc/kramdown-2.4.0/ri/Kramdown/Converter/HashAST/cdesc-HashAST.ri fails MD5 checksum
............................................................................................................................................/opt/pkg/lib/ruby/gems/3.1.0/doc/http_parser.rb-0.8.0/ri/HTTP/cdesc-HTTP.ri fails MD5 checksum
.................................................../opt/pkg/share/ri/3.1.0/system/Net/HTTPGatewayTimeOut/cdesc-HTTPGatewayTimeOut.ri fails MD5 checksum
/opt/pkg/share/ri/3.1.0/system/Net/HTTPRequestTimeOut/cdesc-HTTPRequestTimeOut.ri fails MD5 checksum
...........
Checked 134888 files from 270 packages.
Done.
0

The following patch fixes all of the above issues.

Index: files/admin/admin.h
===================================================================
RCS file: /cvsroot/pkgsrc/pkgtools/pkg_install/files/admin/admin.h,v
retrieving revision 1.6
diff -u -r1.6 admin.h
--- files/admin/admin.h	2 Feb 2009 12:35:00 -0000	1.6
+++ files/admin/admin.h	8 Sep 2023 10:54:13 -0000
@@ -27,7 +27,7 @@
 extern int quiet;
 extern int verbose;
 
-void 	check(char **);
+int 	check(char **);
 
 void	audit_pkgdb(int, char **);
 void	audit_pkg(int, char **);
Index: files/admin/check.c
===================================================================
RCS file: /cvsroot/pkgsrc/pkgtools/pkg_install/files/admin/check.c,v
retrieving revision 1.12
diff -u -r1.12 check.c
--- files/admin/check.c	2 Dec 2020 10:45:47 -0000	1.12
+++ files/admin/check.c	8 Sep 2023 10:54:13 -0000
@@ -83,12 +83,13 @@
 /*
  * Assumes CWD is in the database directory ($PREFIX/pkgdb/<pkg>)!
  */
-static void 
+static int
 check1pkg(const char *pkgdir, int *filecnt, int *pkgcnt)
 {
 	FILE   *f;
 	plist_t *p;
 	package_t Plist;
+	int	rv = 0;
 	char   *PkgName, *dirp = NULL, *md5file;
 	char    file[MaxPathSize];
 	char   *content;
@@ -120,8 +121,10 @@
 					if (strncmp(p->next->name, CHECKSUM_HEADER, ChecksumHeaderLen) == 0) {
 						if ((md5file = MD5File(file, NULL)) != NULL) {
 							/* Mismatch? */
-							if (strcmp(md5file, p->next->name + ChecksumHeaderLen) != 0)
-								printf("%s fails MD5 checksum\n", file);
+							if (strcmp(md5file, p->next->name + ChecksumHeaderLen) != 0) {
+								fprintf(stderr, "%s fails MD5 checksum\n", file);
+								rv = 1;
+							}
 
 							free(md5file);
 						}
@@ -133,11 +136,13 @@
 						if ((cc = readlink(file, &buf[SymlinkHeaderLen],
 							  sizeof(buf) - SymlinkHeaderLen - 1)) < 0) {
 							warnx("can't readlink `%s'", file);
+							rv = 1;
 						} else {
 							buf[SymlinkHeaderLen + cc] = 0x0;
 							if (strcmp(buf, p->next->name) != 0) {
-								printf("symlink (%s) is not same as recorded value, %s: %s\n",
+								fprintf(stderr, "symlink (%s) is not same as recorded value, %s: %s\n",
 								    file, buf, p->next->name);
+								rv = 1;
 							}
 						}
 					}
@@ -146,8 +151,10 @@
 				(*filecnt)++;
 			} else if (isbrokenlink(file)) {
 				warnx("%s: Symlink `%s' exists and is in %s but target does not exist!", PkgName, file, CONTENTS_FNAME);
+				rv = 1;
 			} else {
 				warnx("%s: File `%s' is in %s but not on filesystem!", PkgName, file, CONTENTS_FNAME);
+				rv = 1;
 			}
 			break;
 		case PLIST_CWD:
@@ -181,11 +188,14 @@
 	free_plist(&Plist);
 	fclose(f);
 	(*pkgcnt)++;
+
+	return rv;
 }
 
 struct checkpattern_arg {
 	int filecnt;
 	int pkgcnt;
+	int errcnt;
 	int got_match;
 };
 
@@ -194,16 +204,15 @@
 {
 	struct checkpattern_arg *arg = vp;
 
-	check1pkg(pkg, &arg->filecnt, &arg->pkgcnt);
-	if (!quiet)
-		printf(".");
+	if (check1pkg(pkg, &arg->filecnt, &arg->pkgcnt) != 0)
+		arg->errcnt++;
 
 	arg->got_match = 1;
 
 	return 0;
 }
 
-static void
+static int
 check_pkg(const char *pkg, int *filecnt, int *pkgcnt, int allow_unmatched)
 {
 	struct checkpattern_arg arg;
@@ -211,6 +220,7 @@
 
 	arg.filecnt = *filecnt;
 	arg.pkgcnt = *pkgcnt;
+	arg.errcnt = 0;
 	arg.got_match = 0;
 
 	if (match_installed_pkgs(pkg, checkpattern_fn, &arg) == -1)
@@ -218,12 +228,12 @@
 	if (arg.got_match != 0) {
 		*filecnt = arg.filecnt;
 		*pkgcnt = arg.pkgcnt;
-		return;
+		return (arg.errcnt) ? 1 : 0;
 	}
 
 	if (ispkgpattern(pkg)) {
 		if (allow_unmatched)
-			return;
+			return 0;
 		errx(EXIT_FAILURE, "No matching pkg for %s.", pkg);
 	}
 
@@ -238,26 +248,31 @@
 
 	*filecnt = arg.filecnt;
 	*pkgcnt = arg.pkgcnt;
+
+	return (arg.errcnt) ? 1 : 0;
 }
 
-void
+int
 check(char **argv)
 {
-	int filecnt, pkgcnt;
+	int filecnt, pkgcnt, errcnt = 0;
 
 	filecnt = 0;
 	pkgcnt = 0;
 	setbuf(stdout, NULL);
 
 	if (*argv == NULL) {
-		check_pkg("*", &filecnt, &pkgcnt, 1);
+		errcnt += check_pkg("*", &filecnt, &pkgcnt, 1);
 	} else {
 		for (; *argv != NULL; ++argv)
-			check_pkg(*argv, &filecnt, &pkgcnt, 0);
+			errcnt += check_pkg(*argv, &filecnt, &pkgcnt, 0);
+	}
+
+	if (!quiet) {
+		printf("Checked %d file%s from %d package%s.\n",
+		    filecnt, (filecnt == 1) ? "" : "s",
+		    pkgcnt, (pkgcnt == 1) ? "" : "s");
 	}
 
-	printf("\n");
-	printf("Checked %d file%s from %d package%s.\n",
-	    filecnt, (filecnt == 1) ? "" : "s",
-	    pkgcnt, (pkgcnt == 1) ? "" : "s");
+	return (errcnt) ? 1: 0;
 }
Index: files/admin/main.c
===================================================================
RCS file: /cvsroot/pkgsrc/pkgtools/pkg_install/files/admin/main.c,v
retrieving revision 1.69
diff -u -r1.69 main.c
--- files/admin/main.c	2 Dec 2020 10:45:47 -0000	1.69
+++ files/admin/main.c	8 Sep 2023 10:54:13 -0000
@@ -458,7 +458,7 @@
 	char		 lsdir[MaxPathSize];
 	char		 sfx[MaxPathSize];
 	char		*lsdirp = NULL;
-	int		 ch;
+	int		 ch, rv;
 
 	setprogname(argv[0]);
 
@@ -561,12 +561,14 @@
 	} else if (strcasecmp(argv[0], "check") == 0) {
 		argv++;		/* "check" */
 
-		check(argv);
+		rv = check(argv);
 
 		if (!quiet) {
 			printf("Done.\n");
 		}
 
+		return rv;
+
 	} else if (strcasecmp(argv[0], "lsall") == 0) {
 		argv++;		/* "lsall" */
 

Behaviour and testing post-patch:

• Only errors are printed in quiet mode, no dots, exit status is correct.

$ pkg_admin -q check; echo $?
/opt/pkg/lib/ruby/gems/3.1.0/doc/i18n-1.14.1/ri/I18n/Gettext/Helpers/N_-i.ri fails MD5 checksum
/opt/pkg/lib/ruby/gems/3.1.0/doc/em-websocket-0.5.3/ri/EventMachine/WebSocket/cdesc-WebSocket.ri fails MD5 checksum
/opt/pkg/lib/ruby/gems/3.1.0/doc/kramdown-2.4.0/ri/Kramdown/Converter/HashAST/cdesc-HashAST.ri fails MD5 checksum
/opt/pkg/lib/ruby/gems/3.1.0/doc/http_parser.rb-0.8.0/ri/HTTP/cdesc-HTTP.ri fails MD5 checksum
/opt/pkg/share/ri/3.1.0/system/Net/HTTPGatewayTimeOut/cdesc-HTTPGatewayTimeOut.ri fails MD5 checksum
/opt/pkg/share/ri/3.1.0/system/Net/HTTPRequestTimeOut/cdesc-HTTPRequestTimeOut.ri fails MD5 checksum
1

$ pkg_admin check ruby31-i18n-1.14.1; echo $?
/opt/pkg/lib/ruby/gems/3.1.0/doc/i18n-1.14.1/ri/I18n/Gettext/Helpers/N_-i.ri fails MD5 checksum
Checked 425 files from 1 package.
Done.
1

• Patterns are correct for matches, non-matches, and non-existent packages:

$ pkg_admin check 'coreutils-[0-9]*'; echo $?
Checked 510 files from 1 package.
Done.
0

$ pkg_admin -q check coreutils; echo $?
0

$ pkg_admin check 'oreutils-[0-9]*'; echo $?
pkg_admin: No matching pkg for oreutils-[0-9]*.
1

$ pkg_admin check oreutils; echo $?
pkg_admin: cannot find package oreutils
1