cvs commit: src/release Makefile

[Marcel Moolenaar]

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 :-)

> > > 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:

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).

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.

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...

> > > 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.

>  Or if you just want to rerelease the same
> release, you should pass it the same flags.

Yes. This is the basic use of rerelease AFAICT.

> There are other ways to shoot yourself in the foot.  You can
> interrupt "make release" while it's doing "cvs co" of ports,
> and run "make rerelease" with -DRELEASENOUPDATE.  You'll be
> shooted in the foot too.

Yes.

> 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...

> The
> checkout/update operation of CVS could be guessed by checking if
> the target directory exists.

Probably, yes.

> 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

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

So: please do not conditionally put the commands in ${CHROOTDIR}/mk.
Have them unconditionally there, but have them conditionally executed.

-- 
 Marcel Moolenaar	  USPA: A-39004		 marcel at xcllnt.net