[dpdk-dev] mk: fix FreeBSD build

Message ID 1468847463-107132-1-git-send-email-sergio.gonzalez.monroy@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Sergio Gonzalez Monroy July 18, 2016, 1:11 p.m. UTC
  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(-)
  

Comments

Thomas Monjalon July 18, 2016, 1:25 p.m. UTC | #1
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
  
Sergio Gonzalez Monroy July 18, 2016, 1:39 p.m. UTC | #2
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
  
Christian Ehrhardt July 18, 2016, 1:54 p.m. UTC | #3
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
>
  
Sergio Gonzalez Monroy July 18, 2016, 2:06 p.m. UTC | #4
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
>
>
  
Thomas Monjalon July 18, 2016, 2:07 p.m. UTC | #5
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?
  
Sergio Gonzalez Monroy July 18, 2016, 2:51 p.m. UTC | #6
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
  

Patch

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 ; \