From owner-freebsd-ports@FreeBSD.ORG Wed May 16 01:35:27 2012 Return-Path: Delivered-To: freebsd-ports@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 8417D106564A; Wed, 16 May 2012 01:35:27 +0000 (UTC) (envelope-from gerald@pfeifer.com) Received: from ainaz.pair.com (ainaz.pair.com [209.68.2.66]) by mx1.freebsd.org (Postfix) with ESMTP id 5AF698FC0C; Wed, 16 May 2012 01:35:27 +0000 (UTC) Received: from K48.suse.de (nat.nue.novell.com [195.135.221.2]) by ainaz.pair.com (Postfix) with ESMTPSA id BE6773F446; Tue, 15 May 2012 21:35:20 -0400 (EDT) Date: Wed, 16 May 2012 03:35:18 +0200 (CEST) From: Gerald Pfeifer To: Andriy Gapon , Mark Linimon In-Reply-To: <4FAC3084.80101@FreeBSD.org> Message-ID: References: <4F578AA7.4060008@FreeBSD.org> <4F990D9A.3090100@FreeBSD.org> <4FA643FA.3050206@FreeBSD.org> <4FAB6E01.50108@FreeBSD.org> <4FAC3084.80101@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Cc: freebsd-ports@FreeBSD.org Subject: Re: WITH_GCC X-BeenThere: freebsd-ports@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting software to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 May 2012 01:35:27 -0000 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. :-)