All of lore.kernel.org
 help / color / mirror / Atom feed
* Duplicate config symbols
@ 2016-06-13 11:50 Christian Ehrhardt
  2016-06-13 13:07 ` Ferruh Yigit
  2016-06-13 13:47 ` Thomas Monjalon
  0 siblings, 2 replies; 22+ messages in thread
From: Christian Ehrhardt @ 2016-06-13 11:50 UTC (permalink / raw)
  To: dev

Hi,
I wondered multiple times now when changing a config symbol that some of
them are in the .config file multiple times.
I totally feel like I'm overlooking something, but still it might be worth
to ask.

In particular:
awk -F "=" '/=/ {print $1}' debian/build/static-root/.config | sort | uniq
-c | sort -n | grep -v 1
     2 CONFIG_RTE_ARCH
     2 CONFIG_RTE_EAL_IGB_UIO
     2 CONFIG_RTE_EAL_VFIO
     2 CONFIG_RTE_EXEC_ENV
     2 CONFIG_RTE_KNI_KMOD
     2 CONFIG_RTE_LIBRTE_KNI
     2 CONFIG_RTE_LIBRTE_PMD_AF_PACKET
     2 CONFIG_RTE_LIBRTE_PMD_VHOST
     2 CONFIG_RTE_LIBRTE_POWER
     2 CONFIG_RTE_LIBRTE_VHOST
     2 CONFIG_RTE_MACHINE
     2 CONFIG_RTE_TOOLCHAIN

Is there any reason to do so or is this an issue in make config?

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Duplicate config symbols
  2016-06-13 11:50 Duplicate config symbols Christian Ehrhardt
@ 2016-06-13 13:07 ` Ferruh Yigit
  2016-06-13 13:47 ` Thomas Monjalon
  1 sibling, 0 replies; 22+ messages in thread
From: Ferruh Yigit @ 2016-06-13 13:07 UTC (permalink / raw)
  To: Christian Ehrhardt, dev

On 6/13/2016 12:50 PM, Christian Ehrhardt wrote:
> Hi,
> I wondered multiple times now when changing a config symbol that some of
> them are in the .config file multiple times.
> I totally feel like I'm overlooking something, but still it might be worth
> to ask.
> 
> In particular:
> awk -F "=" '/=/ {print $1}' debian/build/static-root/.config | sort | uniq
> -c | sort -n | grep -v 1
>      2 CONFIG_RTE_ARCH
>      2 CONFIG_RTE_EAL_IGB_UIO
>      2 CONFIG_RTE_EAL_VFIO
>      2 CONFIG_RTE_EXEC_ENV
>      2 CONFIG_RTE_KNI_KMOD
>      2 CONFIG_RTE_LIBRTE_KNI
>      2 CONFIG_RTE_LIBRTE_PMD_AF_PACKET
>      2 CONFIG_RTE_LIBRTE_PMD_VHOST
>      2 CONFIG_RTE_LIBRTE_POWER
>      2 CONFIG_RTE_LIBRTE_VHOST
>      2 CONFIG_RTE_MACHINE
>      2 CONFIG_RTE_TOOLCHAIN
> 
> Is there any reason to do so or is this an issue in make config?
> 
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
> 

This happened with commit:
43f4364 config: remove duplicate information

I commented something similar, and send a patch to fix this:
http://dpdk.org/ml/archives/dev/2016-March/034718.html

With current implementation, the last occurrence of config item overwrites.

Regards,
ferruh

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Duplicate config symbols
  2016-06-13 11:50 Duplicate config symbols Christian Ehrhardt
  2016-06-13 13:07 ` Ferruh Yigit
@ 2016-06-13 13:47 ` Thomas Monjalon
  2016-06-13 15:09   ` Christian Ehrhardt
  1 sibling, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2016-06-13 13:47 UTC (permalink / raw)
  To: Christian Ehrhardt; +Cc: dev

2016-06-13 13:50, Christian Ehrhardt:
> I wondered multiple times now when changing a config symbol that some of
> them are in the .config file multiple times.
> I totally feel like I'm overlooking something, but still it might be worth
> to ask.
[...]
> Is there any reason to do so or is this an issue in make config?

It is an issue in "make config" which has never been considered important.
We could remove the first - overridden - occurences.
I think it can be fixed in mk/rte.sdkconfig.mk.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Duplicate config symbols
  2016-06-13 13:47 ` Thomas Monjalon
@ 2016-06-13 15:09   ` Christian Ehrhardt
  2016-06-13 15:10     ` [RFC] mk: filter duplicate configuration entries Christian Ehrhardt
  2016-06-13 16:55     ` Duplicate config symbols Thomas Monjalon
  0 siblings, 2 replies; 22+ messages in thread
From: Christian Ehrhardt @ 2016-06-13 15:09 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Mon, Jun 13, 2016 at 3:47 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:

> 2016-06-13 13:50, Christian Ehrhardt:
> > I wondered multiple times now when changing a config symbol that some of
> > them are in the .config file multiple times.
> > I totally feel like I'm overlooking something, but still it might be
> worth
> > to ask.
> [...]
> > Is there any reason to do so or is this an issue in make config?
>
> It is an issue in "make config" which has never been considered important.
>

I didn't want to make it more important :-)
I'm fine with the second occurrence overwriting as it did a while now and
knowing it is not a totally unknown issue.

I had seen the old argument for not moving them out completely in the old
thread - thanks for the link - that was important to understand why they
are still there.
Also found related patches from Keith about that now.


> We could remove the first - overridden - occurences.
> I think it can be fixed in mk/rte.sdkconfig.mk.
>

You mean just filtering them eventually while keeping them where they are
so that the old request to have "the base config shows all config options"
still fulfilled - yeah that could be an approach to make everybody happy.
But then it would make for some evil unreadable sed or such, I could live
with it as is knowing it is accepted as-is.
I'll submit an RFC, but hope for someone with more dark magic to make it
nicer.

Kind Regards,
Christian

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFC] mk: filter duplicate configuration entries
  2016-06-13 15:09   ` Christian Ehrhardt
@ 2016-06-13 15:10     ` Christian Ehrhardt
  2016-06-28 16:11       ` Ferruh Yigit
  2016-06-13 16:55     ` Duplicate config symbols Thomas Monjalon
  1 sibling, 1 reply; 22+ messages in thread
From: Christian Ehrhardt @ 2016-06-13 15:10 UTC (permalink / raw)
  To: christian.ehrhardt, ferruh.yigit, thomas.monjalon, dev

Due to the hierarchy and the demand to keep the base config shoing all
options some options end up multiple times in the .config file.

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 <christian.ehrhardt@canonical.com>
---
 mk/rte.sdkconfig.mk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index a3acfe6..734aa06 100644
--- a/mk/rte.sdkconfig.mk
+++ b/mk/rte.sdkconfig.mk
@@ -70,6 +70,11 @@ config: notemplate
 else
 config: $(RTE_OUTPUT)/include/rte_config.h $(RTE_OUTPUT)/Makefile
 	$(Q)$(MAKE) depdirs
+	tac $(RTE_OUTPUT)/.config | awk --field-separator '=' '!/^#/ {print $$1}' | while read config; do \
+		if [ $$(grep "^$${config}=" $(RTE_OUTPUT)/.config | wc -l) -gt 1 ]; then \
+			sed -i "0,/$${config}/{//d}" $(RTE_OUTPUT)/.config; \
+		fi; \
+	done
 	@echo "Configuration done"
 endif
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: Duplicate config symbols
  2016-06-13 15:09   ` Christian Ehrhardt
  2016-06-13 15:10     ` [RFC] mk: filter duplicate configuration entries Christian Ehrhardt
@ 2016-06-13 16:55     ` Thomas Monjalon
  1 sibling, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2016-06-13 16:55 UTC (permalink / raw)
  To: Christian Ehrhardt; +Cc: dev

2016-06-13 17:09, Christian Ehrhardt:
> On Mon, Jun 13, 2016 at 3:47 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
> wrote:
> 
> > 2016-06-13 13:50, Christian Ehrhardt:
> > > I wondered multiple times now when changing a config symbol that some of
> > > them are in the .config file multiple times.
> > > I totally feel like I'm overlooking something, but still it might be
> > worth
> > > to ask.
> > [...]
> > > Is there any reason to do so or is this an issue in make config?
> >
> > It is an issue in "make config" which has never been considered important.
> 
> I didn't want to make it more important :-)
> I'm fine with the second occurrence overwriting as it did a while now and
> knowing it is not a totally unknown issue.

Being a known issue doesn't mean we should not fix it ;)

> I had seen the old argument for not moving them out completely in the old
> thread - thanks for the link - that was important to understand why they
> are still there.
> Also found related patches from Keith about that now.

There were not enough strong opinions or consensus to move them.

> > We could remove the first - overridden - occurences.
> > I think it can be fixed in mk/rte.sdkconfig.mk.
> 
> You mean just filtering them eventually while keeping them where they are
> so that the old request to have "the base config shows all config options"
> still fulfilled - yeah that could be an approach to make everybody happy.

I would say it is not really related. We may decide any defconfig hierarchy,
and there still will be some cases where values are overriden.

> But then it would make for some evil unreadable sed or such, I could live
> with it as is knowing it is accepted as-is.
> I'll submit an RFC, but hope for someone with more dark magic to make it
> nicer.

There is an awk command in mk/rte.app.mk which could give you some ideas.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] mk: filter duplicate configuration entries
  2016-06-13 15:10     ` [RFC] mk: filter duplicate configuration entries Christian Ehrhardt
@ 2016-06-28 16:11       ` Ferruh Yigit
  2016-06-28 16:38         ` Christian Ehrhardt
  0 siblings, 1 reply; 22+ messages in thread
From: Ferruh Yigit @ 2016-06-28 16:11 UTC (permalink / raw)
  To: Christian Ehrhardt, thomas.monjalon, dev

On 6/13/2016 4:10 PM, Christian Ehrhardt wrote:
> Due to the hierarchy and the demand to keep the base config shoing all
> options some options end up multiple times in the .config file.
> 
> 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 <christian.ehrhardt@canonical.com>
> ---
>  mk/rte.sdkconfig.mk | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
> index a3acfe6..734aa06 100644
> --- a/mk/rte.sdkconfig.mk
> +++ b/mk/rte.sdkconfig.mk
> @@ -70,6 +70,11 @@ config: notemplate
>  else
>  config: $(RTE_OUTPUT)/include/rte_config.h $(RTE_OUTPUT)/Makefile
Not sure if this should go under this rule, or "$(RTE_OUTPUT)/.config:"
and should work with ".config_tmp".

>  	$(Q)$(MAKE) depdirs
> +	tac $(RTE_OUTPUT)/.config | awk --field-separator '=' '!/^#/ {print $$1}' | while read config; do \
Why reversing file since already checking all lines one by one in
original file?

And instead of checking each line, it is possible to get list of
duplicates via "sort | uniq -d".

Although less important, file comments also tripled in final .config.

> +		if [ $$(grep "^$${config}=" $(RTE_OUTPUT)/.config | wc -l) -gt 1 ]; then \
"grep -c" can be used instead of "grep | wc -l"

> +			sed -i "0,/$${config}/{//d}" $(RTE_OUTPUT)/.config; \
> +		fi; \
> +	done
>  	@echo "Configuration done"
>  endif
>  
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] mk: filter duplicate configuration entries
  2016-06-28 16:11       ` Ferruh Yigit
@ 2016-06-28 16:38         ` Christian Ehrhardt
  2016-06-28 16:48           ` Ferruh Yigit
  0 siblings, 1 reply; 22+ messages in thread
From: Christian Ehrhardt @ 2016-06-28 16:38 UTC (permalink / raw)
  To: Ferruh Yigit, Christian Ehrhardt; +Cc: Thomas Monjalon, dev

On Tue, Jun 28, 2016 at 6:11 PM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 6/13/2016 4:10 PM, Christian Ehrhardt wrote:
> > Due to the hierarchy and the demand to keep the base config shoing all
> > options some options end up multiple times in the .config file.
> >
> > 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 <christian.ehrhardt@canonical.com>
> > ---
> >  mk/rte.sdkconfig.mk | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
> > index a3acfe6..734aa06 100644
> > --- a/mk/rte.sdkconfig.mk
> > +++ b/mk/rte.sdkconfig.mk
> > @@ -70,6 +70,11 @@ config: notemplate
> >  else
> >  config: $(RTE_OUTPUT)/include/rte_config.h $(RTE_OUTPUT)/Makefile
> Not sure if this should go under this rule, or "$(RTE_OUTPUT)/.config:"
> and should work with ".config_tmp".
>
> >       $(Q)$(MAKE) depdirs
> > +     tac $(RTE_OUTPUT)/.config | awk --field-separator '=' '!/^#/
> {print $$1}' | while read config; do \
> Why reversing file since already checking all lines one by one in
> original file?
>

Hi,
every other comment is ok I'll rebase and resubmit once I find some time
again.
But for this (tac) the reason is simple - to keep behaviour.
Currently the last one wins.
So if you have
CONFIG_A=n
CONFIG_A=y

Essentially you have
CONFIG_A=y

By the tac and keeping the first occurrence we maintain behavior.
It is interestingly hard to "keep the last occurrence" without such tricks,
but I'm open to suggestions.


> And instead of checking each line, it is possible to get list of
> duplicates via "sort | uniq -d".
>

That would fail for the reasons outlined above.

Although less important, file comments also tripled in final .config.
>
> > +             if [ $$(grep "^$${config}=" $(RTE_OUTPUT)/.config | wc -l)
> -gt 1 ]; then \
> "grep -c" can be used instead of "grep | wc -l"
>
> > +                     sed -i "0,/$${config}/{//d}"
> $(RTE_OUTPUT)/.config; \
> > +             fi; \
> > +     done
> >       @echo "Configuration done"
> >  endif
> >
> >
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] mk: filter duplicate configuration entries
  2016-06-28 16:38         ` Christian Ehrhardt
@ 2016-06-28 16:48           ` Ferruh Yigit
  2016-06-30 11:57             ` Christian Ehrhardt
  0 siblings, 1 reply; 22+ messages in thread
From: Ferruh Yigit @ 2016-06-28 16:48 UTC (permalink / raw)
  To: Christian Ehrhardt; +Cc: Thomas Monjalon, dev

On 6/28/2016 5:38 PM, Christian Ehrhardt wrote:
> On Tue, Jun 28, 2016 at 6:11 PM, Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     On 6/13/2016 4:10 PM, Christian Ehrhardt wrote:
>     > Due to the hierarchy and the demand to keep the base config shoing all
>     > options some options end up multiple times in the .config file.
>     >
>     > 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 <christian.ehrhardt@canonical.com
>     <mailto:christian.ehrhardt@canonical.com>>
>     > ---
>     >  mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk> | 5 +++++
>     >  1 file changed, 5 insertions(+)
>     >
>     > diff --git a/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk> b/mk/rte.sdkconfig.mk
>     <http://rte.sdkconfig.mk>
>     > index a3acfe6..734aa06 100644
>     > --- a/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk>
>     > +++ b/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk>
>     > @@ -70,6 +70,11 @@ config: notemplate
>     >  else
>     >  config: $(RTE_OUTPUT)/include/rte_config.h $(RTE_OUTPUT)/Makefile
>     Not sure if this should go under this rule, or "$(RTE_OUTPUT)/.config:"
>     and should work with ".config_tmp".
> 
>     >       $(Q)$(MAKE) depdirs
>     > +     tac $(RTE_OUTPUT)/.config | awk --field-separator '=' '!/^#/ {print $$1}' | while read config; do \
>     Why reversing file since already checking all lines one by one in
>     original file?
> 
> 
> Hi,
> every other comment is ok I'll rebase and resubmit once I find some time
> again.
> But for this (tac) the reason is simple - to keep behaviour.
> Currently the last one wins.

Correct, but if I am not missing something, reversing doesn't help to this,
how lines deleted taking care of this:
sed -i "0,/$${config}/{//d}" $(RTE_OUTPUT)/.config;

sed works on original file, and deletes first occurrence, independent
from lines from bottom to up, or up to bottom fed into it.

> So if you have
> CONFIG_A=n
> CONFIG_A=y
> 
> Essentially you have 
> CONFIG_A=y
> 
> By the tac and keeping the first occurrence we maintain behavior.
> It is interestingly hard to "keep the last occurrence" without such
> tricks, but I'm open to suggestions.
>  
> 
>     And instead of checking each line, it is possible to get list of
>     duplicates via "sort | uniq -d".
> 
>  
> That would fail for the reasons outlined above.
> 
>     Although less important, file comments also tripled in final .config.
> 
>     > +             if [ $$(grep "^$${config}=" $(RTE_OUTPUT)/.config | wc -l) -gt 1 ]; then \
>     "grep -c" can be used instead of "grep | wc -l"
> 
>     > +                     sed -i "0,/$${config}/{//d}"
>     $(RTE_OUTPUT)/.config; \
>     > +             fi; \
>     > +     done
>     >       @echo "Configuration done"
>     >  endif
>     >
>     >
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] mk: filter duplicate configuration entries
  2016-06-28 16:48           ` Ferruh Yigit
@ 2016-06-30 11:57             ` Christian Ehrhardt
  2016-06-30 12:00               ` [PATCH v2] " Christian Ehrhardt
  0 siblings, 1 reply; 22+ messages in thread
From: Christian Ehrhardt @ 2016-06-30 11:57 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Thomas Monjalon, dev

Hi,
thanks a lot for your feedback.
I looked at it once more and found some issues - the tac was correct, but
at the wrong place.
Also I could simplify the inner section at least a bit.
I agree to the move to the .config target.

I'll submit a v2 now to this thread.
Here is the diff it generates for a:  "make V=1
T=x86_64-native-linuxapp-gcc config"
http://paste.ubuntu.com/18163566/



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Jun 28, 2016 at 6:48 PM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 6/28/2016 5:38 PM, Christian Ehrhardt wrote:
> > On Tue, Jun 28, 2016 at 6:11 PM, Ferruh Yigit <ferruh.yigit@intel.com
> > <mailto:ferruh.yigit@intel.com>> wrote:
> >
> >     On 6/13/2016 4:10 PM, Christian Ehrhardt wrote:
> >     > Due to the hierarchy and the demand to keep the base config shoing
> all
> >     > options some options end up multiple times in the .config file.
> >     >
> >     > 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 <
> christian.ehrhardt@canonical.com
> >     <mailto:christian.ehrhardt@canonical.com>>
> >     > ---
> >     >  mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk> | 5 +++++
> >     >  1 file changed, 5 insertions(+)
> >     >
> >     > diff --git a/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk> b/mk/
> rte.sdkconfig.mk
> >     <http://rte.sdkconfig.mk>
> >     > index a3acfe6..734aa06 100644
> >     > --- a/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk>
> >     > +++ b/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk>
> >     > @@ -70,6 +70,11 @@ config: notemplate
> >     >  else
> >     >  config: $(RTE_OUTPUT)/include/rte_config.h $(RTE_OUTPUT)/Makefile
> >     Not sure if this should go under this rule, or
> "$(RTE_OUTPUT)/.config:"
> >     and should work with ".config_tmp".
> >
> >     >       $(Q)$(MAKE) depdirs
> >     > +     tac $(RTE_OUTPUT)/.config | awk --field-separator '=' '!/^#/
> {print $$1}' | while read config; do \
> >     Why reversing file since already checking all lines one by one in
> >     original file?
> >
> >
> > Hi,
> > every other comment is ok I'll rebase and resubmit once I find some time
> > again.
> > But for this (tac) the reason is simple - to keep behaviour.
> > Currently the last one wins.
>
> Correct, but if I am not missing something, reversing doesn't help to this,
> how lines deleted taking care of this:
> sed -i "0,/$${config}/{//d}" $(RTE_OUTPUT)/.config;
>
> sed works on original file, and deletes first occurrence, independent
> from lines from bottom to up, or up to bottom fed into it.
>
> > So if you have
> > CONFIG_A=n
> > CONFIG_A=y
> >
> > Essentially you have
> > CONFIG_A=y
> >
> > By the tac and keeping the first occurrence we maintain behavior.
> > It is interestingly hard to "keep the last occurrence" without such
> > tricks, but I'm open to suggestions.
> >
> >
> >     And instead of checking each line, it is possible to get list of
> >     duplicates via "sort | uniq -d".
> >
> >
> > That would fail for the reasons outlined above.
> >
> >     Although less important, file comments also tripled in final .config.
> >
> >     > +             if [ $$(grep "^$${config}=" $(RTE_OUTPUT)/.config |
> wc -l) -gt 1 ]; then \
> >     "grep -c" can be used instead of "grep | wc -l"
> >
> >     > +                     sed -i "0,/$${config}/{//d}"
> >     $(RTE_OUTPUT)/.config; \
> >     > +             fi; \
> >     > +     done
> >     >       @echo "Configuration done"
> >     >  endif
> >     >
> >     >
> >
> >
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2] mk: filter duplicate configuration entries
  2016-06-30 11:57             ` Christian Ehrhardt
@ 2016-06-30 12:00               ` Christian Ehrhardt
  2016-07-05 16:47                 ` Ferruh Yigit
  0 siblings, 1 reply; 22+ messages in thread
From: Christian Ehrhardt @ 2016-06-30 12:00 UTC (permalink / raw)
  To: christian.ehrhardt, ferruh.yigit, thomas.monjalon, dev

*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 <christian.ehrhardt@canonical.com>
---
 mk/rte.sdkconfig.mk | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index a3acfe6..6c46122 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) ; \
+		tac $(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 \
+			| tac > $(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 ; \
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] mk: filter duplicate configuration entries
  2016-06-30 12:00               ` [PATCH v2] " Christian Ehrhardt
@ 2016-07-05 16:47                 ` Ferruh Yigit
  2016-07-05 19:47                   ` Thomas Monjalon
  0 siblings, 1 reply; 22+ messages in thread
From: Ferruh Yigit @ 2016-07-05 16:47 UTC (permalink / raw)
  To: Christian Ehrhardt, thomas.monjalon, dev

On 6/30/2016 1:00 PM, Christian Ehrhardt wrote:
> *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 <christian.ehrhardt@canonical.com>
> ---
>  mk/rte.sdkconfig.mk | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
> index a3acfe6..6c46122 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) ; \
> +		tac $(RTE_OUTPUT)/.config_tmp > $(RTE_OUTPUT)/.config_tmp_reverse ; \
Now we are adding new binary dependency (tac) to build system and we now
create a interim file, as far as I understand which is required to use awk.
Although this is a nice piece of awk command, I am not sure if sed
alternative or this is better because of above two issues. This is a
question that I have more than a comment against one to another.

If you prefer to use this one, I confirmed that this works fine.

> +		awk --field-separator '=' '!/^#/ {if (!seen[$$1]) {print ($$0)}; seen[$$1]=1;} \
> +			/^#/ {print($$0)}' $(RTE_OUTPUT)/.config_tmp_reverse \
> +			| tac > $(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 ; \
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] mk: filter duplicate configuration entries
  2016-07-05 16:47                 ` Ferruh Yigit
@ 2016-07-05 19:47                   ` Thomas Monjalon
  2016-07-06  5:37                     ` Christian Ehrhardt
  0 siblings, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2016-07-05 19:47 UTC (permalink / raw)
  To: Ferruh Yigit, Christian Ehrhardt; +Cc: dev

2016-07-05 17:47, Ferruh Yigit:
> On 6/30/2016 1:00 PM, Christian Ehrhardt wrote:
> > +		tac $(RTE_OUTPUT)/.config_tmp > $(RTE_OUTPUT)/.config_tmp_reverse ; \
> Now we are adding new binary dependency (tac) to build system

tac can be replaced by sed '1!G;h;$!d'

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] mk: filter duplicate configuration entries
  2016-07-05 19:47                   ` Thomas Monjalon
@ 2016-07-06  5:37                     ` Christian Ehrhardt
  2016-07-06  5:37                       ` [PATCH v3] " Christian Ehrhardt
  0 siblings, 1 reply; 22+ messages in thread
From: Christian Ehrhardt @ 2016-07-06  5:37 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Ferruh Yigit, dev

Hi,
I came up with something very similar when looking for tac replacements
yesterday, but had no time to finish things.
But your suggestion is even shorter - I had found "sed -n '1{h;T;};G;h;$p;'
file" or "sed -n '1!G;h;$p'".
That removes the tac dependency, which I agree is a good thing.

To chain things up without a temp file one would need the "in-place"
features of sed&awk which I'm not sure they are available (awk >=4.1 and
only GNU awk).
sed -i is only used in validate-abi.sh which might not be used on all
platforms to count as "-i is there already so I can use it".
And I really don't want to break anyone due to that change, just naively
clean up the resulting config a bit.
Also we already have a temp file .config_tmp in the same scope and remove
it on our own.
So it is not that much different to create and remove a second one for that
section.

Thanks for both of your feedback, submitting v3 now ...


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Jul 5, 2016 at 9:47 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:

> 2016-07-05 17:47, Ferruh Yigit:
> > On 6/30/2016 1:00 PM, Christian Ehrhardt wrote:
> > > +           tac $(RTE_OUTPUT)/.config_tmp >
> $(RTE_OUTPUT)/.config_tmp_reverse ; \
> > Now we are adding new binary dependency (tac) to build system
>
> tac can be replaced by sed '1!G;h;$!d'
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3] mk: filter duplicate configuration entries
  2016-07-06  5:37                     ` Christian Ehrhardt
@ 2016-07-06  5:37                       ` Christian Ehrhardt
  2016-07-06  8:06                         ` Ferruh Yigit
  2016-07-06  8:12                         ` Bruce Richardson
  0 siblings, 2 replies; 22+ messages in thread
From: Christian Ehrhardt @ 2016-07-06  5:37 UTC (permalink / raw)
  To: christian.ehrhardt, ferruh.yigit, thomas.monjalon, dev

*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 <christian.ehrhardt@canonical.com>
---
 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 ; \
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] mk: filter duplicate configuration entries
  2016-07-06  5:37                       ` [PATCH v3] " Christian Ehrhardt
@ 2016-07-06  8:06                         ` Ferruh Yigit
  2016-07-06  8:12                         ` Bruce Richardson
  1 sibling, 0 replies; 22+ messages in thread
From: Ferruh Yigit @ 2016-07-06  8:06 UTC (permalink / raw)
  To: Christian Ehrhardt, thomas.monjalon, dev

On 7/6/2016 6:37 AM, 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 <christian.ehrhardt@canonical.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] mk: filter duplicate configuration entries
  2016-07-06  5:37                       ` [PATCH v3] " Christian Ehrhardt
  2016-07-06  8:06                         ` Ferruh Yigit
@ 2016-07-06  8:12                         ` Bruce Richardson
  2016-07-06  8:57                           ` Ferruh Yigit
  1 sibling, 1 reply; 22+ messages in thread
From: Bruce Richardson @ 2016-07-06  8:12 UTC (permalink / raw)
  To: Christian Ehrhardt; +Cc: ferruh.yigit, thomas.monjalon, dev

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 <christian.ehrhardt@canonical.com>
> ---
>  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.

/Bruce

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] mk: filter duplicate configuration entries
  2016-07-06  8:12                         ` Bruce Richardson
@ 2016-07-06  8:57                           ` Ferruh Yigit
  2016-07-06  9:08                             ` Christian Ehrhardt
  2016-07-06  9:30                             ` [PATCH v3] " Bruce Richardson
  0 siblings, 2 replies; 22+ messages in thread
From: Ferruh Yigit @ 2016-07-06  8:57 UTC (permalink / raw)
  To: Bruce Richardson, Christian Ehrhardt; +Cc: thomas.monjalon, dev

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 <christian.ehrhardt@canonical.com>
>> ---
>>  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; \

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] mk: filter duplicate configuration entries
  2016-07-06  8:57                           ` Ferruh Yigit
@ 2016-07-06  9:08                             ` Christian Ehrhardt
  2016-07-06  9:13                               ` [PATCH v4] " Christian Ehrhardt
  2016-07-06  9:30                             ` [PATCH v3] " Bruce Richardson
  1 sibling, 1 reply; 22+ messages in thread
From: Christian Ehrhardt @ 2016-07-06  9:08 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Bruce Richardson, Thomas Monjalon, 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 <ferruh.yigit@intel.com>
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 <christian.ehrhardt@canonical.com>
> >> ---
> >>  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; \
>
>
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4] mk: filter duplicate configuration entries
  2016-07-06  9:08                             ` Christian Ehrhardt
@ 2016-07-06  9:13                               ` Christian Ehrhardt
  2016-07-11 12:47                                 ` Thomas Monjalon
  0 siblings, 1 reply; 22+ messages in thread
From: Christian Ehrhardt @ 2016-07-06  9:13 UTC (permalink / raw)
  To: christian.ehrhardt, ferruh.yigit, thomas.monjalon, dev

*updates in v4*
- replace awk usage with sed
- re-add the old loop to be able to get rid of awk
- add more explanation to the header of the makefile section

*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 <christian.ehrhardt@canonical.com>
---
 mk/rte.sdkconfig.mk | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index a3acfe6..70c59d6 100644
--- a/mk/rte.sdkconfig.mk
+++ b/mk/rte.sdkconfig.mk
@@ -79,11 +79,20 @@ $(RTE_OUTPUT):
 ifdef NODOTCONF
 $(RTE_OUTPUT)/.config: ;
 else
+# Generate config from template, if there are duplicates keep only the last.
+# To do so the temp config is checked for duplicate keys with cut/sort/uniq
+# Then for each of those identified duplicates as long as there are more than
+# just one left the last match is removed.
 $(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) ; \
+		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; \
 		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 ; \
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] mk: filter duplicate configuration entries
  2016-07-06  8:57                           ` Ferruh Yigit
  2016-07-06  9:08                             ` Christian Ehrhardt
@ 2016-07-06  9:30                             ` Bruce Richardson
  1 sibling, 0 replies; 22+ messages in thread
From: Bruce Richardson @ 2016-07-06  9:30 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Christian Ehrhardt, thomas.monjalon, dev

On Wed, Jul 06, 2016 at 09:57:01AM +0100, 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 <christian.ehrhardt@canonical.com>
> >> ---
> >>  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; \
> 

Well, at least on my Fedora box, tac is part of coreutils, so it's probably
pretty widespread on Linux. Unfortunately, it's not available out-of-the-box
on FreeBSD.

/Bruce

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4] mk: filter duplicate configuration entries
  2016-07-06  9:13                               ` [PATCH v4] " Christian Ehrhardt
@ 2016-07-11 12:47                                 ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2016-07-11 12:47 UTC (permalink / raw)
  To: Christian Ehrhardt; +Cc: ferruh.yigit, dev

2016-07-06 11:13, Christian Ehrhardt:
> *updates in v4*
> - replace awk usage with sed
> - re-add the old loop to be able to get rid of awk
> - add more explanation to the header of the makefile section
> 
> *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 <christian.ehrhardt@canonical.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2016-07-11 12:47 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-13 11:50 Duplicate config symbols Christian Ehrhardt
2016-06-13 13:07 ` Ferruh Yigit
2016-06-13 13:47 ` Thomas Monjalon
2016-06-13 15:09   ` Christian Ehrhardt
2016-06-13 15:10     ` [RFC] mk: filter duplicate configuration entries Christian Ehrhardt
2016-06-28 16:11       ` Ferruh Yigit
2016-06-28 16:38         ` Christian Ehrhardt
2016-06-28 16:48           ` Ferruh Yigit
2016-06-30 11:57             ` Christian Ehrhardt
2016-06-30 12:00               ` [PATCH v2] " Christian Ehrhardt
2016-07-05 16:47                 ` Ferruh Yigit
2016-07-05 19:47                   ` Thomas Monjalon
2016-07-06  5:37                     ` Christian Ehrhardt
2016-07-06  5:37                       ` [PATCH v3] " Christian Ehrhardt
2016-07-06  8:06                         ` Ferruh Yigit
2016-07-06  8:12                         ` Bruce Richardson
2016-07-06  8:57                           ` Ferruh Yigit
2016-07-06  9:08                             ` Christian Ehrhardt
2016-07-06  9:13                               ` [PATCH v4] " Christian Ehrhardt
2016-07-11 12:47                                 ` Thomas Monjalon
2016-07-06  9:30                             ` [PATCH v3] " Bruce Richardson
2016-06-13 16:55     ` Duplicate config symbols Thomas Monjalon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.