From mboxrd@z Thu Jan 1 00:00:00 1970 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) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=9YCoDFOCRWmqh9yKiaaf3tl31Mk/o+9PfXgRUiW609k=; b=g6chd6an+NNEIzfnM0TYDPC1B+u73AYVrCfJb2s0vSZHZn2h2Gjue5Vc/AePRZFMGd OoDPbNpzXIR5MuhrGfig8CriMPh69idxxJUtzL4UfIcq/nqndX8hQDo4QoajrL4ittNE 7NE0l9NbcsAoDE7RvveGawniT3m6kJo2lXrTJ8XtV0pkQ4z4YW5rP9q7JAd4b4RtXfVa TlV7kT453g1BGKel8wJ/vSc/jIod+D8omoWC6ddpkr1YyI/9NF5LB2znj3Z3lbhITXor UphZKuNptF7oeGtz9P+09mC0Lnj0PJN22x7hvdyaPk6yJKABqhkmvNxoNDOqs5S0y6kb 6+XQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=9YCoDFOCRWmqh9yKiaaf3tl31Mk/o+9PfXgRUiW609k=; b=UAm6LLCCRxKnUcGzqlgT4MJO3OF548346CWH4c7jpBJlxQ88Omz3NW9gy598mEij8l jEE1Z+maRgTyWF69KdPeUmfrIg1J9RoaQ2JIU5oKelVhYdpTOZ/DEThYq7+hGKoKdELN /bcAFzCL+yElpQcPWhD2DPpBcJoq/NFD+KRGSamU81fibHfqcg3q6M+PtQMYURNrp3HK 9bkc3WHTdAT8KfqBldG/rL8LgGPBHrM4Py5SFhsEG/4qK1NdcYRm0ir0BVtmN+TFszkt 2z4ew5x65Qgk5GgHyN3dllNqE3WUXPWHt1lg9+O+lAKfq/IQD4ZjvqLgGaycjeEGeL/9 31Tw== X-Gm-Message-State: ALyK8tI9WsnFpl1bJNWRUqxGMCUwRzd3VD8Ch6kaTsXf+MyxY6Q5EDFisvncH/TBmcXUtXmvR4cHawzCh+fS9FFC X-Received: by 10.55.22.154 with SMTP id 26mr29199707qkw.193.1467796111694; Wed, 06 Jul 2016 02:08:31 -0700 (PDT) MIME-Version: 1.0 Received: by 10.55.23.77 with HTTP; Wed, 6 Jul 2016 02:08:12 -0700 (PDT) In-Reply-To: <577CC7DD.1000004@intel.com> References: <1467783465-14533-1-git-send-email-christian.ehrhardt@canonical.com> <20160706081238.GA17404@bricha3-MOBL3> <577CC7DD.1000004@intel.com> From: Christian Ehrhardt Date: Wed, 6 Jul 2016 11:08:12 +0200 Message-ID: To: Ferruh Yigit Cc: Bruce Richardson , Thomas Monjalon , dev Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v3] mk: filter duplicate configuration entries X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jul 2016 09:08:32 -0000 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; \ > > > >