From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Ehrhardt Subject: Re: [PATCH v3] mk: filter duplicate configuration entries Date: Wed, 6 Jul 2016 11:08:12 +0200 Message-ID: References: <1467783465-14533-1-git-send-email-christian.ehrhardt@canonical.com> <20160706081238.GA17404@bricha3-MOBL3> <577CC7DD.1000004@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: Bruce Richardson , Thomas Monjalon , dev To: Ferruh Yigit Return-path: Received: from mail-qk0-f182.google.com (mail-qk0-f182.google.com [209.85.220.182]) by dpdk.org (Postfix) with ESMTP id 51E1969FE for ; Wed, 6 Jul 2016 11:08:32 +0200 (CEST) Received: by mail-qk0-f182.google.com with SMTP id s126so25745648qkh.2 for ; Wed, 06 Jul 2016 02:08:32 -0700 (PDT) In-Reply-To: <577CC7DD.1000004@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Thanks Ferruh, I'm personally more an awk guy so that was my natural choice. But I in general agree, the less tools used the better for dependencies and stability. I checked your suggestion and works like a charm. I'll still follow Bruce guidance to add more explanation. v4 should show up any minute ... Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd On Wed, Jul 6, 2016 at 10:57 AM, Ferruh Yigit wrote: > On 7/6/2016 9:12 AM, Bruce Richardson wrote: > > On Wed, Jul 06, 2016 at 07:37:45AM +0200, Christian Ehrhardt wrote: > >> *updates in v3* > >> - replace tac with sed '1!G;h;$!d' to avoid build time dependency > >> > >> *updates in v2* > >> - move to .config target > >> - fix usage order of tac > >> - simplify inner section by only using awk (instead of > awk+loop+bash+sed) > >> > >> Due to the hierarchy and the demand to keep the base config showing all > >> options, some config keys end up multiple times in the .config file. > >> > >> Due to the way the actual config is sourced only the last entry is > >> important. That can confuse people changing values in .config which > >> are then ignored. > >> > >> A suggested solution was to filter for duplicates at the end of the > >> actual config step which is implemented here. > >> > >> Signed-off-by: Christian Ehrhardt > >> --- > >> mk/rte.sdkconfig.mk | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk > >> index a3acfe6..d031bf4 100644 > >> --- a/mk/rte.sdkconfig.mk > >> +++ b/mk/rte.sdkconfig.mk > >> @@ -79,11 +79,17 @@ $(RTE_OUTPUT): > >> ifdef NODOTCONF > >> $(RTE_OUTPUT)/.config: ; > >> else > >> +# Generate config from template, if there are duplicates keep only the > last > >> $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | $(RTE_OUTPUT) > >> $(Q)if [ "$(RTE_CONFIG_TEMPLATE)" != "" -a -f > "$(RTE_CONFIG_TEMPLATE)" ]; then \ > >> $(CPP) -undef -P -x assembler-with-cpp \ > >> -ffreestanding \ > >> -o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \ > >> + sed '1!G;h;$$!d' $(RTE_OUTPUT)/.config_tmp > > $(RTE_OUTPUT)/.config_tmp_reverse ; \ > >> + awk --field-separator '=' '!/^#/ {if (!seen[$$1]) {print > ($$0)}; seen[$$1]=1;} \ > >> + /^#/ {print($$0)}' > $(RTE_OUTPUT)/.config_tmp_reverse \ > >> + | sed '1!G;h;$$!d' > $(RTE_OUTPUT)/.config_tmp ; \ > >> + rm $(RTE_OUTPUT)/.config_tmp_reverse ; \ > >> if ! cmp -s $(RTE_OUTPUT)/.config_tmp > $(RTE_OUTPUT)/.config; then \ > >> cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config > ; \ > >> cp $(RTE_OUTPUT)/.config_tmp > $(RTE_OUTPUT)/.config.orig ; \ > >> -- > > > > Given the length and complexity of the work being done here, using some > pretty > > fancy sed and awk features, I feel that the comment at the top should be > > expanded to actually explain what is being done and how. I would also > include > > in that explanation how sed is being used to reverse a file. Personally, > I > > would have preferred to keep the dependency on tac for a readability > perspective. > > > > By using sed, I didn't really mean using sed instead of tac, but > something close to first version of this patch [1], these are just > different ways of doing same thing. > > I don't know how common "tac" is, but even a box breaks build because > off missing tac, that is problem, specially this change is not a must > but a good to have. > > [1] > for config in $$(grep -v "^#" $(RTE_OUTPUT)/.config_tmp | cut -d"=" -f1 > | sort | uniq -d); do \ > while [ $$(grep "^$${config}=" $(RTE_OUTPUT)/.config_tmp -c ) -gt 1 > ]; do \ > sed -i "0,/^$${config}=/{//d}" $(RTE_OUTPUT)/.config_tmp; \ > done; \ > done; \ > > > >