From owner-cvs-src@FreeBSD.ORG Thu Jun 5 14:01:11 2003 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2037C37B401; Thu, 5 Jun 2003 14:01:11 -0700 (PDT) Received: from ns1.xcllnt.net (209-128-86-226.bayarea.net [209.128.86.226]) by mx1.FreeBSD.org (Postfix) with ESMTP id A3CF843F75; Thu, 5 Jun 2003 14:01:09 -0700 (PDT) (envelope-from marcel@xcllnt.net) Received: from athlon.pn.xcllnt.net (athlon.pn.xcllnt.net [192.168.4.3]) by ns1.xcllnt.net (8.12.9/8.12.9) with ESMTP id h55L19wk026315; Thu, 5 Jun 2003 14:01:09 -0700 (PDT) (envelope-from marcel@piii.pn.xcllnt.net) Received: from athlon.pn.xcllnt.net (localhost [127.0.0.1]) by athlon.pn.xcllnt.net (8.12.9/8.12.9) with ESMTP id h55L19QA000816; Thu, 5 Jun 2003 14:01:09 -0700 (PDT) (envelope-from marcel@athlon.pn.xcllnt.net) Received: (from marcel@localhost) by athlon.pn.xcllnt.net (8.12.9/8.12.9/Submit) id h55L19Yh000815; Thu, 5 Jun 2003 14:01:09 -0700 (PDT) Date: Thu, 5 Jun 2003 14:01:08 -0700 From: Marcel Moolenaar To: Ruslan Ermilov Message-ID: <20030605210108.GA689@athlon.pn.xcllnt.net> References: <200306040517.h545HIY1051372@repoman.freebsd.org> <20030605100007.GA42986@sunbay.com> <20030605170728.GA572@dhcp01.pn.xcllnt.net> <20030605195611.GA2243@sunbay.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20030605195611.GA2243@sunbay.com> User-Agent: Mutt/1.5.4i cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/release Makefile X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Jun 2003 21:01:11 -0000 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@xcllnt.net