* [dpdk-dev] 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: [dpdk-dev] Duplicate config symbols 2016-06-13 11:50 [dpdk-dev] 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: [dpdk-dev] Duplicate config symbols 2016-06-13 11:50 [dpdk-dev] 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: [dpdk-dev] Duplicate config symbols 2016-06-13 13:47 ` Thomas Monjalon @ 2016-06-13 15:09 ` Christian Ehrhardt 2016-06-13 15:10 ` [dpdk-dev] [RFC] mk: filter duplicate configuration entries Christian Ehrhardt 2016-06-13 16:55 ` [dpdk-dev] 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
* [dpdk-dev] [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 ` [dpdk-dev] 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 [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] [RFC] mk: filter duplicate configuration entries 2016-06-13 15:10 ` [dpdk-dev] [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: [dpdk-dev] [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: [dpdk-dev] [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: [dpdk-dev] [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 ` [dpdk-dev] [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
* [dpdk-dev] [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 [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mk: filter duplicate configuration entries 2016-06-30 12:00 ` [dpdk-dev] [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: [dpdk-dev] [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: [dpdk-dev] [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 ` [dpdk-dev] [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
* [dpdk-dev] [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 [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] [PATCH v3] mk: filter duplicate configuration entries 2016-07-06 5:37 ` [dpdk-dev] [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: [dpdk-dev] [PATCH v3] mk: filter duplicate configuration entries 2016-07-06 5:37 ` [dpdk-dev] [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: [dpdk-dev] [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 ` [dpdk-dev] [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: [dpdk-dev] [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 ` [dpdk-dev] [PATCH v4] " Christian Ehrhardt 2016-07-06 9:30 ` [dpdk-dev] [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
* [dpdk-dev] [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 [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] [PATCH v4] mk: filter duplicate configuration entries 2016-07-06 9:13 ` [dpdk-dev] [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
* Re: [dpdk-dev] [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: [dpdk-dev] Duplicate config symbols 2016-06-13 15:09 ` Christian Ehrhardt 2016-06-13 15:10 ` [dpdk-dev] [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
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 [dpdk-dev] 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 ` [dpdk-dev] [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 ` [dpdk-dev] [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 ` [dpdk-dev] [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 ` [dpdk-dev] [PATCH v4] " Christian Ehrhardt 2016-07-11 12:47 ` Thomas Monjalon 2016-07-06 9:30 ` [dpdk-dev] [PATCH v3] " Bruce Richardson 2016-06-13 16:55 ` [dpdk-dev] Duplicate config symbols Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).