From mboxrd@z Thu Jan  1 00:00:00 1970
From: Olivier MATZ <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH v2] mk: allow updates to build config on make
 install
Date: Tue, 20 May 2014 13:37:53 +0200
Message-ID: <537B3E91.4030400@6wind.com>
References: <1400062955-27338-1-git-send-email-bruce.richardson@intel.com>
 <1400082910-16804-1-git-send-email-bruce.richardson@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
To: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, dev-VfR2kkLFssw@public.gmane.org
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <1400082910-16804-1-git-send-email-bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>

Hi Bruce,

On 05/14/2014 05:55 PM, Bruce Richardson wrote:
> When running "make config" and additional config.orig file is also
> generated, which is intended to hold the original, clean configuration
> from the template.
> When running make install, we first check if there is no existing
> .config file, and run make config if not. If there is a file, we then
> check if it's unmodified, in which case we regenerate a new .config to
> take account of any possible updates to the template. Finally, in the
> case where there is an existing .config file, and it HAS been modified,
> we then do a check to see if the template has had further updates, and
> throw an error if so. If no updates, we continue with the build using
> the existing, user-modified config.
>
> Signed-off-by: Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

It looks good to me, except the small typos below.

> --- a/mk/rte.sdkinstall.mk
> +++ b/mk/rte.sdkinstall.mk
> @@ -54,10 +54,20 @@ INSTALL_TARGETS := $(addsuffix _install,\
>   .PHONY: install
>   install: $(INSTALL_TARGETS)
>
> +
>   %_install:
>   	@echo ================== Installing $*
>   	$(Q)if [ ! -f $(BUILD_DIR)/$*/.config ]; then \
>   		$(MAKE) config T=$* O=$(BUILD_DIR)/$*; \

No need to add an extra line here.

> +	elif cmp -s $(BUILD_DIR)/$*/.config.orig $(BUILD_DIR)/$*/.config; then \
> +		$(MAKE) config T=$* O=$(BUILD_DIR)/$*; \
> +	else \
> +		$(MAKE) config T=$* O=/tmp/$*; \
> +		if  ! cmp -s /tmp/$*/.config.orig $(BUILD_DIR)/$*/.config.orig ; then \
> +			echo "Config Conflict: Local config and standard config have both changed" ; \
> +			exit 1; \

I would remove extra upper-case chars and say "template" instead
of "standard". Like that:

   echo "Config conflict: local config and template config have both
changed"

Apart from these minor changes,
Acked-by: Olivier Matz <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>


Regards,
Olivier