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

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

> > > +.endif
> > > 
> > >  COPYDIRS=        \
> > >  
> > >   hotspot/src/os/linux/launcher \

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to