Date: Wed, 16 May 2012 03:35:18 +0200 (CEST) From: Gerald Pfeifer <gerald@pfeifer.com> To: Andriy Gapon <avg@FreeBSD.org>, Mark Linimon <linimon@lonesome.com> Cc: freebsd-ports@FreeBSD.org Subject: Re: WITH_GCC Message-ID: <alpine.LNX.2.00.1205160324510.2599@zbenl.fvgr> In-Reply-To: <4FAC3084.80101@FreeBSD.org> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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. 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? > +.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? 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? > .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? > -_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? 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. > +_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, and can this be done differently? > +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? Gerald 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. :-)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.LNX.2.00.1205160324510.2599>