diff mbox series

[v2,1/3] package/libgeos: fix build failure due to missing -mcmodel=large

Message ID 20210726223435.2906458-2-giulio.benetti@benettiengineering.com
State Accepted
Headers show
Series Fix binutils bug 21464 and remove it | expand

Commit Message

Giulio Benetti July 26, 2021, 10:34 p.m. UTC
When building libgeos for or1k -mcmodel=large is needed to link, so let's
add that gcc option in case we're building for or1k.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
 package/libgeos/libgeos.mk | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Maxim Kochetkov July 27, 2021, 4:26 a.m. UTC | #1
27.07.2021 01:34, Giulio Benetti wrote:
> When building libgeos for or1k -mcmodel=large is needed to link, so let's
> add that gcc option in case we're building for or1k.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> ---
>   package/libgeos/libgeos.mk | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/package/libgeos/libgeos.mk b/package/libgeos/libgeos.mk
> index a355732af8..43211d0434 100644
> --- a/package/libgeos/libgeos.mk
> +++ b/package/libgeos/libgeos.mk
> @@ -23,6 +23,10 @@ ifeq ($(BR2_arm)$(BR2_armeb),y)
>   LIBGEOS_CONF_OPTS += -DDISABLE_GEOS_INLINE=ON
>   endif
>   
> +ifeq ($(BR2_or1k),y)
> +LIBGEOS_CXXFLAGS += -mcmodel=large
> +endif
> +
>   LIBGEOS_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(LIBGEOS_CXXFLAGS)"
>   
>   $(eval $(cmake-package))
> 
Reviewed-by: Maxim Kochetkov <fido_max@inbox.ru>
Arnout Vandecappelle Aug. 17, 2021, 8:34 p.m. UTC | #2
On 27/07/2021 00:34, Giulio Benetti wrote:
> When building libgeos for or1k -mcmodel=large is needed to link, so let's
> add that gcc option in case we're building for or1k.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> ---
>  package/libgeos/libgeos.mk | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/libgeos/libgeos.mk b/package/libgeos/libgeos.mk
> index a355732af8..43211d0434 100644
> --- a/package/libgeos/libgeos.mk
> +++ b/package/libgeos/libgeos.mk
> @@ -23,6 +23,10 @@ ifeq ($(BR2_arm)$(BR2_armeb),y)
>  LIBGEOS_CONF_OPTS += -DDISABLE_GEOS_INLINE=ON
>  endif
>  
> +ifeq ($(BR2_or1k),y)
> +LIBGEOS_CXXFLAGS += -mcmodel=large

 According to git, -mcmodel was added to GCC after the 11.2.0 release. Are you
sure that this works?

 Oooh, we have a patch that adds it. But that patch is missing for 11.1.0 for
some reason. Romain, you apparently didn't carry any of our patches when you
added GCC 11 in 2ab88dac293aed5adf2504e8b182f9d8a306b935. The commit message
doesn't say anything. Do you remember anything about the patches? The patch was
added to master on May 21 and you only submitted the GCC bump on June 12, but
perhaps you already had it lying around for a while?

 Giulio, maybe you can port the patch to 11.1.0?

 Regards,
 Arnout

> +endif
> +
>  LIBGEOS_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(LIBGEOS_CXXFLAGS)"
>  
>  $(eval $(cmake-package))
>
Giulio Benetti Aug. 17, 2021, 8:56 p.m. UTC | #3
Hi Arnout,

On 8/17/21 10:34 PM, Arnout Vandecappelle wrote:
> 
> 
> On 27/07/2021 00:34, Giulio Benetti wrote:
>> When building libgeos for or1k -mcmodel=large is needed to link, so let's
>> add that gcc option in case we're building for or1k.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>> ---
>>   package/libgeos/libgeos.mk | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/package/libgeos/libgeos.mk b/package/libgeos/libgeos.mk
>> index a355732af8..43211d0434 100644
>> --- a/package/libgeos/libgeos.mk
>> +++ b/package/libgeos/libgeos.mk
>> @@ -23,6 +23,10 @@ ifeq ($(BR2_arm)$(BR2_armeb),y)
>>   LIBGEOS_CONF_OPTS += -DDISABLE_GEOS_INLINE=ON
>>   endif
>>   
>> +ifeq ($(BR2_or1k),y)
>> +LIBGEOS_CXXFLAGS += -mcmodel=large
> 
>   According to git, -mcmodel was added to GCC after the 11.2.0 release. Are you
> sure that this works?
> 
>   Oooh, we have a patch that adds it. But that patch is missing for 11.1.0 for
> some reason. Romain, you apparently didn't carry any of our patches when you
> added GCC 11 in 2ab88dac293aed5adf2504e8b182f9d8a306b935. The commit message
> doesn't say anything. Do you remember anything about the patches? The patch was
> added to master on May 21 and you only submitted the GCC bump on June 12, but
> perhaps you already had it lying around for a while?
> 
>   Giulio, maybe you can port the patch to 11.1.0?

You've read into my mind :-), I've sent it yesterday:
https://patchwork.ozlabs.org/project/buildroot/list/?series=258108

and that is valid for both libgeos and profobuf.

Best regards!
Arnout Vandecappelle Aug. 17, 2021, 8:59 p.m. UTC | #4
On 17/08/2021 22:34, Arnout Vandecappelle wrote:
> 
> 
> On 27/07/2021 00:34, Giulio Benetti wrote:
>> When building libgeos for or1k -mcmodel=large is needed to link, so let's
>> add that gcc option in case we're building for or1k.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>> ---
>>  package/libgeos/libgeos.mk | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/package/libgeos/libgeos.mk b/package/libgeos/libgeos.mk
>> index a355732af8..43211d0434 100644
>> --- a/package/libgeos/libgeos.mk
>> +++ b/package/libgeos/libgeos.mk
>> @@ -23,6 +23,10 @@ ifeq ($(BR2_arm)$(BR2_armeb),y)
>>  LIBGEOS_CONF_OPTS += -DDISABLE_GEOS_INLINE=ON
>>  endif
>>  
>> +ifeq ($(BR2_or1k),y)
>> +LIBGEOS_CXXFLAGS += -mcmodel=large
> 
>  According to git, -mcmodel was added to GCC after the 11.2.0 release. Are you
> sure that this works?
> 
>  Oooh, we have a patch that adds it. But that patch is missing for 11.1.0 for
> some reason. Romain, you apparently didn't carry any of our patches when you
> added GCC 11 in 2ab88dac293aed5adf2504e8b182f9d8a306b935. The commit message
> doesn't say anything. Do you remember anything about the patches? The patch was
> added to master on May 21 and you only submitted the GCC bump on June 12, but
> perhaps you already had it lying around for a while?
> 
>  Giulio, maybe you can port the patch to 11.1.0?

 ... and now I see you already did that in [1]. Thanks!

 Regards,
 Arnout

[1]
https://patchwork.ozlabs.org/project/buildroot/patch/20210815180359.1575824-1-giulio.benetti@benettiengineering.com/


> 
>  Regards,
>  Arnout
> 
>> +endif
>> +
>>  LIBGEOS_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(LIBGEOS_CXXFLAGS)"
>>  
>>  $(eval $(cmake-package))
>>
Arnout Vandecappelle Aug. 17, 2021, 9:04 p.m. UTC | #5
On 27/07/2021 00:34, Giulio Benetti wrote:
> When building libgeos for or1k -mcmodel=large is needed to link, so let's
> add that gcc option in case we're building for or1k.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

 Applied to master, thanks.

 I've added a comment to the commit message to explain that this option is from
a Buildroot patch.

 I've also applied the second patch with the same comment.

 Regards,
 Arnout

> ---
>  package/libgeos/libgeos.mk | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/libgeos/libgeos.mk b/package/libgeos/libgeos.mk
> index a355732af8..43211d0434 100644
> --- a/package/libgeos/libgeos.mk
> +++ b/package/libgeos/libgeos.mk
> @@ -23,6 +23,10 @@ ifeq ($(BR2_arm)$(BR2_armeb),y)
>  LIBGEOS_CONF_OPTS += -DDISABLE_GEOS_INLINE=ON
>  endif
>  
> +ifeq ($(BR2_or1k),y)
> +LIBGEOS_CXXFLAGS += -mcmodel=large
> +endif
> +
>  LIBGEOS_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(LIBGEOS_CXXFLAGS)"
>  
>  $(eval $(cmake-package))
>
Romain Naour Aug. 18, 2021, 8:01 p.m. UTC | #6
Hello Arnout,

Le 17/08/2021 à 22:34, Arnout Vandecappelle a écrit :
> 
> 
> On 27/07/2021 00:34, Giulio Benetti wrote:
>> When building libgeos for or1k -mcmodel=large is needed to link, so let's
>> add that gcc option in case we're building for or1k.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>> ---
>>  package/libgeos/libgeos.mk | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/package/libgeos/libgeos.mk b/package/libgeos/libgeos.mk
>> index a355732af8..43211d0434 100644
>> --- a/package/libgeos/libgeos.mk
>> +++ b/package/libgeos/libgeos.mk
>> @@ -23,6 +23,10 @@ ifeq ($(BR2_arm)$(BR2_armeb),y)
>>  LIBGEOS_CONF_OPTS += -DDISABLE_GEOS_INLINE=ON
>>  endif
>>  
>> +ifeq ($(BR2_or1k),y)
>> +LIBGEOS_CXXFLAGS += -mcmodel=large
> 
>  According to git, -mcmodel was added to GCC after the 11.2.0 release. Are you
> sure that this works?
> 
>  Oooh, we have a patch that adds it. But that patch is missing for 11.1.0 for
> some reason. Romain, you apparently didn't carry any of our patches when you
> added GCC 11 in 2ab88dac293aed5adf2504e8b182f9d8a306b935. The commit message
> doesn't say anything. Do you remember anything about the patches? The patch was
> added to master on May 21 and you only submitted the GCC bump on June 12, but
> perhaps you already had it lying around for a while?

Indeed, I missed this patch.

> 
>  Giulio, maybe you can port the patch to 11.1.0?

Yes please.

Best regards,
Romain


> 
>  Regards,
>  Arnout
> 
>> +endif
>> +
>>  LIBGEOS_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(LIBGEOS_CXXFLAGS)"
>>  
>>  $(eval $(cmake-package))
>>
Giulio Benetti Aug. 18, 2021, 8:54 p.m. UTC | #7
Hi Romain,

On 8/18/21 10:01 PM, Romain Naour wrote:
> Hello Arnout,
> 
> Le 17/08/2021 à 22:34, Arnout Vandecappelle a écrit :
>>
>>
>> On 27/07/2021 00:34, Giulio Benetti wrote:
>>> When building libgeos for or1k -mcmodel=large is needed to link, so let's
>>> add that gcc option in case we're building for or1k.
>>>
>>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>>> ---
>>>   package/libgeos/libgeos.mk | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/package/libgeos/libgeos.mk b/package/libgeos/libgeos.mk
>>> index a355732af8..43211d0434 100644
>>> --- a/package/libgeos/libgeos.mk
>>> +++ b/package/libgeos/libgeos.mk
>>> @@ -23,6 +23,10 @@ ifeq ($(BR2_arm)$(BR2_armeb),y)
>>>   LIBGEOS_CONF_OPTS += -DDISABLE_GEOS_INLINE=ON
>>>   endif
>>>   
>>> +ifeq ($(BR2_or1k),y)
>>> +LIBGEOS_CXXFLAGS += -mcmodel=large
>>
>>   According to git, -mcmodel was added to GCC after the 11.2.0 release. Are you
>> sure that this works?
>>
>>   Oooh, we have a patch that adds it. But that patch is missing for 11.1.0 for
>> some reason. Romain, you apparently didn't carry any of our patches when you
>> added GCC 11 in 2ab88dac293aed5adf2504e8b182f9d8a306b935. The commit message
>> doesn't say anything. Do you remember anything about the patches? The patch was
>> added to master on May 21 and you only submitted the GCC bump on June 12, but
>> perhaps you already had it lying around for a while?
> 
> Indeed, I missed this patch.
> 
>>
>>   Giulio, maybe you can port the patch to 11.1.0?
> 
> Yes please.

Already sent and applied here :-):
https://git.buildroot.net/buildroot/commit/?id=137225cef423ccbebc31d0c562bf3f37192739ac

I'm going to add myself to toolchain as maintainer too, so I keep up to 
date on toolchain movement and also I've added a bunch of binutils/gcc
bugs. So it's time for it :-)

Best regards
diff mbox series

Patch

diff --git a/package/libgeos/libgeos.mk b/package/libgeos/libgeos.mk
index a355732af8..43211d0434 100644
--- a/package/libgeos/libgeos.mk
+++ b/package/libgeos/libgeos.mk
@@ -23,6 +23,10 @@  ifeq ($(BR2_arm)$(BR2_armeb),y)
 LIBGEOS_CONF_OPTS += -DDISABLE_GEOS_INLINE=ON
 endif
 
+ifeq ($(BR2_or1k),y)
+LIBGEOS_CXXFLAGS += -mcmodel=large
+endif
+
 LIBGEOS_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(LIBGEOS_CXXFLAGS)"
 
 $(eval $(cmake-package))