Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 16 May 2012 08:56:18 +0300
From:      Andriy Gapon <avg@FreeBSD.org>
To:        Gerald Pfeifer <gerald@pfeifer.com>
Cc:        Mark Linimon <linimon@lonesome.com>, freebsd-ports@FreeBSD.org
Subject:   Re: WITH_GCC
Message-ID:  <4FB34182.20605@FreeBSD.org>
In-Reply-To: <alpine.LNX.2.00.1205160324510.2599@zbenl.fvgr>
References:  <CAGFTUwPUFdP=Z20%2BbL59qFuh_V6R1R-GcyrK03dxESL6ZyGz7A@mail.gmail.com> <4F578AA7.4060008@FreeBSD.org> <4F990D9A.3090100@FreeBSD.org> <4FA643FA.3050206@FreeBSD.org> <4FAB6E01.50108@FreeBSD.org> <4FAC3084.80101@FreeBSD.org> <alpine.LNX.2.00.1205160324510.2599@zbenl.fvgr>

next in thread | previous in thread | raw e-mail | index | archive | help
on 16/05/2012 04:35 Gerald Pfeifer said the following:
> Hi Andriy,
> 
> Mark Linimon worked on a similar patchset in the past.  Have the
> two of you synced and shared patches?  I did review some of his
> a bit ago, and while I do not have that any more, I believe it
> was somewhat different than your approach.

Yes.   While I tried only to add support for requesting a different version of
GCC, Mark is also adding support for a different compiler altogether (clang).
Mark's patch is also more comprehensive in other aspects (e.g. WANT_GCC).
We are looking at each other's changes :-)

> I'll provide some comments below.  Note, I am not opposed to the
> patch, but feel it may be better broken up in distinct and independent
> changes.  Some should go through a full cluster build.  And some
> give me serious headache.
> 
> On Fri, 11 May 2012, Andriy Gapon wrote:
>> Hopefully it should handle the bootstrapping better by accounting for 
>> lang/gcc*
>> ports dependencies and avoiding creating any circular dependencies.
>> For simplicity the GCC ports and their dependencies are forced to be 
>> built with the base GCC, although this does not have to be required.
> 
> Looking at the patch, there is no documentation.  Can you please add
> some comments to document the new setting and how it interacts with
> what is in place as of today?

I think that I provided that in one of my earlier emails.  You are correct that
it should go in as a documentation comment.
But at the moment, here it is for the reference:

[quote]
The idea behind the patch:
- if WITH_GCC is not defined, then everything should be as before
- if WITH_GCC is defined, but USE_GCC is not defined, then USE_GCC gets set
  from WITH_GCC and then everything should be handled as before
- if both are defined
  o if USE_GCC is a concrete version, then it wins
  o else (if USE_GCC has the "X+") form, then the minimum requested version
becomes MAX(X, Y), where Y is from WITH_GCC [*]

[*] Note that whether WITH_GCC has "Y" or "Y+" form doesn't matter in this case.

In all cases WITH_GCC can be used only to increase minimum required GCC version,
unless a port wants a concrete fixed version.
[/quote]

>> +.if defined(WITH_GCC) && ${PORTNAME} != gcc
>> +
>> +# See if we can use a later version or exclusively the one specified.
>> +_WITH_GCC:=	${WITH_GCC:S/+//}
> 
> Shouldn't there first be some code that handles the case where USE_GCC
> and WITH_GCC are specified at the same time?

My approach is different:
- first validate that WITH_GCC has a sane value
- set USE_GCC if it is not set but WITH_GCC is set

Only after these steps I try to handle the case where both knobs are set
(according to the logic described at the beginning).

> Also, is this duplication of code really necessary between the two
> settings, or could that be avoided by setting USE_GCC appropriately
> under the right conditions ("if and only if...") or something like
> that?

The code duplication is not nice, agreed.  But I do not see what you suggest.
There are two different variables with a value in the same format, both could be
set at the same time, they could be set to different values, both need to be
validated.

>>  .if defined(_GCC_ORLATER)
>> +. if defined(_WITH_GCC)
>> +.  if ${_USE_GCC} < ${_WITH_GCC}
>> +_USE_GCC:= ${_WITH_GCC}
>> +.  endif
>> +. endif
> 
> When can this happen?  And can this be handled earlier, cf. above?

This is the case when e.g. a port has USE_GCC=4.2+ and a user specifies
WITH_GCC=4.6+.  I can't see how this case can be handled earlier, but I wouldn't
rule out that it could be possible.

>> -_GCC_BUILD_DEPENDS:=	gcc${V}
>>  _GCC_PORT_DEPENDS:=	gcc${V}
>> +.   if ${V} == ${GCC_DEFAULT_V}
>> +_GCC_BUILD_DEPENDS:=	gcc
>> +.   else
>> +_GCC_BUILD_DEPENDS:=	gcc${V}
>> +.   endif
> 
> Isn't this an unrelated change to what you are mainly working on?

Yes.  I locally preferred that gcc 4.6 dependency is handled by lang/gcc instead
of lang/gcc46 when neither is installed yet.

> I'd prefer to avoid that, unless there is a good reason.  And if
> there is, I'm open (and like) to see this go in separately.

I am not sure if it could be considered a good reason, but lang/gcc seems to be
more "stable" and thus user-friendlier.  But yes, the snippet should be excluded
from this patch.

>> +_GCC_OWN_DEPENDS!= (cd ${PORTSDIR}/lang/${_GCC_BUILD_DEPENDS} && ${MAKE} -V
>> _UNIFIED_DEPENDS)
>> +.   for _CURDIR in ${.CURDIR} # only loop variable are expanded in variable
>> modifiers
>> +.   if ${_GCC_OWN_DEPENDS:M*\:${_CURDIR}} != ""
>> +.undef _GCC_BUILD_DEPENDS
>> +.undef _GCC_PORT_DEPENDS
>> +.   else
> 
> Headache, major headache. :-)  What is this for, do we really need
> it, and why,

This is done to handle dependencies of a gcc port itself.

An example.  Current lang/gcc46 has the following:
LIB_DEPENDS=    gmp.10:${PORTSDIR}/math/gmp \
                mpfr.4:${PORTSDIR}/math/mpfr \
                mpc.2:${PORTSDIR}/math/mpc
RUN_DEPENDS+=   ${LOCALBASE}/bin/as:${PORTSDIR}/devel/binutils

Let's presume that a user defines WITH_GCC=4.6+ and then does make install in
lang/gcc46 on a pristine system (no ports installed yet).  To satisfy the
dependencies the port build system would try build and install math/gmp.
Because of the WITH_GCC it would try to build math/gmp with gcc 4.6 and thus it
would try to install lang/gcc46.  Because lang/gcc46 depends on math/gmp it
would then try to install that port, but because...
You see where I am going? :-)
The recursion could be started from any of the required ports as easily.

> and can this be done differently?

I presume that almost everything can be done differently :-)
The above is the best _I_ could do.

>> +CFLAGS+=		${CFLAGS.${CC}}
>> +CXXFLAGS+=		${CXXFLAGS.${CC}}
> 
> Similarly here.  Where does this come from, why is it related to
> the WITH_GCC versus USE_GCC patch?  Can and should this be split
> out?  How is it used and where?  Where is it defined?

This should be split out.  The use case is to make it possible to set something
like the following in make.conf:
CFLAGS.gcc46+= -fearlier-gcc-fails-with-this-flag-but-I-want-it-with-gcc46

> PS: I won't be able to do FreeBSD work the coming two weeks (at all, 
> probably) but am very open to working with you and this change or
> changes split out of that as well as other changes. Do not read
> anything into it if responses may take a bit at times. :-)

OK.
The latest iteration of the patch: http://people.freebsd.org/~avg/bsd.gcc.mk.6.patch

-- 
Andriy Gapon



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4FB34182.20605>