Patchwork UBUNTU: Enable perf to be more helpful when perf_<version> does not exist.

login
register
mail settings
Submitter Lee Jones
Date June 1, 2010, 10:47 a.m.
Message ID <4C04E538.5070007@canonical.com>
Download mbox | patch
Permalink /patch/54166/
State Accepted
Delegated to: Leann Ogasawara
Headers show

Comments

Lee Jones - June 1, 2010, 10:47 a.m.
> The pull request is correct. For most patches (except the truly
> gargantuan ones) I attach or inline so that patchwork detects it as a
> patch which makes the release maintainers job easier.

commit 8a2775c937d9349e8c7ccb915a3d2b134c13f09b
Author: Lee Jones <lee.jones@canonical.com>
Date:   Tue May 25 11:49:32 2010 +0100

    UBUNTU: Enable perf to be more helpful when perf_<version> does not
exist.
   
    http://bugs.launchpad.net/bugs/570500
   
    Signed-off-by: Lee Jones <lee.jones@canonical.com>



Hopefully this is better.

Kind regards,
Lee
Stefan Bader - June 1, 2010, 1:43 p.m.
On 06/01/2010 12:47 PM, Lee Jones wrote:
> 
>> The pull request is correct. For most patches (except the truly
>> gargantuan ones) I attach or inline so that patchwork detects it as a
>> patch which makes the release maintainers job easier.
> 
> commit 8a2775c937d9349e8c7ccb915a3d2b134c13f09b
> Author: Lee Jones <lee.jones@canonical.com>
> Date:   Tue May 25 11:49:32 2010 +0100
> 
>     UBUNTU: Enable perf to be more helpful when perf_<version> does not
> exist.
>    
>     http://bugs.launchpad.net/bugs/570500
>    
>     Signed-off-by: Lee Jones <lee.jones@canonical.com>
> 
> 
> diff --git a/debian/tools/perf b/debian/tools/perf
> index 79253d2..ab35fab 100644 (file)
> --- a/debian/tools/perf
> +++ b/debian/tools/perf
> @@ -1,7 +1,16 @@
>  #!/bin/bash
> -version=`uname -r`
> -flavour=${version#*-}
> -flavour=${flavour#*-}
> -version=${version%-$flavour}

I think those are the merits of that oh so understandable and clear shell
syntax. Clearly ${...#word} removes the shortest prefix while ${...##word}
removes the longest match. :-)
But the comments help imo.

-Stefan

> +full_version=`uname -r`
>  
> -exec "perf_$version" "$@"
> +# Removing flavour from version i.e. generic or server.
> +flavour_abi=${full_version#*-}
> +flavour=${flavour_abi#*-}
> +version=${full_version%-$flavour}
> +perf="perf_$version"
> +
> +if ! which "$perf" > /dev/null; then
> +    echo "$perf not found" >&2
> +    echo "You may need to install linux-tools-$version" >&2
> +    exit 2
> +fi
> +
> +exec "$perf" "$@"
> 
> Hopefully this is better.
> 
> Kind regards,
> Lee
>
Lee Jones - June 1, 2010, 3:41 p.m.
On 01/06/10 14:43, Stefan Bader wrote:
> On 06/01/2010 12:47 PM, Lee Jones wrote:
>   
>>     
>>> The pull request is correct. For most patches (except the truly
>>> gargantuan ones) I attach or inline so that patchwork detects it as a
>>> patch which makes the release maintainers job easier.
>>>       
>> commit 8a2775c937d9349e8c7ccb915a3d2b134c13f09b
>> Author: Lee Jones <lee.jones@canonical.com>
>> Date:   Tue May 25 11:49:32 2010 +0100
>>
>>     UBUNTU: Enable perf to be more helpful when perf_<version> does not
>> exist.
>>    
>>     http://bugs.launchpad.net/bugs/570500
>>    
>>     Signed-off-by: Lee Jones <lee.jones@canonical.com>
>>
>>
>> diff --git a/debian/tools/perf b/debian/tools/perf
>> index 79253d2..ab35fab 100644 (file)
>> --- a/debian/tools/perf
>> +++ b/debian/tools/perf
>> @@ -1,7 +1,16 @@
>>  #!/bin/bash
>> -version=`uname -r`
>> -flavour=${version#*-}
>> -flavour=${flavour#*-}
>> -version=${version%-$flavour}
>>     
> I think those are the merits of that oh so understandable and clear shell
> syntax. Clearly ${...#word} removes the shortest prefix while ${...##word}
> removes the longest match. :-)
> But the comments help imo.
>
> -Stefan
>
>   
>> +full_version=`uname -r`
>>  
>> -exec "perf_$version" "$@"
>> +# Removing flavour from version i.e. generic or server.
>> +flavour_abi=${full_version#*-}
>> +flavour=${flavour_abi#*-}
>> +version=${full_version%-$flavour}
>> +perf="perf_$version"
>> +
>> +if ! which "$perf" > /dev/null; then
>> +    echo "$perf not found" >&2
>> +    echo "You may need to install linux-tools-$version" >&2
>> +    exit 2
>> +fi
>> +
>> +exec "$perf" "$@"
>>
>> Hopefully this is better.
>>
>> Kind regards,
>> Lee
>>
>>     

Yes, it's not the most intuitive language.
Leann Ogasawara - June 1, 2010, 4:52 p.m.
On Tue, 2010-06-01 at 11:47 +0100, Lee Jones wrote:
> > The pull request is correct. For most patches (except the truly
> > gargantuan ones) I attach or inline so that patchwork detects it as a
> > patch which makes the release maintainers job easier.

Hi Lee,

Based on the nominations in the bug report, I assume this patch is
intended for both Maverick and Lucid.

And just a small tweak noted below. . .

> commit 8a2775c937d9349e8c7ccb915a3d2b134c13f09b
> Author: Lee Jones <lee.jones@canonical.com>
> Date:   Tue May 25 11:49:32 2010 +0100
> 
>     UBUNTU: Enable perf to be more helpful when perf_<version> does not
> exist.
>    
>     http://bugs.launchpad.net/bugs/570500

This should be:

BugLink: http://bugs.launchpad.net/bugs/570500

The launchpad janitor parses for the above syntax and will auto close
the bug when the fix is uploaded.

Thanks,
Leann

>    
>     Signed-off-by: Lee Jones <lee.jones@canonical.com>
> 
> 
> diff --git a/debian/tools/perf b/debian/tools/perf
> index 79253d2..ab35fab 100644 (file)
> --- a/debian/tools/perf
> +++ b/debian/tools/perf
> @@ -1,7 +1,16 @@
>  #!/bin/bash
> -version=`uname -r`
> -flavour=${version#*-}
> -flavour=${flavour#*-}
> -version=${version%-$flavour}
> +full_version=`uname -r`
>  
> -exec "perf_$version" "$@"
> +# Removing flavour from version i.e. generic or server.
> +flavour_abi=${full_version#*-}
> +flavour=${flavour_abi#*-}
> +version=${full_version%-$flavour}
> +perf="perf_$version"
> +
> +if ! which "$perf" > /dev/null; then
> +    echo "$perf not found" >&2
> +    echo "You may need to install linux-tools-$version" >&2
> +    exit 2
> +fi
> +
> +exec "$perf" "$@"
> 
> Hopefully this is better.
> 
> Kind regards,
> Lee
>

Patch

diff --git a/debian/tools/perf b/debian/tools/perf
index 79253d2..ab35fab 100644 (file)
--- a/debian/tools/perf
+++ b/debian/tools/perf
@@ -1,7 +1,16 @@ 
 #!/bin/bash
-version=`uname -r`
-flavour=${version#*-}
-flavour=${flavour#*-}
-version=${version%-$flavour}
+full_version=`uname -r`
 
-exec "perf_$version" "$@"
+# Removing flavour from version i.e. generic or server.
+flavour_abi=${full_version#*-}
+flavour=${flavour_abi#*-}
+version=${full_version%-$flavour}
+perf="perf_$version"
+
+if ! which "$perf" > /dev/null; then
+    echo "$perf not found" >&2
+    echo "You may need to install linux-tools-$version" >&2
+    exit 2
+fi
+
+exec "$perf" "$@"