threads / patch / 22314

patch, 4 partsengine.pl: Fix a recent breakage of the buildsystem generator

Subject: [PATCH 1/4] engine.pl: Fix a recent breakage of the buildsystem generator

## tl;dr

5 messages between Jan 20, 2010 and Jan 22, 2010. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Ramsay Jones· Jan 20, 2010, 19:23 UTC · lore

Commit ade2ca0c (Do not try to remove directories when removing old links, 27-10-2009) added an expression to a 'test' using an '-o' or connective. This resulted in the buildsystem generator mistaking the conditional 'rm' for a linker command. In order to fix the breakage, we filter the out the conditional 'rm' commands before attempting to identify other commands.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 contrib/buildsystems/engine.pl |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Show changes to contrib/buildsystems/engine.pl +5 −0
diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
index d506717..245af73 100644
--- a/contrib/buildsystems/engine.pl
+++ b/contrib/buildsystems/engine.pl
@@ -135,6 +135,11 @@ sub parseMakeOutput
             }
         } while($ate_next);
 
+        if ($text =~ /^test / && $text =~ /|| rm -f /) {
+            # commands removing executables, if they exist
+            next;
+        }
+
         if($text =~ / -c /) {
             # compilation
             handleCompileLine($text, $line);

-- 
1.6.6
Pete Harlan· Jan 20, 2010, 20:57 UTC · re: Ramsay Jones · lore

Re: [PATCH 1/4] engine.pl: Fix a recent breakage of the buildsystem generator

On 01/20/2010 11:23 AM, Ramsay Jones wrote:
Show 9 quoted lines
> diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
> index d506717..245af73 100644
> --- a/contrib/buildsystems/engine.pl
> +++ b/contrib/buildsystems/engine.pl
> @@ -135,6 +135,11 @@ sub parseMakeOutput
>              }
>          } while($ate_next);
>  
> +        if ($text =~ /^test / && $text =~ /|| rm -f /) {
That test on the right needs to escape its pipes or it will always match.
--Pete
Show 7 quoted lines
> +            # commands removing executables, if they exist
> +            next;
> +        }
> +
>          if($text =~ / -c /) {
>              # compilation
>              handleCompileLine($text, $line);
Ramsay Jones· Jan 22, 2010, 18:58 UTC · re: Pete Harlan · lore

Re: [PATCH 1/4] engine.pl: Fix a recent breakage of the buildsystem generator

Pete Harlan wrote:
Show 12 quoted lines
> On 01/20/2010 11:23 AM, Ramsay Jones wrote:
>> diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
>> index d506717..245af73 100644
>> --- a/contrib/buildsystems/engine.pl
>> +++ b/contrib/buildsystems/engine.pl
>> @@ -135,6 +135,11 @@ sub parseMakeOutput
>>              }
>>          } while($ate_next);
>>  
>> +        if ($text =~ /^test / && $text =~ /|| rm -f /) {
> 
> That test on the right needs to escape its pipes or it will always match.

Heh, well spotted! As you may have already noticed, I copy/pasted the code from the comment further down in this function, but didn't take enough care in doing so... Oops! ;-)

The obvious solution, also suggested later by Sebastion, is to simply remove the second regex from the test since it does not alter the outcome (and still fixes the problem)...

ATB, Ramsay Jones

Sebastian Schuberth· Jan 20, 2010, 22:54 UTC · re: Ramsay Jones · lore

Re: [PATCH 1/4] engine.pl: Fix a recent breakage of the buildsystem generator

On 20.01.2010 20:23, Ramsay Jones wrote:
 > '-o' or connective. This resulted in the buildsystem generator
 > mistaking the conditional 'rm' for a linker command. In order
Thanks for spotting the cause of this! Some comments:
1. How about deleting lines 183-185 in same run? That commented out 
code, too, is missing the escapes for the pipes that Pete mentioned anyway.

-# } elsif ($text =~ /^test / && $text =~ /|| rm -f /) { -# # commands removing executables, if they exist -#

2. Couldn't we reduce the test to just
+        if ($text =~ /^test /) {
+            # options to test may be mistaken for linker options
+            next;
+        }
+
3. If the above won't do for some reason, I'd still prefer something like
+        if ($text =~ /^test / && $text =~ / -o /) {
+            # options to test may be mistaken for linker options
+            next;
+        }
+
as it makes more clear what's the problem in such lines.
-- 
Sebastian Schuberth
Ramsay Jones· Jan 22, 2010, 19:40 UTC · re: Sebastian Schuberth · lore

Re: [PATCH 1/4] engine.pl: Fix a recent breakage of the buildsystem generator

Sebastian Schuberth wrote:
Show 13 quoted lines
> On 20.01.2010 20:23, Ramsay Jones wrote:
> 
>  > '-o' or connective. This resulted in the buildsystem generator
>  > mistaking the conditional 'rm' for a linker command. In order
> 
> Thanks for spotting the cause of this! Some comments:
> 
> 1. How about deleting lines 183-185 in same run? That commented out 
> code, too, is missing the escapes for the pipes that Pete mentioned anyway.
> 
> -#        } elsif ($text =~ /^test / && $text =~ /|| rm -f /) {
> -#            # commands removing executables, if they exist
> -#
OK, will do.
Show 8 quoted lines
> 
> 2. Couldn't we reduce the test to just
> 
> +        if ($text =~ /^test /) {
> +            # options to test may be mistaken for linker options
> +            next;
> +        }
> +
Yes, good idea.  I will send a new version of the patch soon.
Thanks!

ATB, Ramsay Jones

← back to recent threads