[PATCH] configure.ac: don't mangle CFLAGS

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] configure.ac: don't mangle CFLAGS

Fabrice Fontaine
Ensure that the sed expression to strip debugging options from CFLAGS
doesn't mangle flags like -mfloat-gprs=double, breaking the build.

Patch by: Peter Korsgaard and Vicente Olivert Riera
(See https://git.buildroot.net/buildroot/tree/package/subversion/0001-dont-mangle-cflags.patch?h=2020.08)
---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index f1b2a9929e..dd0680cfb2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1110,8 +1110,8 @@ if test "$enable_debugging" = "yes" ; then
   CXXFLAGS="$CXXFLAGS -DSVN_DEBUG -DAP_DEBUG"
 elif test "$enable_debugging" = "no" ; then
   AC_MSG_NOTICE([Disabling debugging])
-  CFLAGS=["`echo $CFLAGS' ' | $SED -e 's/-g[0-9] //g' -e 's/-g //g'`"]
-  CXXFLAGS=["`echo $CXXFLAGS' ' | $SED -e 's/-g[0-9] //g' -e 's/-g //g'`"]
+  CFLAGS=["`echo $CFLAGS' ' | $SED -e 's/-g[0-9]* //g'`"]
+  CXXFLAGS=["`echo $CXXFLAGS' ' | $SED -e 's/-g[0-9]* //g'`"]
   dnl Compile with NDEBUG to get rid of assertions
   CFLAGS="$CFLAGS -DNDEBUG"
   CXXFLAGS="$CXXFLAGS -DNDEBUG"
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] configure.ac: don't mangle CFLAGS

Branko Čibej
On 09.09.2020 23:34, Fabrice Fontaine wrote:
Ensure that the sed expression to strip debugging options from CFLAGS
doesn't mangle flags like -mfloat-gprs=double, breaking the build.

Patch by: Peter Korsgaard and Vicente Olivert Riera
(See https://git.buildroot.net/buildroot/tree/package/subversion/0001-dont-mangle-cflags.patch?h=2020.08)
---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index f1b2a9929e..dd0680cfb2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1110,8 +1110,8 @@ if test "$enable_debugging" = "yes" ; then
   CXXFLAGS="$CXXFLAGS -DSVN_DEBUG -DAP_DEBUG"
 elif test "$enable_debugging" = "no" ; then
   AC_MSG_NOTICE([Disabling debugging])
-  CFLAGS=["`echo $CFLAGS' ' | $SED -e 's/-g[0-9] //g' -e 's/-g //g'`"]
-  CXXFLAGS=["`echo $CXXFLAGS' ' | $SED -e 's/-g[0-9] //g' -e 's/-g //g'`"]
+  CFLAGS=["`echo $CFLAGS' ' | $SED -e 's/-g[0-9]* //g'`"]
+  CXXFLAGS=["`echo $CXXFLAGS' ' | $SED -e 's/-g[0-9]* //g'`"]
   dnl Compile with NDEBUG to get rid of assertions
   CFLAGS="$CFLAGS -DNDEBUG"
   CXXFLAGS="$CXXFLAGS -DNDEBUG"

There is no functional change in your patch, and the original expressions will not mangle '-mfloat-gprs=double' because the patterns contain a trailing space.

I'm confused.

-- Brane
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] configure.ac: don't mangle CFLAGS

Branko Čibej
On 10.09.2020 05:41, Branko Čibej wrote:
On 09.09.2020 23:34, Fabrice Fontaine wrote:
Ensure that the sed expression to strip debugging options from CFLAGS
doesn't mangle flags like -mfloat-gprs=double, breaking the build.

Patch by: Peter Korsgaard and Vicente Olivert Riera
(See https://git.buildroot.net/buildroot/tree/package/subversion/0001-dont-mangle-cflags.patch?h=2020.08)
---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index f1b2a9929e..dd0680cfb2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1110,8 +1110,8 @@ if test "$enable_debugging" = "yes" ; then
   CXXFLAGS="$CXXFLAGS -DSVN_DEBUG -DAP_DEBUG"
 elif test "$enable_debugging" = "no" ; then
   AC_MSG_NOTICE([Disabling debugging])
-  CFLAGS=["`echo $CFLAGS' ' | $SED -e 's/-g[0-9] //g' -e 's/-g //g'`"]
-  CXXFLAGS=["`echo $CXXFLAGS' ' | $SED -e 's/-g[0-9] //g' -e 's/-g //g'`"]
+  CFLAGS=["`echo $CFLAGS' ' | $SED -e 's/-g[0-9]* //g'`"]
+  CXXFLAGS=["`echo $CXXFLAGS' ' | $SED -e 's/-g[0-9]* //g'`"]
   dnl Compile with NDEBUG to get rid of assertions
   CFLAGS="$CFLAGS -DNDEBUG"
   CXXFLAGS="$CXXFLAGS -DNDEBUG"

There is no functional change in your patch, and the original expressions will not mangle '-mfloat-gprs=double' because the patterns contain a trailing space.

I'm confused.


$ echo '-mfloat-gprs=double ' | sed -e 's/-g[0-9] //g' -e 's/-g //g'
-mfloat-gprs=double


-- Brane

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] configure.ac: don't mangle CFLAGS

Fabrice Fontaine
Hello Brane,

Le jeu. 10 sept. 2020 à 05:53, Branko Čibej <[hidden email]> a écrit :

>
> On 10.09.2020 05:41, Branko Čibej wrote:
>
> On 09.09.2020 23:34, Fabrice Fontaine wrote:
>
> Ensure that the sed expression to strip debugging options from CFLAGS
> doesn't mangle flags like -mfloat-gprs=double, breaking the build.
>
> Patch by: Peter Korsgaard and Vicente Olivert Riera
> (See https://git.buildroot.net/buildroot/tree/package/subversion/0001-dont-mangle-cflags.patch?h=2020.08)
> ---
>  configure.ac | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index f1b2a9929e..dd0680cfb2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1110,8 +1110,8 @@ if test "$enable_debugging" = "yes" ; then
>    CXXFLAGS="$CXXFLAGS -DSVN_DEBUG -DAP_DEBUG"
>  elif test "$enable_debugging" = "no" ; then
>    AC_MSG_NOTICE([Disabling debugging])
> -  CFLAGS=["`echo $CFLAGS' ' | $SED -e 's/-g[0-9] //g' -e 's/-g //g'`"]
> -  CXXFLAGS=["`echo $CXXFLAGS' ' | $SED -e 's/-g[0-9] //g' -e 's/-g //g'`"]
> +  CFLAGS=["`echo $CFLAGS' ' | $SED -e 's/-g[0-9]* //g'`"]
> +  CXXFLAGS=["`echo $CXXFLAGS' ' | $SED -e 's/-g[0-9]* //g'`"]
>    dnl Compile with NDEBUG to get rid of assertions
>    CFLAGS="$CFLAGS -DNDEBUG"
>    CXXFLAGS="$CXXFLAGS -DNDEBUG"
>
>
> There is no functional change in your patch, and the original expressions will not mangle '-mfloat-gprs=double' because the patterns contain a trailing space.
Thanks for your feedback, indeed you're right.
Peter made this patch to fix subversion 1.7.18. At this time, there
was no space: https://git.buildroot.net/buildroot/commit/package/subversion/0001-dont-mangle-cflags.patch?h=2020.08&id=395c88051efb4b84f752be4eea1b34b13c80a1dc
Vicente updated this patch when bumping the version to 1.9.2. However,
at this time, the sed expression was already corrected.
In fact, this issue was fixed in version 1.8.0 with
https://github.com/apache/subversion/commit/f071ec0c26cdf47e89fa90b31d2233ee1a2b00c2.
So, I'll drop this patch from buildroot.

>
> I'm confused.
>
>
>
> $ echo '-mfloat-gprs=double ' | sed -e 's/-g[0-9] //g' -e 's/-g //g'
> -mfloat-gprs=double
>
>
>
> -- Brane
>
>
Best Regards,

Fabrice
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] configure.ac: don't mangle CFLAGS

Branko Čibej
On 10.09.2020 07:36, Fabrice Fontaine wrote:
Hello Brane,

Le jeu. 10 sept. 2020 à 05:53, Branko Čibej [hidden email] a écrit :
On 10.09.2020 05:41, Branko Čibej wrote:

On 09.09.2020 23:34, Fabrice Fontaine wrote:

Ensure that the sed expression to strip debugging options from CFLAGS
doesn't mangle flags like -mfloat-gprs=double, breaking the build.

Patch by: Peter Korsgaard and Vicente Olivert Riera
(See https://git.buildroot.net/buildroot/tree/package/subversion/0001-dont-mangle-cflags.patch?h=2020.08)
---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index f1b2a9929e..dd0680cfb2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1110,8 +1110,8 @@ if test "$enable_debugging" = "yes" ; then
   CXXFLAGS="$CXXFLAGS -DSVN_DEBUG -DAP_DEBUG"
 elif test "$enable_debugging" = "no" ; then
   AC_MSG_NOTICE([Disabling debugging])
-  CFLAGS=["`echo $CFLAGS' ' | $SED -e 's/-g[0-9] //g' -e 's/-g //g'`"]
-  CXXFLAGS=["`echo $CXXFLAGS' ' | $SED -e 's/-g[0-9] //g' -e 's/-g //g'`"]
+  CFLAGS=["`echo $CFLAGS' ' | $SED -e 's/-g[0-9]* //g'`"]
+  CXXFLAGS=["`echo $CXXFLAGS' ' | $SED -e 's/-g[0-9]* //g'`"]
   dnl Compile with NDEBUG to get rid of assertions
   CFLAGS="$CFLAGS -DNDEBUG"
   CXXFLAGS="$CXXFLAGS -DNDEBUG"


There is no functional change in your patch, and the original expressions will not mangle '-mfloat-gprs=double' because the patterns contain a trailing space.
Thanks for your feedback, indeed you're right.
Peter made this patch to fix subversion 1.7.18. At this time, there
was no space: https://git.buildroot.net/buildroot/commit/package/subversion/0001-dont-mangle-cflags.patch?h=2020.08&id=395c88051efb4b84f752be4eea1b34b13c80a1dc
Vicente updated this patch when bumping the version to 1.9.2. However,
at this time, the sed expression was already corrected.
In fact, this issue was fixed in version 1.8.0 with
https://github.com/apache/subversion/commit/f071ec0c26cdf47e89fa90b31d2233ee1a2b00c2.


Yes, I remember fixing that years ago. :)


So, I'll drop this patch from buildroot.

Cool.

-- Brane