Subject: pkg/5977: some reduction of code duplication in bsd.pkg.mk
To: None <gnats-bugs@gnats.netbsd.org>
From: None <jbernard@ox.mines.edu>
List: netbsd-bugs
Date: 08/15/1998 14:01:59
>Number:         5977
>Category:       pkg
>Synopsis:       some reduction of code duplication in bsd.pkg.mk
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    gnats-admin (GNATS administrator)
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sat Aug 15 13:05:00 1998
>Last-Modified:
>Originator:     Jim Bernard
>Organization:
	Speaking for myself
>Release:        August 15, 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:
	The other day, while working on a fix to bsd.pkg.mk for PR 5962,
	I was bothered by the fairly large amount of code duplication in
	the do-fetch target, which was exacerbated by the changes I made,
	but I didn't do anything about it at the time.  Well, it's kept
	bugging me, so here's a patch that eliminates that duplication.

>How-To-Repeat:
	Read the do-fetch target.

>Fix:
	The only real differences between the loops that fetched the
	DISTFILES and the PATCHFILES were in the sites used, so I assign
	the list of sites to a shell variable before entering the loop,
	and use the shell variable instead of the make macro inside the
	loop.  Then, the entire body of the loop is assigned to a make
	variable (_FETCH_FILE), which is expanded inside the actual loops.
	Now the target is _much_ easier to read, and should be easier to
	maintain as well.

	Note that the code assigned to _FETCH_FILE is simply lifted _with
	indentation_ from the loop, which makes it easy to translate down
	into the loops either visually or with an editor.  However, that
	means it has more leading tabs than would be appropriate for a
	macro assignment, so maybe it would be better to strip off a leading
	tab from each line.  I'll leave that decision to someone else.

--- bsd.pkg.mk-dist	Sat Aug 15 05:29:01 1998
+++ bsd.pkg.mk	Sat Aug 15 13:31:47 1998
@@ -742,26 +742,21 @@
 # not happy with the default actions, and you can't solve it by
 # adding pre-* or post-* targets/scripts, override these.
 ################################################################
 
 # Fetch
-
-.if !target(do-fetch)
-do-fetch:
-	@${MKDIR} ${_DISTDIR}
-	@(cd ${_DISTDIR}; \
-	 for file in ${_DISTFILES}; do \
+_FETCH_FILE= \
 		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} ">> $$bfile doesn't seem to exist on this system."; \
-			for site in ${MASTER_SITES}; do \
+			for site in $$sites; do \
 				${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}/$$bfile`; \
 						CKSUM2=`${AWK} '$$1 == "MD5" && $$2 == "\('$$file'\)"{print $$4;}' ${MD5_FILE}`; \
@@ -776,44 +771,25 @@
 				fi \
 			done; \
 			${ECHO_MSG} ">> Couldn't fetch it - please try to retrieve this";\
 			${ECHO_MSG} ">> port manually into ${_DISTDIR} and try again."; \
 			exit 1; \
-		fi \
+		fi
+
+.if !target(do-fetch)
+do-fetch:
+	@${MKDIR} ${_DISTDIR}
+	@(cd ${_DISTDIR}; \
+	 sites="${MASTER_SITES}"; \
+	 for file in ${_DISTFILES}; do \
+		${_FETCH_FILE} \
 	 done)
 .if defined(PATCHFILES)
 	@(cd ${_DISTDIR}; \
+	 sites="${PATCH_SITES}"; \
 	 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} ">> $$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}$${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; \
-		fi \
+		${_FETCH_FILE} \
 	 done)
 .endif
 .endif
 
 # This is for the use of sites which store distfiles which others may
>Audit-Trail:
>Unformatted: