>Number:         5962
>Category:       pkg
>Synopsis:       bsd.pkg.mk checksum check doesn't handle distfiles subdirs
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    gnats-admin (GNATS administrator)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Aug 13 14:20:01 1998
>Last-Modified:
>Originator:     
>Organization:
	Speaking for myself
>Release:        August 13, 1998
>Environment:
System: NetBSD io 1.3G NetBSD 1.3G (FIZ) #0: Mon Aug 10 10:29:03 MDT 1998 root@io:/var/tmp/compile/sys/arch/i386/compile/FIZ i386


>Description:
	After fetching a distfile, bsd.pkg.mk contains commands to verify
	that the md5 checksum of the downloaded file is correct; however, if
	the package is set up to download its distfiles into a subdirectory
	of the distfiles directory, the checksum verification fails, because
	it looks for the file in the wrong location (it looks in the top-level
	distfiles directory).  Furthermore, the code that searches the md5
	file will not find the correct line if a subdirectory is used.

>How-To-Repeat:
	The teTeX package puts its distfiles into distfiles/teTeX, so do:

	  cd /usr/pkgsrc/print/teTeX
	  make

	And the result is (note the absence of use of the subdirectory teTeX of
	distfiles):

230 Guest login ok, access restrictions apply.
Remote system type is UNIX.
Using binary mode to transfer files.
200 Type set to I.
250 CWD command successful.
250 CWD command successful.
local: teTeX-src-0.4pl7.tar.gz remote: teTeX-src-0.4pl7.tar.gz
227 Entering Passive Mode (209,155,82,18,86,73)
150 Opening BINARY mode data connection for 'teTeX-src-0.4pl7.tar.gz' (4341637 bytes).
226 Transfer complete.
4341637 bytes received in 00:40 (105.53 KB/s)
221 Goodbye!
cannot open /usr/pkgsrc/print/teTeX/../../distfiles/teTeX-src-0.4pl7.tar.gz: no such file
*** Error code 2

Stop.
*** Error code 1

Stop.
*** Error code 1

Stop.

>Fix:
	The md5 command in the do-fetch target needs to be applied to _DISTDIR/file
	(which includes the subdirectory name), not DISTDIR/file.  But, the search
	for the filename in the stored md5 file also fails to find correct line if
	a subdirectory prefix is present (which is the case in the filenames in the
	md5 file if DIST_SUBDIR is defined--see the makesum target).  So, since
	_both_ the base filename and the filename with the DIST_SUBDIR prefix are
	needed in the target, and conditionally (depending on whether DIST_SUBDIR
	is set) inserting the prefix into the string used to search the md5 file
	would be awkward, I decided it was simplest to add the prefix in a macro
	called _DISTFILES (analogous to _CKSUMFILES), which is used to set $file,
	do the basename operation exactly once for each file (there were already
	two ${BASENAME} operations in the target, though they would usually be
	short-circuited), and use the basename everywhere except the search
	string.

	While here, I noticed an asymmetry in the fetching of the DISTFILES and
	the PATCHFILES: the checksum is verified immediately after downloading
	the former, but not the latter.  So, I added that to the latter, with
	analogous changes for handling DIST_SUBDIR (added a macro _PATCHFILES
	that has the prefix added, etc.).

	The patched bsd.pkg.mk was tested by fetching files for teTeX
	(uses DIST_SUBDIR), gnuls (uses PATCHFILES), and exmh (uses DIST_SUBDIR
	and PATCHFILES).

	It could be argued that a similar addition of immediate checksum verification
	should be made to the fetch-list-one-pkg target too, but I didn't do so
	in the patch below.

--- bsd.pkg.mk-dist	Wed Aug 12 05:21:48 1998
+++ bsd.pkg.mk	Thu Aug 13 13:24:02 1998
@@ -490,17 +490,21 @@
 	done
 .else
 CKSUMFILES=		${ALLFILES}
 .endif
 
-# List of all files, with ${DIST_SUBDIR} in front.  Used for checksum.
+# List of all files, with ${DIST_SUBDIR} in front.  Used for fetch and checksum.
 .if defined(DIST_SUBDIR)
 _CKSUMFILES?=	${CKSUMFILES:S/^/${DIST_SUBDIR}\//}
+_DISTFILES?=	${DISTFILES:S/^/${DIST_SUBDIR}\//}
 _IGNOREFILES?=	${IGNOREFILES:S/^/${DIST_SUBDIR}\//}
+_PATCHFILES?=	${PATCHFILES:S/^/${DIST_SUBDIR}\//}
 .else
 _CKSUMFILES?=	${CKSUMFILES}
+_DISTFILES?=	${DISTFILES}
 _IGNOREFILES?=	${IGNOREFILES}
+_PATCHFILES?=	${PATCHFILES}
 .endif
 
 # This is what is actually going to be extracted, and is overridable
 #  by user.
 EXTRACT_ONLY?=	${DISTFILES}
@@ -743,24 +747,25 @@
 
 .if !target(do-fetch)
 do-fetch:
 	@${MKDIR} ${_DISTDIR}
 	@(cd ${_DISTDIR}; \
-	 for file in ${DISTFILES}; do \
-		if [ ! -f $$file -a ! -f `${BASENAME} $$file` ]; then \
-			if [ -h $$file -o -h `${BASENAME} $$file` ]; then \
-				${ECHO_MSG} ">> ${_DISTDIR}/$$file is a broken symlink."; \
+	 for file in ${_DISTFILES}; do \
+		bfile=`${BASENAME} $$file`; \
+		if [ ! -f $$file -a ! -f $$bfile ]; then \
+			if [ -h $$file -o -h $$bfile ]; then \
+				${ECHO_MSG} ">> ${_DISTDIR}/$$bfile is a broken symlink."; \
 				${ECHO_MSG} ">> Perhaps a filesystem (most likely a CD) isn't mounted?"; \
 				${ECHO_MSG} ">> Please correct this problem and try again."; \
 				exit 1; \
 			fi ; \
-			${ECHO_MSG} ">> $$file doesn't seem to exist on this system."; \
+			${ECHO_MSG} ">> $$bfile doesn't seem to exist on this system."; \
 			for site in ${MASTER_SITES}; do \
-				${ECHO_MSG} ">> Attempting to fetch $$file from $${site}."; \
-				if ${FETCH_CMD} ${FETCH_BEFORE_ARGS} $${site}$${file} ${FETCH_AFTER_ARGS}; then \
+				${ECHO_MSG} ">> Attempting to fetch $$bfile from $${site}."; \
+				if ${FETCH_CMD} ${FETCH_BEFORE_ARGS} $${site}$${bfile} ${FETCH_AFTER_ARGS}; then \
 					if [ -f ${MD5_FILE} ]; then \
-						CKSUM=`${MD5} < ${DISTDIR}/$$file`; \
+						CKSUM=`${MD5} < ${_DISTDIR}/$$bfile`; \
 						CKSUM2=`${AWK} '$$1 == "MD5" && $$2 == "\('$$file'\)"{print $$4;}' ${MD5_FILE}`; \
 						if [ "$$CKSUM" = "$$CKSUM2" -o "$$CKSUM2" = "IGNORE" ]; then \
 							continue 2; \
 						else \
 							${ECHO_MSG} ">> Checksum failure - trying next site."; \
@@ -775,23 +780,34 @@
 			exit 1; \
 		fi \
 	 done)
 .if defined(PATCHFILES)
 	@(cd ${_DISTDIR}; \
-	 for file in ${PATCHFILES}; do \
-		if [ ! -f $$file -a ! -f `${BASENAME} $$file` ]; then \
-			if [ -h $$file -o -h `${BASENAME} $$file` ]; then \
-				${ECHO_MSG} ">> ${_DISTDIR}/$$file is a broken symlink."; \
+	 for file in ${_PATCHFILES}; do \
+		bfile=`${BASENAME} $$file`; \
+		if [ ! -f $$file -a ! -f $$bfile ]; then \
+			if [ -h $$file -o -h $$bfile ]; then \
+				${ECHO_MSG} ">> ${_DISTDIR}/$$bfile is a broken symlink."; \
 				${ECHO_MSG} ">> Perhaps a filesystem (most likely a CD) isn't mounted?"; \
 				${ECHO_MSG} ">> Please correct this problem and try again."; \
 				exit 1; \
 			fi ; \
-			${ECHO_MSG} ">> $$file doesn't seem to exist on this system."; \
+			${ECHO_MSG} ">> $$bfile doesn't seem to exist on this system."; \
 			for site in ${PATCH_SITES}; do \
 			    ${ECHO_MSG} ">> Attempting to fetch from $${site}."; \
-				if ${FETCH_CMD} ${FETCH_BEFORE_ARGS} $${site}$${file} ${FETCH_AFTER_ARGS}; then \
-					continue 2; \
+				if ${FETCH_CMD} ${FETCH_BEFORE_ARGS} $${site}$${bfile} ${FETCH_AFTER_ARGS}; then \
+					if [ -f ${MD5_FILE} ]; then \
+						CKSUM=`${MD5} < ${_DISTDIR}/$$bfile`; \
+						CKSUM2=`${AWK} '$$1 == "MD5" && $$2 == "\('$$file'\)"{print $$4;}' ${MD5_FILE}`; \
+						if [ "$$CKSUM" = "$$CKSUM2" -o "$$CKSUM2" = "IGNORE" ]; then \
+							continue 2; \
+						else \
+							${ECHO_MSG} ">> Checksum failure - trying next site."; \
+						fi; \
+					else \
+						continue 2; \
+					fi; \
 				fi \
 			done; \
 			${ECHO_MSG} ">> Couldn't fetch it - please try to retrieve this";\
 			${ECHO_MSG} ">> port manually into ${_DISTDIR} and try again."; \
 			exit 1; \
>Audit-Trail:
>Unformatted: