[U-Boot] [PATCH v4 08/10] moveconfig: Print a message for missing compiler

[Joe Hershberger]

Hi Masahiro-san,

On Mon, May 18, 2015 at 10:23 PM, Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
> 2015-05-16 6:40 GMT+09:00 Joe Hershberger <joe.hershberger at ni.com>:
>> A common case for failed builds is a missing compiler. Print a message
>> for that case to tell the user concisely which compiler was expected
>> that was not found.
>>
>> This patch also has the effect of not printing build errors any longer.
>> The next patch will add a switch to optionally bring that back.
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
>> ---
>>
>> Changes in v4: None
>> Changes in v3: None
>> Changes in v2: None
>>
>>  tools/moveconfig.py | 16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
>> index 9e923da..f986f55 100755
>> --- a/tools/moveconfig.py
>> +++ b/tools/moveconfig.py
>> @@ -166,6 +166,7 @@ import os
>>  import re
>>  import shutil
>>  import subprocess
>> +from subprocess import PIPE
>
> Personally I do not prefer from ... import
> because it disturbs the naming space.
>
> Could you use subprocess.PIPE instead?

OK.

>>  import sys
>>  import tempfile
>>  import time
>> @@ -606,11 +607,14 @@ class Slot:
>>              return False
>>
>>          if self.ps.poll() != 0:
>> -
>> +            errmsg = 'Failed to process.'
>> +            errout = self.ps.stderr.read()
>
> This throws exception if "make *_defconfig" or "make savedefconfig" fail.
>
>
>
> Traceback (most recent call last):
>   File "tools/moveconfig.py", line 924, in <module>
>     main()
>   File "tools/moveconfig.py", line 919, in main
>     move_config(config_attrs, options)
>   File "tools/moveconfig.py", line 794, in move_config
>     while not slots.available():
>   File "tools/moveconfig.py", line 717, in available
>     if slot.poll():
>   File "tools/moveconfig.py", line 616, in poll
>     errout = self.ps.stderr.read()
> AttributeError: 'NoneType' object has no attribute 'read'
>
>
>
> Seems better to add PIPE for all the call of subprocess.Popen()

OK

>> +            if errout.find('gcc: command not found') != -1:
>> +                errmsg = 'Compiler not found (%s)' % self.cross_compile
>
> If you do this, should the locale be changed?
>
> Without LANG=C, "command not found" is displayed in Japanese on my computer.
>
>
> If --verbose is given, we will be able to know the cause of erorr.
> "missing compiler" is a special case error?

That's true, but at least for my use-case before I spent several days
getting all tool-chains working, it was nice to know what the build
was trying to use for a cross tool-chain in a concise one-liner
instead of parsing a bunch of error prints. That's part of why I added
the -v flag. It's also helpful (naturally) in getting the compilers
all working.

>>              print >> sys.stderr, log_msg(self.options.color,
>>                                           COLOR_LIGHT_RED,
>>                                           self.defconfig,
>> -                                         "failed to process.")
>> +                                         errmsg),
>>              if self.options.exit_on_error:
>>                  sys.exit("Exit on error.")
>>              else:
>> @@ -644,13 +648,13 @@ class Slot:
>>              self.state = STATE_IDLE
>>              return True
>>
>> -        cross_compile = self.parser.get_cross_compile()
>> +        self.cross_compile = self.parser.get_cross_compile()
>>          cmd = list(self.make_cmd)
>> -        if cross_compile:
>> -            cmd.append('CROSS_COMPILE=%s' % cross_compile)
>> +        if self.cross_compile:
>> +            cmd.append('CROSS_COMPILE=%s' % self.cross_compile)
>>          cmd.append('KCONFIG_IGNORE_DUPLICATES=1')
>>          cmd.append('include/config/auto.conf')
>> -        self.ps = subprocess.Popen(cmd, stdout=self.devnull)
>> +        self.ps = subprocess.Popen(cmd, stdout=self.devnull, stderr=PIPE)
>>          self.state = STATE_AUTOCONF
>>          return False
>>
>
> --
> Best Regards
> Masahiro Yamada
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot