cvs commit: src/release Makefile

[Ruslan Ermilov]

On Thu, Jun 05, 2003 at 02:01:08PM -0700, Marcel Moolenaar wrote:
> On Thu, Jun 05, 2003 at 10:56:11PM +0300, Ruslan Ermilov wrote:
> > > > I think this is not quite right.  Instead, the /tmp/.ports_done
> > > > should be created, similar to /tmp/.world_done, when ports are
> > > > really done.
> > > 
> > > It is. The wording "complete run" is confusing. I meant a complete
> > > run of the make readmes, not of the release itself.
> > > 
> > Not quite.  You also create it when NOPORTREADMES is set.
> > Ports (readmes) aren't created in this case.  Er, and I
> > meant /tmp/.readmes_done of course.
> 
> That's why I called the file .skip_ports. It doesn't mean readmes
> are made, it means that they shouldn't be made (ie skipped). This
> happens when readmes should not be made at all, or when they have
> been made already. I've been more careful than you think :-)
> 
I noticed you were careful, I did.  But then please don't say
"it is", because I meant what I said, "when ports^Wreadmes are
really done".  ;)

> > > > When NOPORTS or NOPORTREADMES are defined, we should just not be
> > > > doing the relevant parts of "make release".
> > > 
> > > Which is exactly what happens. By making the different parts of
> > > a release cycle optional by checking for the existence of files,
> > > you can more easily interfere by creating files or removing them.
> > > Files are also a good way to maintain state across make invocations.
> > > 
> > What bugs me here is that we also (ab)use the /tmp/.skip_ports for
> > remembering "NOPORTS || NOPORTREADMES".
> 
> It's not abuse, because it's designed for that purpose. Hence, you
> don't like my design. Fair enough. Let me explain how I looked at
> it:
> 
Mkay, you didn't mention it in the commit log.

> I think ``make release [list of defines]'' should in principle be
> restarted with ``make rerelease [same list of defines]'', where
> [same list of defines] is functional equivalent to [list of defines].
> Exceptions are defines that control how you want the release to be
> restarted (ie -DRELEASENOUPDATE).
> 
"make rerelease -DRELEASENOUPDATE" is normal, but your test
case was "make release -DNOPORTS" and then "make rerelease
-DRELEASENOUPDATE", which I consider as one of many ways
to foot shoot themselves.  ;)

> Second order functionality such as the ability to skip ports for the
> release but have them made for the rerelease is nice, but given our
> current release process is flawed. Hence, second order functionality.
> 
Agreed, this is secondary.

> This change was made because 5.1-RC1 for ia64 got severily delayed
> due to having to restart it often (due to instability) and having no
> way (other than defines like NOPORTREADMES or NOPORTS) to skip remaking
> the readmes. Using the defines is highly unwanted because there may
> be consequences for the final output. Whether it would in this case
> is not really important...
> 
It is clear.  I like the change in principle, just one thing I don't
like in implementation. ;)

> > > > And of course, it
> > > > should be possible to run "make -DNOPORTS release" first, and be
> > > > able to run "make rerelease" later, and get the ports built.
> > > 
> > > It's not that simple AFAICT. If you follow a release -DNOPORTS -DNODOC
> > > with a rerelease -DRELEASENOUPDATE, you won't have a ports tree
> > > at all so you cannot possibly expect to have all the readmes built.
> > > Your rerelease will probably fail.
> > > 
> > We could check if /usr/ports/Makefile exists, but this case is
> > a self-foot-shooting IMO.  If you know you didn't check out ports
> > sources with "make release", and expect "make rerelease" to build
> > ports readmes, it should be clear that you don't want
> > -DRELEASENOUPDATE.
> 
> Again, it's not that simple (unfortunately). One may need to update
> the source tree and at the same time want ports. The point here is
> not that we should allow al this fickleness, but that we have many
> knobs and we have a lot of flexibility and most combinations simply
> don't work if you don't have a FreeBSD Release GURU badge nailed to
> your forehead and know how to circumvent the pitfalls.
> 
Who said that this should be simple?  ;)

> > I'm actually thinking of getting rid of "rerelease", and instead
> > adding the -DNOCLEAN knob to "make release" to replace it.
> 
> Good thinking. I don't know how feasible it is, but it's definitely
> something we should consider. Our release process has grown out of
> control in some ways. Fresh solutions are needed...
> 
Mail me your concerns, I wish to consider them.

> > Yes, this change is needed in either case.  It means: if we update
> > the ports sources, we should be regenerating readmes.  How about
> > this instead (without anti-footshooting measure)?
> > 
> > %%%
> > Index: Makefile
> > ===================================================================
> > RCS file: /home/ncvs/src/release/Makefile,v
> > retrieving revision 1.782
> > diff -u -r1.782 Makefile
> > --- Makefile	4 Jun 2003 22:24:43 -0000	1.782
> > +++ Makefile	5 Jun 2003 19:34:07 -0000
> > @@ -384,6 +384,7 @@
> >  .if !defined(NOPORTS)
> >  	cd ${CHROOTDIR}/usr/ports && ${CVSPREFIX} cvs -R ${CVSARGS} -q update ${CVSCMDARGS} -P -d
> >  .endif
> > +	rm -f ${CHROOTDIR}/tmp/.readmes_done
> >  .if defined(DOMINIMALDOCPORTS) && ${DOMINIMALDOCPORTS} == "YES"
> >  	for i in ${MINIMALDOCPORTS}; do \
> >  		( cd ${CHROOTDIR}/usr/$$i && ${CVSPREFIX} cvs -R ${CVSARGS} -q update ${CVSCMDARGS} -P -d ) ; \
> > @@ -453,21 +454,20 @@
> >  	echo "	${CROSSMAKE} ${WORLD_FLAGS} -DNOCLEAN buildworld && \\" >> ${CHROOTDIR}/mk
> >  	echo "	touch /tmp/.world_done"		>> ${CHROOTDIR}/mk
> >  	echo "fi"				>> ${CHROOTDIR}/mk
> > -	echo "if [ ! -f /tmp/.skip_ports ]; then" >> ${CHROOTDIR}/mk
> > +.if defined(NOPORTS) || defined(NOPORTREADMES)
> > +	echo "if [ ! -f /tmp/.readmes_done ]; then" >> ${CHROOTDIR}/mk
> >  	echo "	echo \">>> make readmes started on \`LC_ALL=C TZ=GMT date\`\"" >> ${CHROOTDIR}/mk
> >  	echo "	cd /usr/ports"			>> ${CHROOTDIR}/mk
> >  	echo "	make ${PORTREADMES_FLAGS} readmes" >> ${CHROOTDIR}/mk
> > -	echo "	touch /tmp/.skip_ports"		>> ${CHROOTDIR}/mk
> > +	echo "	touch /tmp/.readmes_done"	>> ${CHROOTDIR}/mk
> >  	echo "	echo \">>> make readmes finished on \`LC_ALL=C TZ=GMT date\`\"" >> ${CHROOTDIR}/mk
> >  	echo "fi"				>> ${CHROOTDIR}/mk
> > +.endif
> >  	echo "cd /usr/src/release"		>> ${CHROOTDIR}/mk
> >  	echo "make obj"				>> ${CHROOTDIR}/mk
> >  	echo "make \$${_RELTARGET}"		>> ${CHROOTDIR}/mk
> >  	echo "echo \">>> make ${.TARGET} for ${TARGET} finished on \`LC_ALL=C TZ=GMT date\`\"" >> ${CHROOTDIR}/mk
> >  	chmod 755 ${CHROOTDIR}/mk
> > -.if defined(NOPORTS) || defined(NOPORTREADMES)
> > -	touch ${CHROOTDIR}/tmp/.skip_ports
> > -.endif
> >  	# Ensure md.ko is loaded if md(4) is not statically compiled into the kernel
> >  	-mdconfig 2>/dev/null
> >  	env -i /usr/sbin/chroot ${CHROOTDIR} /mk
> > %%%
> 
> It's not an anti-foot-shooting measure you're removing. It's the
> basic functionality to have all the commands present in
> ${CHROOTDIR}/mk in all cases, even if you don't actually use them.
> It has been suggested before (in the 5.0-R timeframe) that having
> the commands to make the readmes in ${CHROOT}/mk helps to manually
> fiddle with the process. That's why the previous version had 
> 
> 	.if defined(NOPORTS) || defined(NOPORTREADMES)
> 		echo "if false; then"		>> ${CHROOTDIR}/mk
> 	.else
> 		echo "if true; then"		>> ${CHROOTDIR}/mk
> 	.endif
> 
How the "if false; then cd /usr/ports && make readmes" is really
helpful?  How this is better than just "cd /usr/ports && make
readmes" manually?  But okay, I agree we shouldn't be doing
this in general.

> However, this resulted in unnecessary remakes of readmes and was
> changed to the version we have now.
> 
Clear.

> So: please do not conditionally put the commands in ${CHROOTDIR}/mk.
> Have them unconditionally there, but have them conditionally executed.
> 
Sure, how is this then (better viewed as the diff to 1.780)?
Feel free to commit it if you like it, I'm going to sleep
now. ;)


Cheers,
-- 
Ruslan Ermilov		Sysadmin and DBA,
ru at sunbay.com		Sunbay Software Ltd,
ru at FreeBSD.org		FreeBSD committer
-------------- next part --------------
Index: Makefile
===================================================================
RCS file: /home/ncvs/src/release/Makefile,v
retrieving revision 1.782
diff -u -r1.782 Makefile
--- Makefile	4 Jun 2003 22:24:43 -0000	1.782
+++ Makefile	5 Jun 2003 21:36:49 -0000
@@ -384,6 +384,7 @@
 .if !defined(NOPORTS)
 	cd ${CHROOTDIR}/usr/ports && ${CVSPREFIX} cvs -R ${CVSARGS} -q update ${CVSCMDARGS} -P -d
 .endif
+	rm -f ${CHROOTDIR}/tmp/.readmes_done
 .if defined(DOMINIMALDOCPORTS) && ${DOMINIMALDOCPORTS} == "YES"
 	for i in ${MINIMALDOCPORTS}; do \
 		( cd ${CHROOTDIR}/usr/$$i && ${CVSPREFIX} cvs -R ${CVSARGS} -q update ${CVSCMDARGS} -P -d ) ; \
@@ -453,11 +454,15 @@
 	echo "	${CROSSMAKE} ${WORLD_FLAGS} -DNOCLEAN buildworld && \\" >> ${CHROOTDIR}/mk
 	echo "	touch /tmp/.world_done"		>> ${CHROOTDIR}/mk
 	echo "fi"				>> ${CHROOTDIR}/mk
-	echo "if [ ! -f /tmp/.skip_ports ]; then" >> ${CHROOTDIR}/mk
+.if defined(NOPORTS) || defined(NOPORTREADMES)
+	echo "if false; then"			>> ${CHROOTDIR}/mk
+.else
+	echo "if [ ! -f /tmp/.readmes_done ]; then" >> ${CHROOTDIR}/mk
+.endif
 	echo "	echo \">>> make readmes started on \`LC_ALL=C TZ=GMT date\`\"" >> ${CHROOTDIR}/mk
 	echo "	cd /usr/ports"			>> ${CHROOTDIR}/mk
 	echo "	make ${PORTREADMES_FLAGS} readmes" >> ${CHROOTDIR}/mk
-	echo "	touch /tmp/.skip_ports"		>> ${CHROOTDIR}/mk
+	echo "	touch /tmp/.readmes_done"	>> ${CHROOTDIR}/mk
 	echo "	echo \">>> make readmes finished on \`LC_ALL=C TZ=GMT date\`\"" >> ${CHROOTDIR}/mk
 	echo "fi"				>> ${CHROOTDIR}/mk
 	echo "cd /usr/src/release"		>> ${CHROOTDIR}/mk
@@ -465,9 +470,6 @@
 	echo "make \$${_RELTARGET}"		>> ${CHROOTDIR}/mk
 	echo "echo \">>> make ${.TARGET} for ${TARGET} finished on \`LC_ALL=C TZ=GMT date\`\"" >> ${CHROOTDIR}/mk
 	chmod 755 ${CHROOTDIR}/mk
-.if defined(NOPORTS) || defined(NOPORTREADMES)
-	touch ${CHROOTDIR}/tmp/.skip_ports
-.endif
 	# Ensure md.ko is loaded if md(4) is not statically compiled into the kernel
 	-mdconfig 2>/dev/null
 	env -i /usr/sbin/chroot ${CHROOTDIR} /mk
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20030606/4a0ab5f9/attachment.bin