DPDK patches and discussions
 help / color / mirror / Atom feed
* [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).