Re: [PATCH 3/3] binman: Allow resolving host-specific tools from env vars

2020-09-05 Thread Alper Nebi Yasak 
On 05/09/2020 19:37, Simon Glass wrote:
> Please see below.
> 
> Also please add a mention of the CROSS_COMPILE thing in binman's README.
> 
>> +def GetHostCompileTool(name):
>> +"""Get the host-specific version for a compile tool
>> +
>> +This checks the environment variables that specify which version of
>> +the tool should be used.
> 
> Can you please expand the comment to mention the environment variables
> it checks?

OK, will send a v2 after doing these with the fixes for the first patch
(unless you point out until then something new that I should fix).

Re: [PATCH 3/3] binman: Allow resolving host-specific tools from env vars

2020-09-05 Thread Simon Glass 
On Sat, 5 Sep 2020 at 08:44, Alper Nebi Yasak  wrote:
>
> This patch lets tools.Run() use host-specific versions with the
> for_host keyword argument, based on the host-specific environment
> variables (HOSTCC, HOSTOBJCOPY, HOSTSTRIP, etc.).
>
> Signed-off-by: Alper Nebi Yasak 
> ---
> Not sure if this patch will ever be useful, but it complements the
> previous patch very well.

Yes, agreed.

>
>  tools/patman/tools.py | 32 +++-
>  1 file changed, 31 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass 

Please see below.

Also please add a mention of the CROSS_COMPILE thing in binman's README.

>
> diff --git a/tools/patman/tools.py b/tools/patman/tools.py
> index ee8b70d0cc..6d539fe594 100644
> --- a/tools/patman/tools.py
> +++ b/tools/patman/tools.py
> @@ -188,6 +188,31 @@ def PathHasFile(path_spec, fname):
>  return True
>  return False
>
> +def GetHostCompileTool(name):
> +"""Get the host-specific version for a compile tool
> +
> +This checks the environment variables that specify which version of
> +the tool should be used.

Can you please expand the comment to mention the environment variables
it checks?


> +
> +Args:
> +name: Command name to run
> +
> +Returns:
> +host_name: Exact command name to run instead
> +extra_args: List of extra arguments to pass
> +"""
> +host_name = None
> +extra_args = []
> +if name in ('as', 'ld', 'cc', 'cpp', 'ar', 'nm', 'ldr', 'strip',
> +'objcopy', 'objdump', 'dtc'):
> +host_name, *host_args = env.get('HOST' + name.upper(), '').split(' ')
> +elif name == 'c++':
> +host_name, *host_args = env.get('HOSTCXX', '').split(' ')
> +
> +if host_name:
> +return host_name, extra_args
> +return name, []
> +
>  def GetTargetCompileTool(name, cross_compile=None):
>  """Get the target-specific version for a compile tool
>
> @@ -250,6 +275,7 @@ def Run(name, *args, **kwargs):
>  Args:
>  name: Command name to run
>  args: Arguments to the tool
> +for_host: True to resolve the command to the version for the host
>  for_target: False to run the command as-is, without resolving it
> to the version for the compile target
>
> @@ -258,7 +284,8 @@ def Run(name, *args, **kwargs):
>  """
>  try:
>  binary = kwargs.get('binary')
> -for_target = kwargs.get('for_target', True)
> +for_host = kwargs.get('for_host', False)
> +for_target = kwargs.get('for_target', not for_host)
>  env = None
>  if tool_search_paths:
>  env = dict(os.environ)
> @@ -266,6 +293,9 @@ def Run(name, *args, **kwargs):
>  if for_target:
>  name, extra_args = GetTargetCompileTool(name)
>  args = tuple(extra_args) + args
> +elif for_host:
> +name, extra_args = GetHostCompileTool(name)
> +args = tuple(extra_args) + args
>  all_args = (name,) + args
>  result = command.RunPipe([all_args], capture=True, 
> capture_stderr=True,
>   env=env, raise_on_error=False, 
> binary=binary)
> --
> 2.28.0
>

[PATCH 3/3] binman: Allow resolving host-specific tools from env vars

2020-09-05 Thread Alper Nebi Yasak 
This patch lets tools.Run() use host-specific versions with the
for_host keyword argument, based on the host-specific environment
variables (HOSTCC, HOSTOBJCOPY, HOSTSTRIP, etc.).

Signed-off-by: Alper Nebi Yasak 
---
Not sure if this patch will ever be useful, but it complements the
previous patch very well.

 tools/patman/tools.py | 32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index ee8b70d0cc..6d539fe594 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -188,6 +188,31 @@ def PathHasFile(path_spec, fname):
 return True
 return False
 
+def GetHostCompileTool(name):
+"""Get the host-specific version for a compile tool
+
+This checks the environment variables that specify which version of
+the tool should be used.
+
+Args:
+name: Command name to run
+
+Returns:
+host_name: Exact command name to run instead
+extra_args: List of extra arguments to pass
+"""
+host_name = None
+extra_args = []
+if name in ('as', 'ld', 'cc', 'cpp', 'ar', 'nm', 'ldr', 'strip',
+'objcopy', 'objdump', 'dtc'):
+host_name, *host_args = env.get('HOST' + name.upper(), '').split(' ')
+elif name == 'c++':
+host_name, *host_args = env.get('HOSTCXX', '').split(' ')
+
+if host_name:
+return host_name, extra_args
+return name, []
+
 def GetTargetCompileTool(name, cross_compile=None):
 """Get the target-specific version for a compile tool
 
@@ -250,6 +275,7 @@ def Run(name, *args, **kwargs):
 Args:
 name: Command name to run
 args: Arguments to the tool
+for_host: True to resolve the command to the version for the host
 for_target: False to run the command as-is, without resolving it
to the version for the compile target
 
@@ -258,7 +284,8 @@ def Run(name, *args, **kwargs):
 """
 try:
 binary = kwargs.get('binary')
-for_target = kwargs.get('for_target', True)
+for_host = kwargs.get('for_host', False)
+for_target = kwargs.get('for_target', not for_host)
 env = None
 if tool_search_paths:
 env = dict(os.environ)
@@ -266,6 +293,9 @@ def Run(name, *args, **kwargs):
 if for_target:
 name, extra_args = GetTargetCompileTool(name)
 args = tuple(extra_args) + args
+elif for_host:
+name, extra_args = GetHostCompileTool(name)
+args = tuple(extra_args) + args
 all_args = (name,) + args
 result = command.RunPipe([all_args], capture=True, capture_stderr=True,
  env=env, raise_on_error=False, binary=binary)
-- 
2.28.0