Subject: Re: Builk build vs. p5-Test-Harness
To: Alistair Crooks <agc@pkgsrc.org>
From: Krister Walfridsson <cato@df.lth.se>
List: tech-pkg
Date: 06/22/2004 23:08:41
On Tue, 22 Jun 2004, Alistair Crooks wrote:

> > I suggest changing this to:
> >
> >   for each dependency
> >     if "dependency is broken"
> >       fail
> >     install dependency
> >   build
> >
> > This is somewhat tricker than what my pseudo code above indicates (for
> > example, we need to keep track of dependencies in the form as they are
> > written in the DEPENDS, rather than just the directories as they are now)
> > but I think this can be done with only minor changes to bsd.bulk-pkg.mk.
>
> I'd be interested to know if your pseudo-code above works - can you elaborate
> a bit more on the change in dependency tracking, please?

Hmm.  I guess my pseudo code was a bit too simplified, so let's
look at devel/p5-Test-Simple as an example.

The first stage of the bulk build is as previous -- we create a
.depends file with the line:

  devel/p5-Test-Simple depends on:  devel/p5-Test-Harness lang/perl58

We know that these lines have all necessary depends, but they may
have some dependencies that are not really needed.


The build churns on, and does eventually reach devel/p5-Test-Harness.
This pkg fails, so a .broken.html file is placed in
pkgsrc/devel/p5-Test-Harness (but the dependencies are not marked as
broken yet).


The build continues, and reaches devel/p5-Test-Simple.  Here we do
not use the dependencies from the .depends file -- we use the
dependencies from DEPENDS and BUILD_DEPENDS instead:

  {perl>=5.0,perl-thread>=5.0}:../../lang/perl58
  {perl{,-thread}>=5.8.3,p5-Test-Harness-[0-9]*}:../../devel/p5-Test-Harness

We start with {perl>=5.0,perl-thread>=5.0}:../../lang/perl58.
- Is {perl>=5.0,perl-thread>=5.0} already installed?
- No.
- OK, does ../../lang/perl58/.broken.html exist?
- No.
- This is good, let's install it, and proceed with the next dependency.
- Is {perl{,-thread}>=5.8.3,p5-Test-Harness-[0-9]*}
- Yes
- Good. Let's build the package.
[...]
- Success!


Note that it is important that the dependencies are in the correct order,
and I'm not sure how we should ensure that (the current source
always places bl3 dependencies after explicit DEPENDS, which actually
makes devel/p5-Test-Simple fail.  I cheated in my example above...)


I have implemented this, and it seems to work (I hacked around the
ordering problem by always moving perl to the beginning of the list...)

I have attached the patch below.  It seems to work, but there are still
much to do before it is ready for prime time...

   /Krister



Index: bsd.bulk-pkg.mk
===================================================================
RCS file: /cvsroot/pkgsrc/mk/bulk/bsd.bulk-pkg.mk,v
retrieving revision 1.60
diff -b -u -r1.60 bsd.bulk-pkg.mk
--- bsd.bulk-pkg.mk	10 Apr 2004 16:23:00 -0000	1.60
+++ bsd.bulk-pkg.mk	22 Jun 2004 20:29:05 -0000
@@ -217,6 +217,44 @@
 	fi ; \
 	${ECHO_MSG} $$uptodate

+.if defined(BUILDLINK_DEPENDS.perl) && defined(BUILDLINK_PKGSRCDIR.perl)
+HACKDEPENDS+=	${BUILDLINK_DEPENDS.perl}:${BUILDLINK_PKGSRCDIR.perl}
+.endif
+
+bulk-preinstall:
+	@${ECHO_MSG} "BULK> Installing packages which are required to build ${PKGNAME}." ;
+.	for dep in ${HACKDEPENDS} ${DEPENDS} ${BUILD_DEPENDS}
+	  @pkg="${dep:C/:.*//}";					\
+	  reldir="${dep:C/[^:]*://:C/:.*$//}";				\
+	  thisdir=`${PWD_CMD}`;						\
+	  cd $$thisdir/$$reldir;					\
+	  WD=`${PWD_CMD}`;						\
+	  d=`dirname $$WD`;						\
+	  pkgdir=`basename $$d`/`basename $$WD`;			\
+	  pkgname=`${GREP} "^$$pkgdir " ${INDEXFILE} | ${AWK} '{print $$2}'` ; \
+	  cd $$thisdir;							\
+	  if [ -z "$$pkgname" ]; then continue ; fi ;\
+	  if [ -f ${_PKGSRCDIR}/$$pkgdir/${BROKENFILE} ]; then \
+		${ECHO_MSG} "BULK> Required package $$pkgname ($$pkgdir) is broken" ; \
+		false ; \
+	  fi ; \
+	  found=`${PKG_BEST_EXISTS} "$$pkg" || ${TRUE}`;		\
+	  if [ "X$$found" != "X" ]; then \
+		${ECHO_MSG} "BULK> Required package $$pkgname ($$pkgdir) is already installed" ; \
+	  else \
+		pkgfile=${PACKAGES}/All/$${pkgname}.tgz ;\
+		if [ -f $$pkgfile ]; then \
+			${ECHO_MSG} "BULK> ${PKG_ADD} ${PKG_ARGS_ADD} $$pkgfile"; \
+			${DO} ${PKG_ADD} ${PKG_ARGS_ADD} $$pkgfile || ${ECHO_MSG} "warning: could not add $$pkgfile." ; \
+		else \
+			${ECHO_MSG} "BULK> warning: $$pkgfile does not exist.  It will be rebuilt." ;\
+		fi ;\
+	  fi ;
+.	endfor
+
+
+
+
 # rebuild binpkg if any of the pkg files is newer than the binary archive
 # set DO to ":" to not actually do anything (debugging, ...)
 bulk-package:
@@ -318,23 +356,7 @@
 			done ; \
 		fi ;\
 		if [ "${USE_BULK_CACHE}" = "yes" ]; then \
-			${SHCOMMENT} "Install required depends via binarypkgs XXX" ; \
-			${ECHO_MSG} "BULK> Installing packages which are required to build ${PKGNAME}." ;\
-			for pkgdir in `${GREP} "^${PKGPATH} " ${DEPENDSFILE} | ${SED} -e 's;^.*:;;g'` ${BULK_PREREQ} ; do \
-				pkgname=`${GREP} "^$$pkgdir " ${INDEXFILE} | ${AWK} '{print $$2}'` ; \
-				if [ -z "$$pkgname" ]; then continue ; fi ;\
-				pkgfile=${PACKAGES}/All/$${pkgname}.tgz ;\
-				if ${PKG_INFO} -qe $$pkgname ; then \
-					${ECHO_MSG} "BULK> Required package $$pkgname ($$pkgdir) is already installed" ; \
-				else \
-					if [ -f $$pkgfile ]; then \
-						${ECHO_MSG} "BULK> ${PKG_ADD} ${PKG_ARGS_ADD} $$pkgfile"; \
-						${DO} ${PKG_ADD} ${PKG_ARGS_ADD} $$pkgfile || ${ECHO_MSG} "warning: could not add $$pkgfile." ; \
-					else \
-						${ECHO_MSG} "BULK> warning: $$pkgfile does not exist.  It will be rebuilt." ;\
-					fi ;\
-				fi ;\
-			done ;\
+			${MAKE} bulk-preinstall ; \
 		fi ;\
 		if [ -f ${INTERACTIVE_COOKIE} ]; then \
 			${ECHO_MSG} "BULK> Removing old marker for INTERACTIVE_STAGE..." ; \
@@ -354,35 +376,6 @@
 			${ECHO_MSG} ${MAKE} deinstall ; \
 			${DO}       ${MAKE} deinstall ; \
 			nbrokenby=0;\
-			if [ "${USE_BULK_CACHE}" = "yes" ]; then \
-				${ECHO_MSG} "BULK> Marking all packages which depend upon ${PKGNAME} as broken:"; \
-				for pkgdir in `${GREP} "^${PKGPATH} " ${SUPPORTSFILE} | ${SED} -e 's;^.*:;;g'`; do \
-					pkgname=`${GREP} "^$$pkgdir " ${INDEXFILE} | ${AWK} '{print $$2}'` ;\
-					if [ -z "$$pkgname" ]; then pkgname=unknown ; fi ; \
-					${ECHO_MSG} "BULK> marking package that requires ${PKGNAME} as broken: $$pkgname ($$pkgdir)";\
-					pkgerr="-1"; \
-					pkgignore=`(cd ${_PKGSRCDIR}/$$pkgdir && ${MAKE} show-var VARNAME=PKG_FAIL_REASON)`; \
-					pkgskip=`(cd ${_PKGSRCDIR}/$$pkgdir && ${MAKE} show-var VARNAME=PKG_SKIP_REASON)`; \
-					if [ ! -z "$${pkgignore}$${pkgskip}" -a ! -f ${_PKGSRCDIR}/$$pkgdir/${BROKENFILE} ]; then \
-						 ${ECHO_MSG} "BULK> $$pkgname ($$pkgdir) may not be packaged because:" >> ${_PKGSRCDIR}/$$pkgdir/${BROKENFILE};\
-						 ${ECHO_MSG} "BULK> $$pkgignore" >> ${_PKGSRCDIR}/$$pkgdir/${BROKENFILE};\
-						 ${ECHO_MSG} "BULK> $$pkgskip" >> ${_PKGSRCDIR}/$$pkgdir/${BROKENFILE};\
-						if [ -z "`(cd ${_PKGSRCDIR}/$$pkgdir && ${MAKE} show-var VARNAME=BROKEN)`" ]; then \
-							pkgerr="0"; \
-						else \
-							pkgerr="1"; \
-						fi; \
-					fi; \
-					${ECHO_MSG} "BULK> $$pkgname ($$pkgdir) is broken because it depends upon ${PKGNAME} (${PKGPATH}) which is broken." \
-						>> ${_PKGSRCDIR}/$$pkgdir/${BROKENFILE};\
-					${ECHO_MSG} "Please view the <a href=\"../../${PKGPATH}/${BROKENFILE}\">build log for ${PKGNAME}</a>" \
-						>> ${_PKGSRCDIR}/$$pkgdir/${BROKENFILE};\
-					nbrokenby=`expr $$nbrokenby + 1`;\
-					if ${GREP} -q " $$pkgdir/${BROKENFILE}" ${_PKGSRCDIR}/${BROKENFILE} ; then :; else \
-						${ECHO} " $$pkgerr $$pkgdir/${BROKENFILE} 0 " >> ${_PKGSRCDIR}/${BROKENFILE} ;\
-					fi ;\
-				done ;\
-			fi ;\
 			nerrors=`${GREP} -c '^\*\*\* Error code' ${BROKENFILE} || true`; \
 			if [ -f ${INTERACTIVE_COOKIE} ]; then \
 				nerrors="0"; \