Date: Sat, 4 Sep 2010 14:26:27 -0700 From: Greg Lewis <glewis@eyesbeyond.com> To: David Naylor <naylor.b.david@gmail.com> Cc: Anonymous <swell.k@gmail.com>, freebsd-ports@freebsd.org Subject: Re: MAKE_JOBS and openjdk6 Message-ID: <20100904212627.GA15260@misty.eyesbeyond.com> In-Reply-To: <201008291037.40908.naylor.b.david@gmail.com> References: <201006251808.29467.naylor.b.david@gmail.com> <86lj7q31s8.fsf@gmail.com> <20100828213022.GA78573@misty.eyesbeyond.com> <201008291037.40908.naylor.b.david@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Aug 29, 2010 at 10:37:37AM +0200, David Naylor wrote:
> On Saturday 28 August 2010 23:30:22 Greg Lewis wrote:
> > On Sun, Aug 29, 2010 at 12:44:39AM +0400, Anonymous wrote:
> > > Greg Lewis <glewis@eyesbeyond.com> writes:
> > > > I would argue that overriding a private variable is a hack (other ports
> > > > doing it doesn't make it not a hack).
> > >
> > > You could've spoke up in ports/148754 about your concern in order for
> > > portmgr@ to notice. The PR strived to be less intrusive than divorcing
> > > build jobs from make jobs. Besides, I think adding more clutter to
> > > Makefiles defeats purpose of having stuff in bsd.port.mk.
> >
> > In that case, whichever way you cut it, we're deliberately trying to
> > circumvent what is in bsd.port.mk.
>
> The circumvention is required because bsd.port.mk assumes a certain interface
> that may not be applicable for all ports.
>
> > > > Alternative patch attached which seems to achieve the same result from
> > > > my perspective without overriding _MAKE_JOBS.
> > >
> > > Hardcoding kern.smp.cpus and ignoring MAKE_JOBS_SAFE/UNSAFE doesn't seem
> > > like a less hacky solution. I'd argue that it's more confusing because
> > > MAKE_JOBS_UNSAFE is not equal to DISABLE_MAKE_JOBS.
> >
> > The patch I attached (a) does not ignore MAKE_JOBS_{SAFE,UNSAFE} and (b)
> > the first patch similarly uses DISABLE_MAKE_JOBS.
> >
> > The first patch does the following:
> >
> > 1. Sets MAKE_JOBS_SAFE _erroneously_ (the port is _not_ MAKE_JOBS_SAFE)
> > purely so it can force the setting of MAKE_JOBS_NUMBER.
>
> Yes and no. The port is partially MAKE_JOBS_SAFE and is able to build some of
> the code using make jobs. This is similar to python26: it is _SAFE but only a
> small portion of the build actually uses more than one job which in effect
> makes it the same as _UNSAFE (from a performance perspective).
>
> > 2. Overrides passing of -j to the make invocation by fiddling the private
> > variable _MAKE_JOBS, which it has to do because of (1).
> >
> > The one I just provided
> >
> > 1. Leaves the port correctly marked as MAKE_JOBS_UNSAFE and doesn't mess
> > with any private variables.
>
> Your attached patch does not explicitly define either MAKE_JOBS_(UN)SAFE. I
> would by happy with it being defined as _UNSAFE. If there are no other
> problems with your patch (see my comment at the bottom) then I'm happy for
> this patch to be committed.
I'd already committed a change that marked the port as MAKE_JOBS_UNSAFE, so
the patch didn't include that.
> There will still be issues with scripts that try some form of load balancing
> when building ports but either way it will not be doing what was advertised.
> Similar to python.
>
> > 2. Respects MAKE_JOB_NUMBER if it is set and otherwise uses the sysctl
> > kern.smp.cpus, the latter being what the port _already_ does.
> >
> > > > Index: Makefile
> > > > ===================================================================
> > > > RCS file: /var/fcvs/ports/java/openjdk6/Makefile,v
> > > > retrieving revision 1.28
> > > > diff -u -r1.28 Makefile
> > > > --- Makefile 15 Aug 2010 05:23:06 -0000 1.28
> > > > +++ Makefile 28 Aug 2010 18:27:44 -0000
> > > > @@ -147,8 +147,14 @@
> > > >
> > > > USE_DISPLAY= yes
> > > > .endif
> > > >
> > > > -BUILD_JOBS_NUMBER!= ${SYSCTL} -n kern.smp.cpus
> > > > +.if !defined(DISABLE_MAKE_JOBS)
> > > > +.if defined(MAKE_JOBS_NUMBER)
> > > > +BUILD_JOBS_NUMBER= ${MAKE_JOBS_NUMBER}
> > > > +.else
> > > > +BUILD_JOBS_NUMBER= `${SYSCTL} -n kern.smp.cpus`
> > > > +.endif
> > > >
> > > > MAKE_ENV+= HOTSPOT_BUILD_JOBS=${BUILD_JOBS_NUMBER}
>
> Is it safe to pass an empty HOTSPOT_BUILD_JOBS to MAKE_ENV? (i.e. when
> DISABLE_MAKE_JOBS is defined.)
Good thought. I tried the build with DISABLE_MAKE_JOBS set and experienced
no problems, so I think we're ok on that front.
> > > > +.endif
> > > >
> > > > COPYDIRS= \
> > > >
> > > > hotspot/src/os/linux/launcher \
I'm going to commit the change in a day or two if there are no further
objections. I'll then use it as a template for changes to the other JDK
ports.
--
Greg Lewis Email : glewis@eyesbeyond.com
Eyes Beyond Web : http://www.eyesbeyond.com
Information Technology FreeBSD : glewis@FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100904212627.GA15260>