Re: grafana hits pkgsrc limits again

[Thomas Klausner <wiz%NetBSD.org@localhost>]

Here's the patch I came up with for checksum for the first issue.

On Tue, Jul 09, 2024 at 10:03:40AM +0200, Thomas Klausner wrote:
> Hi!
> 
> For the last grafana update, I had to update the distinfo code to
> handle a huge list of go module files that grafana needs.
> 
> I just tried updating grafana to 11.1.0 (see wip/grafana), and now I
> run into a similar limit in the checksum step, from
> mk/checksum/checksum.mk:
> 
> ${_CHECKSUM_CMD} ${DISTINFO_FILE:tA} ${_CKSUMFILES}
> 
> The ${_CKSUMFILES} variable gets too big for the sh(1) command line
> length limit.
> 
> So I implemented a similar change like the one I did for 'make
> distinfo' - putting the list of files to check into a file and then
> pass that to checksum.awk (see attached).
> 
> This seems to work better, but... it doesn't find a checksum for
> 'buf.build_gen_go_bufbuild_protovalidate_protocolbuffers_go_@v_v1.31.0-20230802163732-1c33ebd9ecfa.1.mod'.
> 
> I think the problem is that we ... yes, pass the whole file list again
> on the command line, this time from checksum.awk to digest(1), see
> mk/checksum/checksum.awk:
> 
>         # We now have a list of distfiles to be checked for each algorithm,
>         # pass them all to a single digest(1) command and parse the checksums                                                                                                                     # to be compared against distinfo.
>         #
>         for (algorithm in distsums) {
>                 cmd = sprintf("%s %s %s", DIGEST, algorithm,                                                                                                                                                   distsums[algorithm])
>                 while ((cmd | getline) > 0) {
> 
> and I guess that hits a shell limit again.
> 
> Suggestions for fixing this?
> 
> Passing them one by one will be a huge speed decrease for packages
> with many modules, so that's out.
> 
> Should we try passing it in groups of a hundred or a thousand? Or
> probably better teach digest(1) to accept a list of files argument?
> 
> Anyone interested in working on this?
>  Thomas

Index: checksum/checksum.awk
===================================================================
RCS file: /cvsroot/pkgsrc/mk/checksum/checksum.awk,v
retrieving revision 1.3
diff -u -r1.3 checksum.awk
--- checksum/checksum.awk	7 Oct 2020 18:09:52 -0000	1.3
+++ checksum/checksum.awk	9 Jul 2024 07:42:57 -0000
@@ -27,6 +27,8 @@
 # OPTIONS
 #	-a algorithm	Only verify checksums for the specified algorithm.
 #
+#	-f file		Read list of files to check from 'file'
+#
 #	-p		The specified files are patches, so strip out any
 #			lines containing NetBSD RCS ID tags before
 #			computing the checksums for verification.
@@ -50,6 +52,7 @@
 	only_alg = ""
 	distinfo = ""
 	exitcode = 0
+	filelist = ""
 	patch = 0
 	suffix = ""
 
@@ -57,6 +60,8 @@
 		opt = ARGV[arg]
 		if (opt == "-a") {
 			only_alg = ARGV[++arg]
+		} else if (opt == "-f") {
+			filelist = ARGV[++arg]
 		} else if (opt == "-p") {
 			patch = 1
 		} else if (opt == "-s") {
@@ -92,8 +97,16 @@
 	# order to keep things simple, distfiles[] is also used when operating
 	# in patch mode (-p).
 	#
+	arg_count = 0
+	if (filelist != "") {
+		while (getline < filelist) { arg_list[arg_count++] = $0 }
+	}
 	while (arg < ARGC) {
-		distfile = ARGV[arg++]
+		arg_list[arg_count++] = ARGV[arg++]
+	}
+	i = 0
+	while (i < arg_count) {
+		distfile = arg_list[i++]
 		sfile = distfile
 		if (suffix) {
 			sfile = strip_suffix(sfile)
@@ -241,8 +254,8 @@
 			    distfile)
 			seen[distfile] = 1
 		} else {
-			err(sprintf("%s: Checksum %s mismatch for %s",
-			    progname, algorithm, distfile))
+			err(sprintf("%s: Checksum %s mismatch for %s: expected %s, computed %s",
+				    progname, algorithm, distfile, checksum, checksums[algorithm,distfile]))
 			exit 1
 		}
 	}
Index: checksum/checksum.mk
===================================================================
RCS file: /cvsroot/pkgsrc/mk/checksum/checksum.mk,v
retrieving revision 1.30
diff -u -r1.30 checksum.mk
--- checksum/checksum.mk	15 May 2024 08:31:35 -0000	1.30
+++ checksum/checksum.mk	9 Jul 2024 07:42:57 -0000
@@ -30,22 +30,26 @@
 	    DIGEST=${TOOLS_DIGEST:Q} SED=${TOOLS_CMDLINE_SED:Q}		\
 	    ${AWK} -f ${PKGSRCDIR}/mk/checksum/checksum.awk --
 
-
 .if defined(NO_CHECKSUM) || empty(_CKSUMFILES)
 checksum checksum-phase:
 	@${DO_NADA}
 .else
 checksum checksum-phase:
+.for file in ${_CKSUMFILES}
+	@${ECHO} ${file} >> ${_DISTINFO_INPUTFILE}
+.endfor
 	${RUN} set -e;							\
 	case ${.TARGET:Q} in						\
 	*-phase)	${TEST} ! -f ${_COOKIE.checksum} || exit 0 ;;	\
 	esac;								\
-	if cd ${DISTDIR} && ${_CHECKSUM_CMD} ${DISTINFO_FILE:tA} ${_CKSUMFILES}; then \
+	if cd ${DISTDIR} && ${_CHECKSUM_CMD} -f ${_DISTINFO_INPUTFILE} ${DISTINFO_FILE:tA}; then \
+		${RM} ${_DISTINFO_INPUTFILE};				\
 		${TRUE};						\
 	else								\
 		${ERROR_MSG} "Make sure the Makefile and checksum file (${DISTINFO_FILE})"; \
 		${ERROR_MSG} "are up to date.  If you want to override this check, type"; \
 		${ERROR_MSG} "\"${MAKE} NO_CHECKSUM=yes [other args]\"."; \
+		${RM} ${_DISTINFO_INPUTFILE};				\
 		exit 1;							\
 	fi
 .endif