{"thread":{"id":"22314","subject":"[PATCH 1/4] engine.pl: Fix a recent breakage of the buildsystem generator","startedAt":"2010-01-20T19:23:36Z","lastAt":"2010-01-22T19:40:42Z","messageCount":5,"participants":["Ramsay Jones","Pete Harlan","Sebastian Schuberth"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"132217","messageId":"4B575838.2010504@ramsay1.demon.co.uk","threadId":"22314","inReplyTo":null,"subject":"[PATCH 1/4] engine.pl: Fix a recent breakage of the buildsystem generator","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2010-01-20T19:23:36Z","receivedAt":"2010-01-20T19:23:36Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nCommit ade2ca0c (Do not try to remove directories when removing\nold links, 27-10-2009) added an expression to a 'test' using an\n'-o' or connective. This resulted in the buildsystem generator\nmistaking the conditional 'rm' for a linker command. In order\nto fix the breakage, we filter the out the conditional 'rm'\ncommands before attempting to identify other commands.\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n contrib/buildsystems/engine.pl |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl\nindex d506717..245af73 100644\n--- a/contrib/buildsystems/engine.pl\n+++ b/contrib/buildsystems/engine.pl\n@@ -135,6 +135,11 @@ sub parseMakeOutput\n             }\n         } while($ate_next);\n \n+        if ($text =~ /^test / && $text =~ /|| rm -f /) {\n+            # commands removing executables, if they exist\n+            next;\n+        }\n+\n         if($text =~ / -c /) {\n             # compilation\n             handleCompileLine($text, $line);\n-- \n1.6.6\n"},{"id":"132221","messageId":"4B576E38.5080604@pcharlan.com","threadId":"22314","inReplyTo":"4B575838.2010504@ramsay1.demon.co.uk","subject":"Re: [PATCH 1/4] engine.pl: Fix a recent breakage of the buildsystem generator","fromName":"Pete Harlan","fromEmail":"pgit@pcharlan.com","sentAt":"2010-01-20T20:57:28Z","receivedAt":"2010-01-20T20:57:28Z","isPatch":true,"sender":{"key":"pgit@pcharlan.com","avatar":null},"body":"On 01/20/2010 11:23 AM, Ramsay Jones wrote:\n> diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl\n> index d506717..245af73 100644\n> --- a/contrib/buildsystems/engine.pl\n> +++ b/contrib/buildsystems/engine.pl\n> @@ -135,6 +135,11 @@ sub parseMakeOutput\n>              }\n>          } while($ate_next);\n>  \n> +        if ($text =~ /^test / && $text =~ /|| rm -f /) {\n\nThat test on the right needs to escape its pipes or it will always match.\n\n--Pete\n\n> +            # commands removing executables, if they exist\n> +            next;\n> +        }\n> +\n>          if($text =~ / -c /) {\n>              # compilation\n>              handleCompileLine($text, $line);\n"},{"id":"132242","messageId":"4B5789AD.2080807@gmail.com","threadId":"22314","inReplyTo":"4B575838.2010504@ramsay1.demon.co.uk","subject":"Re: [PATCH 1/4] engine.pl: Fix a recent breakage of the buildsystem generator","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2010-01-20T22:54:37Z","receivedAt":"2010-01-20T22:54:37Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On 20.01.2010 20:23, Ramsay Jones wrote:\n\n > '-o' or connective. This resulted in the buildsystem generator\n > mistaking the conditional 'rm' for a linker command. In order\n\nThanks for spotting the cause of this! Some comments:\n\n1. How about deleting lines 183-185 in same run? That commented out \ncode, too, is missing the escapes for the pipes that Pete mentioned anyway.\n\n-#        } elsif ($text =~ /^test / && $text =~ /|| rm -f /) {\n-#            # commands removing executables, if they exist\n-#\n\n\n2. Couldn't we reduce the test to just\n\n+        if ($text =~ /^test /) {\n+            # options to test may be mistaken for linker options\n+            next;\n+        }\n+\n\n3. If the above won't do for some reason, I'd still prefer something like\n\n+        if ($text =~ /^test / && $text =~ / -o /) {\n+            # options to test may be mistaken for linker options\n+            next;\n+        }\n+\n\nas it makes more clear what's the problem in such lines.\n\n-- \nSebastian Schuberth\n"},{"id":"132418","messageId":"4B59F549.8070308@ramsay1.demon.co.uk","threadId":"22314","inReplyTo":"4B576E38.5080604@pcharlan.com","subject":"Re: [PATCH 1/4] engine.pl: Fix a recent breakage of the buildsystem generator","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2010-01-22T18:58:17Z","receivedAt":"2010-01-22T18:58:17Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Pete Harlan wrote:\n> On 01/20/2010 11:23 AM, Ramsay Jones wrote:\n>> diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl\n>> index d506717..245af73 100644\n>> --- a/contrib/buildsystems/engine.pl\n>> +++ b/contrib/buildsystems/engine.pl\n>> @@ -135,6 +135,11 @@ sub parseMakeOutput\n>>              }\n>>          } while($ate_next);\n>>  \n>> +        if ($text =~ /^test / && $text =~ /|| rm -f /) {\n> \n> That test on the right needs to escape its pipes or it will always match.\n\nHeh, well spotted!\nAs you may have already noticed, I copy/pasted the code from the comment\nfurther down in this function, but didn't take enough care in doing so...\nOops! ;-)\n\nThe obvious solution, also suggested later by Sebastion, is to simply\nremove the second regex from the test since it does not alter the\noutcome (and still fixes the problem)...\n\nATB,\nRamsay Jones\n"},{"id":"132422","messageId":"4B59FF3A.5080502@ramsay1.demon.co.uk","threadId":"22314","inReplyTo":"4B5789AD.2080807@gmail.com","subject":"Re: [PATCH 1/4] engine.pl: Fix a recent breakage of the buildsystem generator","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2010-01-22T19:40:42Z","receivedAt":"2010-01-22T19:40:42Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Sebastian Schuberth wrote:\n> On 20.01.2010 20:23, Ramsay Jones wrote:\n> \n>  > '-o' or connective. This resulted in the buildsystem generator\n>  > mistaking the conditional 'rm' for a linker command. In order\n> \n> Thanks for spotting the cause of this! Some comments:\n> \n> 1. How about deleting lines 183-185 in same run? That commented out \n> code, too, is missing the escapes for the pipes that Pete mentioned anyway.\n> \n> -#        } elsif ($text =~ /^test / && $text =~ /|| rm -f /) {\n> -#            # commands removing executables, if they exist\n> -#\n\nOK, will do.\n\n> \n> 2. Couldn't we reduce the test to just\n> \n> +        if ($text =~ /^test /) {\n> +            # options to test may be mistaken for linker options\n> +            next;\n> +        }\n> +\n\nYes, good idea.  I will send a new version of the patch soon.\n\nThanks!\n\nATB,\nRamsay Jones\n"}]}