* [dpdk-dev] [PATCH] mk: fix FreeBSD build @ 2016-07-18 13:11 Sergio Gonzalez Monroy 2016-07-18 13:25 ` Thomas Monjalon 2016-07-18 16:06 ` [dpdk-dev] [PATCH v2] " Sergio Gonzalez Monroy 0 siblings, 2 replies; 15+ messages in thread From: Sergio Gonzalez Monroy @ 2016-07-18 13:11 UTC (permalink / raw) To: dev The sed syntax of '0,/regexp/' is GNU specific and fails with non GNU sed in FreeBSD. To solve the issue we can use awk instead to remove duplicates. Fixes: b2063f104db7 ("mk: filter duplicate configuration entries") Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> --- mk/rte.sdkconfig.mk | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk index e93237f..5c28b7b 100644 --- a/mk/rte.sdkconfig.mk +++ b/mk/rte.sdkconfig.mk @@ -88,11 +88,9 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | $(RTE_OUTPUT) $(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; \ + grep -v "^#" $(RTE_OUTPUT)/.config_tmp | awk -F'=' '{a[$$1]=$$0} END {for (i in a) print a[i]}' > $(RTE_OUTPUT)/.config_tmp2 ; \ + mv $(RTE_OUTPUT)/.config_tmp2 $(RTE_OUTPUT)/.config_tmp ; \ + rm -f $(RTE_OUTPUT)/.config_tmp2 ; \ 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.4.11 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH] mk: fix FreeBSD build 2016-07-18 13:11 [dpdk-dev] [PATCH] mk: fix FreeBSD build Sergio Gonzalez Monroy @ 2016-07-18 13:25 ` Thomas Monjalon 2016-07-18 13:39 ` Sergio Gonzalez Monroy 2016-07-18 16:06 ` [dpdk-dev] [PATCH v2] " Sergio Gonzalez Monroy 1 sibling, 1 reply; 15+ messages in thread From: Thomas Monjalon @ 2016-07-18 13:25 UTC (permalink / raw) To: Sergio Gonzalez Monroy, christian.ehrhardt; +Cc: dev 2016-07-18 14:11, Sergio Gonzalez Monroy: > The sed syntax of '0,/regexp/' is GNU specific and fails with > non GNU sed in FreeBSD. > > To solve the issue we can use awk instead to remove duplicates. Christian, an opinion please? > Fixes: b2063f104db7 ("mk: filter duplicate configuration entries") > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> [...] > - 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; \ > + grep -v "^#" $(RTE_OUTPUT)/.config_tmp | awk -F'=' '{a[$$1]=$$0} END {for (i in a) print a[i]}' > $(RTE_OUTPUT)/.config_tmp2 ; \ > + mv $(RTE_OUTPUT)/.config_tmp2 $(RTE_OUTPUT)/.config_tmp ; \ > + rm -f $(RTE_OUTPUT)/.config_tmp2 ; \ You can avoid creating/deleting the file .config_tmp2 by using a variable: config=$(grep -v '^#' $(RTE_OUTPUT)/.config_tmp) echo "$config" | awk ... > $(RTE_OUTPUT)/.config_tmp ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH] mk: fix FreeBSD build 2016-07-18 13:25 ` Thomas Monjalon @ 2016-07-18 13:39 ` Sergio Gonzalez Monroy 2016-07-18 13:54 ` Christian Ehrhardt 0 siblings, 1 reply; 15+ messages in thread From: Sergio Gonzalez Monroy @ 2016-07-18 13:39 UTC (permalink / raw) To: Thomas Monjalon, christian.ehrhardt; +Cc: dev On 18/07/2016 14:25, Thomas Monjalon wrote: > 2016-07-18 14:11, Sergio Gonzalez Monroy: >> The sed syntax of '0,/regexp/' is GNU specific and fails with >> non GNU sed in FreeBSD. >> >> To solve the issue we can use awk instead to remove duplicates. > Christian, an opinion please? Sorry, forgot to CC him. >> Fixes: b2063f104db7 ("mk: filter duplicate configuration entries") >> >> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> > [...] >> - 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; \ >> + grep -v "^#" $(RTE_OUTPUT)/.config_tmp | awk -F'=' '{a[$$1]=$$0} END {for (i in a) print a[i]}' > $(RTE_OUTPUT)/.config_tmp2 ; \ >> + mv $(RTE_OUTPUT)/.config_tmp2 $(RTE_OUTPUT)/.config_tmp ; \ >> + rm -f $(RTE_OUTPUT)/.config_tmp2 ; \ > You can avoid creating/deleting the file .config_tmp2 by using a variable: > config=$(grep -v '^#' $(RTE_OUTPUT)/.config_tmp) > echo "$config" | awk ... > $(RTE_OUTPUT)/.config_tmp Sure. Sergio ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH] mk: fix FreeBSD build 2016-07-18 13:39 ` Sergio Gonzalez Monroy @ 2016-07-18 13:54 ` Christian Ehrhardt 2016-07-18 14:06 ` Sergio Gonzalez Monroy 2016-07-18 14:07 ` Thomas Monjalon 0 siblings, 2 replies; 15+ messages in thread From: Christian Ehrhardt @ 2016-07-18 13:54 UTC (permalink / raw) To: Sergio Gonzalez Monroy; +Cc: Thomas Monjalon, dev, Ferruh Yigit Hi Sergio, you might have seen that I had a similar version with awk in v2 IIRC. I also had the secondary tmp file just like you now. So, since it is so close to my old submission I wont object :-) Back then the discussion went for reduced build time dependencies and avoiding a second temp file, which was ok for me - so sed was chosen. I see that breaking on BSD causes us to rework it again, sorry that I was unable to test there. If you could come up with a Solution "sed + no-temp2 + noGNUspecifics" that would be great and solve everybodies needs. If not, it is a call up to the participants of the old discussion if not working on BSD outweighs their old feedback (I guess so). Most active in the discussion back then was Ferruh IIRC - setting to CC. Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd On Mon, Jul 18, 2016 at 3:39 PM, Sergio Gonzalez Monroy < sergio.gonzalez.monroy@intel.com> wrote: > On 18/07/2016 14:25, Thomas Monjalon wrote: > >> 2016-07-18 14:11, Sergio Gonzalez Monroy: >> >>> The sed syntax of '0,/regexp/' is GNU specific and fails with >>> non GNU sed in FreeBSD. >>> >>> To solve the issue we can use awk instead to remove duplicates. >>> >> Christian, an opinion please? >> > > Sorry, forgot to CC him. > > Fixes: b2063f104db7 ("mk: filter duplicate configuration entries") >>> >>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> >>> >> [...] >> >>> - 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; \ >>> + grep -v "^#" $(RTE_OUTPUT)/.config_tmp | awk -F'=' >>> '{a[$$1]=$$0} END {for (i in a) print a[i]}' > $(RTE_OUTPUT)/.config_tmp2 ; >>> \ >>> + mv $(RTE_OUTPUT)/.config_tmp2 $(RTE_OUTPUT)/.config_tmp >>> ; \ >>> + rm -f $(RTE_OUTPUT)/.config_tmp2 ; \ >>> >> You can avoid creating/deleting the file .config_tmp2 by using a variable: >> config=$(grep -v '^#' $(RTE_OUTPUT)/.config_tmp) >> echo "$config" | awk ... > $(RTE_OUTPUT)/.config_tmp >> > > Sure. > > Sergio > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH] mk: fix FreeBSD build 2016-07-18 13:54 ` Christian Ehrhardt @ 2016-07-18 14:06 ` Sergio Gonzalez Monroy 2016-07-18 14:07 ` Thomas Monjalon 1 sibling, 0 replies; 15+ messages in thread From: Sergio Gonzalez Monroy @ 2016-07-18 14:06 UTC (permalink / raw) To: Christian Ehrhardt; +Cc: Thomas Monjalon, dev, Ferruh Yigit On 18/07/2016 14:54, Christian Ehrhardt wrote: > Hi Sergio, > you might have seen that I had a similar version with awk in v2 IIRC. > I also had the secondary tmp file just like you now. > So, since it is so close to my old submission I wont object :-) > > Back then the discussion went for reduced build time dependencies and > avoiding a second temp file, which was ok for me - so sed was chosen. > I haven't seen a noticeable performance impact by using second temp file. Thomas has suggested using a temp var instead of second temp file, what do you think about that? > I see that breaking on BSD causes us to rework it again, sorry that I > was unable to test there. > No worries. > If you could come up with a Solution "sed + no-temp2 + noGNUspecifics" > that would be great and solve everybodies needs. > If not, it is a call up to the participants of the old discussion if > not working on BSD outweighs their old feedback (I guess so). > Any reason of sed over awk? I reckon awk is simpler syntax for this job. From what I have seen most of the time is spent on resolving directory/library dependencies not on the .config itself. Sergio > Most active in the discussion back then was Ferruh IIRC - setting to CC. > > > > Christian Ehrhardt > Software Engineer, Ubuntu Server > Canonical Ltd > > On Mon, Jul 18, 2016 at 3:39 PM, Sergio Gonzalez Monroy > <sergio.gonzalez.monroy@intel.com > <mailto:sergio.gonzalez.monroy@intel.com>> wrote: > > On 18/07/2016 14:25, Thomas Monjalon wrote: > > 2016-07-18 14:11, Sergio Gonzalez Monroy: > > The sed syntax of '0,/regexp/' is GNU specific and fails with > non GNU sed in FreeBSD. > > To solve the issue we can use awk instead to remove > duplicates. > > Christian, an opinion please? > > > Sorry, forgot to CC him. > > Fixes: b2063f104db7 ("mk: filter duplicate configuration > entries") > > Signed-off-by: Sergio Gonzalez Monroy > <sergio.gonzalez.monroy@intel.com > <mailto:sergio.gonzalez.monroy@intel.com>> > > [...] > > - 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; \ > + grep -v "^#" $(RTE_OUTPUT)/.config_tmp | > awk -F'=' '{a[$$1]=$$0} END {for (i in a) print a[i]}' > > $(RTE_OUTPUT)/.config_tmp2 ; \ > + mv $(RTE_OUTPUT)/.config_tmp2 > $(RTE_OUTPUT)/.config_tmp ; \ > + rm -f $(RTE_OUTPUT)/.config_tmp2 ; \ > > You can avoid creating/deleting the file .config_tmp2 by using > a variable: > config=$(grep -v '^#' $(RTE_OUTPUT)/.config_tmp) > echo "$config" | awk ... > $(RTE_OUTPUT)/.config_tmp > > > Sure. > > Sergio > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH] mk: fix FreeBSD build 2016-07-18 13:54 ` Christian Ehrhardt 2016-07-18 14:06 ` Sergio Gonzalez Monroy @ 2016-07-18 14:07 ` Thomas Monjalon 2016-07-18 14:51 ` Sergio Gonzalez Monroy 1 sibling, 1 reply; 15+ messages in thread From: Thomas Monjalon @ 2016-07-18 14:07 UTC (permalink / raw) To: Christian Ehrhardt; +Cc: Sergio Gonzalez Monroy, dev, Ferruh Yigit 2016-07-18 15:54, Christian Ehrhardt: > Hi Sergio, > you might have seen that I had a similar version with awk in v2 IIRC. I > also had the secondary tmp file just like you now. > So, since it is so close to my old submission I wont object :-) > > Back then the discussion went for reduced build time dependencies and > avoiding a second temp file, which was ok for me - so sed was chosen. I don't see "awk" as a real dependency. I think it is as much common as "sed". Isn't it? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH] mk: fix FreeBSD build 2016-07-18 14:07 ` Thomas Monjalon @ 2016-07-18 14:51 ` Sergio Gonzalez Monroy 0 siblings, 0 replies; 15+ messages in thread From: Sergio Gonzalez Monroy @ 2016-07-18 14:51 UTC (permalink / raw) To: Thomas Monjalon, Christian Ehrhardt; +Cc: dev, Ferruh Yigit On 18/07/2016 15:07, Thomas Monjalon wrote: > 2016-07-18 15:54, Christian Ehrhardt: >> Hi Sergio, >> you might have seen that I had a similar version with awk in v2 IIRC. I >> also had the secondary tmp file just like you now. >> So, since it is so close to my old submission I wont object :-) >> >> Back then the discussion went for reduced build time dependencies and >> avoiding a second temp file, which was ok for me - so sed was chosen. > I don't see "awk" as a real dependency. I think it is as much common > as "sed". Isn't it? I would agree. So v2 using temp var instead? Sergio ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-dev] [PATCH v2] mk: fix FreeBSD build 2016-07-18 13:11 [dpdk-dev] [PATCH] mk: fix FreeBSD build Sergio Gonzalez Monroy 2016-07-18 13:25 ` Thomas Monjalon @ 2016-07-18 16:06 ` Sergio Gonzalez Monroy 2016-07-19 10:32 ` Ferruh Yigit 2016-07-19 13:40 ` [dpdk-dev] [PATCH v3] " Sergio Gonzalez Monroy 1 sibling, 2 replies; 15+ messages in thread From: Sergio Gonzalez Monroy @ 2016-07-18 16:06 UTC (permalink / raw) To: dev; +Cc: thomas.monjalon, christian.ehrhardt, ferruh.yigit The sed syntax of '0,/regexp/' is GNU specific and fails with non GNU sed in FreeBSD. To solve the issue we can use awk instead to remove duplicates. Fixes: b2063f104db7 ("mk: filter duplicate configuration entries") Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> --- v2: - Use temp var instead of temp file mk/rte.sdkconfig.mk | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk index e93237f..c2b6e13 100644 --- a/mk/rte.sdkconfig.mk +++ b/mk/rte.sdkconfig.mk @@ -88,11 +88,8 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | $(RTE_OUTPUT) $(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; \ + config=$$(grep -v "^#" $(RTE_OUTPUT)/.config_tmp) ; \ + echo "$$config" | awk -F'=' '{a[$$1]=$$0} END {for (i in a) print a[i]}' > $(RTE_OUTPUT)/.config_tmp ; \ 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.4.11 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mk: fix FreeBSD build 2016-07-18 16:06 ` [dpdk-dev] [PATCH v2] " Sergio Gonzalez Monroy @ 2016-07-19 10:32 ` Ferruh Yigit 2016-07-19 11:01 ` Christian Ehrhardt 2016-07-19 13:40 ` [dpdk-dev] [PATCH v3] " Sergio Gonzalez Monroy 1 sibling, 1 reply; 15+ messages in thread From: Ferruh Yigit @ 2016-07-19 10:32 UTC (permalink / raw) To: Sergio Gonzalez Monroy, dev; +Cc: thomas.monjalon, christian.ehrhardt On 7/18/2016 5:06 PM, Sergio Gonzalez Monroy wrote: > The sed syntax of '0,/regexp/' is GNU specific and fails with > non GNU sed in FreeBSD. > > To solve the issue we can use awk instead to remove duplicates. > > Fixes: b2063f104db7 ("mk: filter duplicate configuration entries") > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> > --- > > v2: > - Use temp var instead of temp file > > mk/rte.sdkconfig.mk | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk > index e93237f..c2b6e13 100644 > --- a/mk/rte.sdkconfig.mk > +++ b/mk/rte.sdkconfig.mk > @@ -88,11 +88,8 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | $(RTE_OUTPUT) > $(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; \ > + config=$$(grep -v "^#" $(RTE_OUTPUT)/.config_tmp) ; \ > + echo "$$config" | awk -F'=' '{a[$$1]=$$0} END {for (i in a) print a[i]}' > $(RTE_OUTPUT)/.config_tmp ; \ This is another nice awk command. A few comments about new command: - Removes all comments from final config - Spreads config option all over the file, logical grouping of options removed. When both happens at the same time, I have a concern that this may lead missing some config options when somebody wants to update local config file, but I am OK if everybody is OK. > 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] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mk: fix FreeBSD build 2016-07-19 10:32 ` Ferruh Yigit @ 2016-07-19 11:01 ` Christian Ehrhardt 2016-07-19 11:40 ` Sergio Gonzalez Monroy 0 siblings, 1 reply; 15+ messages in thread From: Christian Ehrhardt @ 2016-07-19 11:01 UTC (permalink / raw) To: Ferruh Yigit; +Cc: Sergio Gonzalez Monroy, dev, Thomas Monjalon Hi, I haven't tested the new suggested way, just went into explaining what formerly were the reasons. But I'd strongly vote against reordering and dropping comments. Sergio - v3 had still awk for some parts. It doesn't have the "0,..." you mentioned. Could you check if that is already using GNU-sed only syntax - http://dpdk.org/dev/patchwork/patch/14592/ ? If this would be ok - AND - if it creates the same .config as the current code I'd think that is the way to go. Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd On Tue, Jul 19, 2016 at 12:32 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote: > On 7/18/2016 5:06 PM, Sergio Gonzalez Monroy wrote: > > The sed syntax of '0,/regexp/' is GNU specific and fails with > > non GNU sed in FreeBSD. > > > > To solve the issue we can use awk instead to remove duplicates. > > > > Fixes: b2063f104db7 ("mk: filter duplicate configuration entries") > > > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> > > --- > > > > v2: > > - Use temp var instead of temp file > > > > mk/rte.sdkconfig.mk | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk > > index e93237f..c2b6e13 100644 > > --- a/mk/rte.sdkconfig.mk > > +++ b/mk/rte.sdkconfig.mk > > @@ -88,11 +88,8 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | > $(RTE_OUTPUT) > > $(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; \ > > + config=$$(grep -v "^#" $(RTE_OUTPUT)/.config_tmp) ; \ > > + echo "$$config" | awk -F'=' '{a[$$1]=$$0} END {for (i in > a) print a[i]}' > $(RTE_OUTPUT)/.config_tmp ; \ > This is another nice awk command. > > A few comments about new command: > - Removes all comments from final config > - Spreads config option all over the file, logical grouping of options > removed. > > When both happens at the same time, I have a concern that this may lead > missing some config options when somebody wants to update local config > file, but I am OK if everybody is OK. > > > > 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] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mk: fix FreeBSD build 2016-07-19 11:01 ` Christian Ehrhardt @ 2016-07-19 11:40 ` Sergio Gonzalez Monroy 0 siblings, 0 replies; 15+ messages in thread From: Sergio Gonzalez Monroy @ 2016-07-19 11:40 UTC (permalink / raw) To: Christian Ehrhardt; +Cc: dev, Thomas Monjalon, Ferruh Yigit Sorry Christian, I did miss your previous patches. On 19/07/2016 12:01, Christian Ehrhardt wrote: > Hi, > I haven't tested the new suggested way, just went into explaining what > formerly were the reasons. > But I'd strongly vote against reordering and dropping comments. > Fair enough. We would still have some not-order options if they are duplicated as we would just keep the last one but nevertheless most of them would be properly grouped and ordered. I'll rework something based on your v3 patch. Sergio > Sergio - v3 had still awk for some parts. > It doesn't have the "0,..." you mentioned. > Could you check if that is already using GNU-sed only syntax - > http://dpdk.org/dev/patchwork/patch/14592/ ? > > If this would be ok - AND - if it creates the same .config as the > current code I'd think that is the way to go. > > > Christian Ehrhardt > Software Engineer, Ubuntu Server > Canonical Ltd > > On Tue, Jul 19, 2016 at 12:32 PM, Ferruh Yigit <ferruh.yigit@intel.com > <mailto:ferruh.yigit@intel.com>> wrote: > > On 7/18/2016 5:06 PM, Sergio Gonzalez Monroy wrote: > > The sed syntax of '0,/regexp/' is GNU specific and fails with > > non GNU sed in FreeBSD. > > > > To solve the issue we can use awk instead to remove duplicates. > > > > Fixes: b2063f104db7 ("mk: filter duplicate configuration entries") > > > > Signed-off-by: Sergio Gonzalez Monroy > <sergio.gonzalez.monroy@intel.com > <mailto:sergio.gonzalez.monroy@intel.com>> > > --- > > > > v2: > > - Use temp var instead of temp file > > > > mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk> | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk> > b/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk> > > index e93237f..c2b6e13 100644 > > --- a/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk> > > +++ b/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk> > > @@ -88,11 +88,8 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) > FORCE | $(RTE_OUTPUT) > > $(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; \ > > + config=$$(grep -v "^#" $(RTE_OUTPUT)/.config_tmp) ; \ > > + echo "$$config" | awk -F'=' '{a[$$1]=$$0} END {for > (i in a) print a[i]}' > $(RTE_OUTPUT)/.config_tmp ; \ > This is another nice awk command. > > A few comments about new command: > - Removes all comments from final config > - Spreads config option all over the file, logical grouping of options > removed. > > When both happens at the same time, I have a concern that this may > lead > missing some config options when somebody wants to update local config > file, but I am OK if everybody is OK. > > > > 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] 15+ messages in thread
* [dpdk-dev] [PATCH v3] mk: fix FreeBSD build 2016-07-18 16:06 ` [dpdk-dev] [PATCH v2] " Sergio Gonzalez Monroy 2016-07-19 10:32 ` Ferruh Yigit @ 2016-07-19 13:40 ` Sergio Gonzalez Monroy 2016-07-19 14:30 ` Christian Ehrhardt 1 sibling, 1 reply; 15+ messages in thread From: Sergio Gonzalez Monroy @ 2016-07-19 13:40 UTC (permalink / raw) To: dev; +Cc: thomas.monjalon, christian.ehrhardt, ferruh.yigit The sed syntax of '0,/regexp/' is GNU specific and fails with non GNU sed in FreeBSD. To solve the issue we can use awk instead to remove duplicates. The awk script basically keeps the last config value, while maintaining order and comments from original config file. Fixes: b2063f104db7 ("mk: filter duplicate configuration entries") Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> --- mk/rte.sdkconfig.mk | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk index e93237f..5c94edf 100644 --- a/mk/rte.sdkconfig.mk +++ b/mk/rte.sdkconfig.mk @@ -88,11 +88,13 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | $(RTE_OUTPUT) $(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; \ + config=$$(cat $(RTE_OUTPUT)/.config_tmp) ; \ + echo "$$config" | awk -F '=' 'BEGIN {i=1} \ + /^#/ {pos[i++]=$$0} \ + !/^#/ {if (!s[$$1]) {pos[i]=$$0; s[$$1]=i++} \ + else {pos[s[$$1]]=$$0}} END \ + {for (j=1; j<i; j++) print pos[j]}' \ + > $(RTE_OUTPUT)/.config_tmp ; \ 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.4.11 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v3] mk: fix FreeBSD build 2016-07-19 13:40 ` [dpdk-dev] [PATCH v3] " Sergio Gonzalez Monroy @ 2016-07-19 14:30 ` Christian Ehrhardt 2016-07-19 15:04 ` Ferruh Yigit 2016-07-21 8:27 ` Thomas Monjalon 0 siblings, 2 replies; 15+ messages in thread From: Christian Ehrhardt @ 2016-07-19 14:30 UTC (permalink / raw) To: Sergio Gonzalez Monroy; +Cc: dev, Thomas Monjalon, Ferruh Yigit Hi, the order changed, but IMHO actually it improved. Things are now at the place they were before, but with the overwriting config value that came later - that is the best of both worlds. Tested a few config runs pre/post patch and compared them sorted (equal) and unsorted - now configs slotted in where they belong. Thanks for updating Sergio Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd On Tue, Jul 19, 2016 at 3:40 PM, Sergio Gonzalez Monroy < sergio.gonzalez.monroy@intel.com> wrote: > The sed syntax of '0,/regexp/' is GNU specific and fails with > non GNU sed in FreeBSD. > > To solve the issue we can use awk instead to remove duplicates. > > The awk script basically keeps the last config value, while > maintaining order and comments from original config file. > > Fixes: b2063f104db7 ("mk: filter duplicate configuration entries") > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> > --- > mk/rte.sdkconfig.mk | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk > index e93237f..5c94edf 100644 > --- a/mk/rte.sdkconfig.mk > +++ b/mk/rte.sdkconfig.mk > @@ -88,11 +88,13 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | > $(RTE_OUTPUT) > $(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; \ > + config=$$(cat $(RTE_OUTPUT)/.config_tmp) ; \ > + echo "$$config" | awk -F '=' 'BEGIN {i=1} \ > + /^#/ {pos[i++]=$$0} \ > + !/^#/ {if (!s[$$1]) {pos[i]=$$0; s[$$1]=i++} \ > + else {pos[s[$$1]]=$$0}} END \ > + {for (j=1; j<i; j++) print pos[j]}' \ > + > $(RTE_OUTPUT)/.config_tmp ; \ > 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.4.11 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v3] mk: fix FreeBSD build 2016-07-19 14:30 ` Christian Ehrhardt @ 2016-07-19 15:04 ` Ferruh Yigit 2016-07-21 8:27 ` Thomas Monjalon 1 sibling, 0 replies; 15+ messages in thread From: Ferruh Yigit @ 2016-07-19 15:04 UTC (permalink / raw) To: Christian Ehrhardt, Sergio Gonzalez Monroy; +Cc: dev, Thomas Monjalon On 7/19/2016 3:30 PM, Christian Ehrhardt wrote: > Hi, > the order changed, but IMHO actually it improved. > Things are now at the place they were before, but with the overwriting > config value that came later - that is the best of both worlds. Agreed, this is a good solution. Also fixes multiple copy of file comment issue ... > > Tested a few config runs pre/post patch and compared them sorted (equal) > and unsorted - now configs slotted in where they belong. > Thanks for updating Sergio > > Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com > <mailto:christian.ehrhardt@canonical.com>> > > Christian Ehrhardt > Software Engineer, Ubuntu Server > Canonical Ltd > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-dev] [PATCH v3] mk: fix FreeBSD build 2016-07-19 14:30 ` Christian Ehrhardt 2016-07-19 15:04 ` Ferruh Yigit @ 2016-07-21 8:27 ` Thomas Monjalon 1 sibling, 0 replies; 15+ messages in thread From: Thomas Monjalon @ 2016-07-21 8:27 UTC (permalink / raw) To: Sergio Gonzalez Monroy; +Cc: Christian Ehrhardt, dev, Ferruh Yigit > > The sed syntax of '0,/regexp/' is GNU specific and fails with > > non GNU sed in FreeBSD. > > > > To solve the issue we can use awk instead to remove duplicates. > > > > The awk script basically keeps the last config value, while > > maintaining order and comments from original config file. > > > > Fixes: b2063f104db7 ("mk: filter duplicate configuration entries") > > > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> > > Hi, > the order changed, but IMHO actually it improved. > Things are now at the place they were before, but with the overwriting > config value that came later - that is the best of both worlds. > > Tested a few config runs pre/post patch and compared them sorted (equal) > and unsorted - now configs slotted in where they belong. > Thanks for updating Sergio > > Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Applied, thanks ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-07-21 8:27 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-07-18 13:11 [dpdk-dev] [PATCH] mk: fix FreeBSD build Sergio Gonzalez Monroy 2016-07-18 13:25 ` Thomas Monjalon 2016-07-18 13:39 ` Sergio Gonzalez Monroy 2016-07-18 13:54 ` Christian Ehrhardt 2016-07-18 14:06 ` Sergio Gonzalez Monroy 2016-07-18 14:07 ` Thomas Monjalon 2016-07-18 14:51 ` Sergio Gonzalez Monroy 2016-07-18 16:06 ` [dpdk-dev] [PATCH v2] " Sergio Gonzalez Monroy 2016-07-19 10:32 ` Ferruh Yigit 2016-07-19 11:01 ` Christian Ehrhardt 2016-07-19 11:40 ` Sergio Gonzalez Monroy 2016-07-19 13:40 ` [dpdk-dev] [PATCH v3] " Sergio Gonzalez Monroy 2016-07-19 14:30 ` Christian Ehrhardt 2016-07-19 15:04 ` Ferruh Yigit 2016-07-21 8:27 ` 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).