Re: [U-Boot] [[RFC Patch] 2/2] tools: buildman: Add compiler wrapper

[Simon Glass]

Hi York,

On 29 September 2016 at 15:12, York Sun  wrote:
>
> Now we can use compiler wrapper such as ccache or distcc for buildman.
>
> Signed-off-by: York Sun 
> CC: Simon Glass 
> ---
> This is not perfect and may need some improvement. Something I didn't figure
> out is how to drop the name under [toolchain-wrapper]. I intended to have this
> format but bsettings.GetItems doesn't like it.
>
> [toolchain-wrapper]
> ccache
>
> It doesn't deal with more than one line for this setting. For now, the last
> setting is taken.
>
>  tools/buildman/README   |  9 +
>  tools/buildman/toolchain.py | 17 +++--
>  2 files changed, 24 insertions(+), 2 deletions(-)

Looks good, a few nits below.

Acked-by: Simon Glass 

>
> diff --git a/tools/buildman/README b/tools/buildman/README
> index 8c5f861..514bebc 100644
> --- a/tools/buildman/README
> +++ b/tools/buildman/README
> @@ -211,6 +211,15 @@ arm: arm-none-eabi-
>
>  and buildman will find arm-none-eabi-gcc in /usr/bin if you have it 
> installed.
>
> +[toolchain-wrapper]
> +wrapper: ccache

I think it is good to have this in a separate section, as you have
done. If we put it in the '[toolchain]' section, the name 'wrapper'
would be special, and mean something, whereas the other names would
not.

> +
> +This tells buildman to use a compiler wrapper in front of CROSS_COMPILE. In
> +this example, ccache. It doesn't affect the toolchain scan. The wrapper is
> +added when CROSS_COMPILE environtal variable is set. The name in this
> +section is ignored. If more than one line is provided, only the last one
> +is taken.
> +
>  3. Make sure you have the require Python pre-requisites
>
>  Buildman uses multiprocessing, Queue, shutil, StringIO, ConfigParser and
> diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py
> index 41e4e4c..01cdb03 100644
> --- a/tools/buildman/toolchain.py
> +++ b/tools/buildman/toolchain.py
> @@ -127,6 +127,15 @@ class Toolchain:
>  return PRIORITY_CALC + prio
>  return PRIORITY_CALC + prio
>
> +def GetWrapper(self, show_warning=True):
> +"""Get toolchain wrapper from the setting file.
> +"""
> +   value = ''
> +   for name, value in bsettings.GetItems('toolchain-wrapper'):
> +if not value:
> +print "Warning: Wrapper not found"
> +return value

Can you make it add the ' ' here?

> +
>  def MakeEnvironment(self, full_path):
>  """Returns an environment for using the toolchain.
>
> @@ -138,10 +147,14 @@ class Toolchain:
>  PATH
>  """
>  env = dict(os.environ)
> +wrapper = self.GetWrapper()
> +if wrapper:
> +wrapper = wrapper + ' '

Instead of here

> +
>  if full_path:
> -env['CROSS_COMPILE'] = os.path.join(self.path, self.cross)
> +env['CROSS_COMPILE'] = wrapper + os.path.join(self.path, 
> self.cross)
>  else:
> -env['CROSS_COMPILE'] = self.cross
> +env['CROSS_COMPILE'] = wrapper + self.cross
>  env['PATH'] = self.path + ':' + env['PATH']
>
>  return env
> --
> 2.7.4
>
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [[RFC Patch] 2/2] tools: buildman: Add compiler wrapper

[York Sun]

Now we can use compiler wrapper such as ccache or distcc for buildman.

Signed-off-by: York Sun 
CC: Simon Glass 
---
This is not perfect and may need some improvement. Something I didn't figure
out is how to drop the name under [toolchain-wrapper]. I intended to have this
format but bsettings.GetItems doesn't like it.

[toolchain-wrapper]
ccache

It doesn't deal with more than one line for this setting. For now, the last
setting is taken.

 tools/buildman/README   |  9 +
 tools/buildman/toolchain.py | 17 +++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/tools/buildman/README b/tools/buildman/README
index 8c5f861..514bebc 100644
--- a/tools/buildman/README
+++ b/tools/buildman/README
@@ -211,6 +211,15 @@ arm: arm-none-eabi-
 
 and buildman will find arm-none-eabi-gcc in /usr/bin if you have it installed.
 
+[toolchain-wrapper]
+wrapper: ccache
+
+This tells buildman to use a compiler wrapper in front of CROSS_COMPILE. In
+this example, ccache. It doesn't affect the toolchain scan. The wrapper is
+added when CROSS_COMPILE environtal variable is set. The name in this
+section is ignored. If more than one line is provided, only the last one
+is taken.
+
 3. Make sure you have the require Python pre-requisites
 
 Buildman uses multiprocessing, Queue, shutil, StringIO, ConfigParser and
diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py
index 41e4e4c..01cdb03 100644
--- a/tools/buildman/toolchain.py
+++ b/tools/buildman/toolchain.py
@@ -127,6 +127,15 @@ class Toolchain:
 return PRIORITY_CALC + prio
 return PRIORITY_CALC + prio
 
+def GetWrapper(self, show_warning=True):
+"""Get toolchain wrapper from the setting file.
+"""
+   value = ''
+   for name, value in bsettings.GetItems('toolchain-wrapper'):
+if not value:
+print "Warning: Wrapper not found"
+return value
+
 def MakeEnvironment(self, full_path):
 """Returns an environment for using the toolchain.
 
@@ -138,10 +147,14 @@ class Toolchain:
 PATH
 """
 env = dict(os.environ)
+wrapper = self.GetWrapper()
+if wrapper:
+wrapper = wrapper + ' '
+
 if full_path:
-env['CROSS_COMPILE'] = os.path.join(self.path, self.cross)
+env['CROSS_COMPILE'] = wrapper + os.path.join(self.path, 
self.cross)
 else:
-env['CROSS_COMPILE'] = self.cross
+env['CROSS_COMPILE'] = wrapper + self.cross
 env['PATH'] = self.path + ':' + env['PATH']
 
 return env
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot