[U-Boot] [PATCH] tools: genboardscfg.py, use default terminal size if undetectable

Roger Meier

author

2014-08-07 10:46 UTC #0 lore raw

PATCH 1/1 tools: genboardscfg.py, use default terminal size if undetectable

The existing terminalsize detection raised an exception on build server. Removes the exception and return a default value.

Signed-off-by Roger Meier <[email protected]>

CC Masahiro Yamada <[email protected]>

---
 tools/genboardscfg.py |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

tools/genboardscfg.py +3 -5

index 734d90b..892c033 100755
@@ -58,11 +58,9 @@ def get_terminal_columns():
         try:
             ret = fcntl.ioctl(sys.stdout.fileno(), termios.TIOCGWINSZ, arg)
         except IOError as exception:
-            if exception.errno != errno.ENOTTY:
-                raise
             # If 'Inappropriate ioctl for device' error occurs,
-            # stdout is probably redirected. Return 0.
-            return 0
+            # stdout is probably redirected. Return default size.
+            return (25, 80)
         return struct.unpack('hhhh', ret)[1]
 
 def get_devnull():

@@ -408,7 +406,7 @@ def __gen_boards_cfg(jobs):
       jobs: The number of jobs to run simultaneously
 
     Note:
-      The incomplete boards.cfg is left over when an error (including 
+      The incomplete boards.cfg is left over when an error (including
       the termination by the keyboard interrupt) occurs on the halfway.
     """
     check_top_directory()

Signature · 1 line

1.7.10.4

Tom Rini

2014-08-07 11:37 UTC #1 lore raw

PATCH 1/1 tools: genboardscfg.py, use default terminal size if undetectable

Quoted 14 lines · Roger Meier

The existing terminalsize detection raised an exception on build server. Removes the exception and return a default value.
Signed-off-by Roger Meier <[email protected]>
CC Masahiro Yamada <[email protected]>
---
 tools/genboardscfg.py |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
tools/genboardscfg.py +0 -0
index 734d90b..892c033 100755

tools/genboardscfg.py def get_terminal_columns(): · line 63

Quoted 10 lines

tools/genboardscfg.py +2 -4

@@ -58,11 +58,9 @@ def get_terminal_columns():
         try:
             ret = fcntl.ioctl(sys.stdout.fileno(), termios.TIOCGWINSZ, arg)
         except IOError as exception:
-            if exception.errno != errno.ENOTTY:
-                raise
             # If 'Inappropriate ioctl for device' error occurs,
-            # stdout is probably redirected. Return 0.
-            return 0
+            # stdout is probably redirected. Return default size.
+            return (25, 80)

25 not 24?

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140807/83fb75a2/attachment.pgp>

Roger Meier

author

2014-08-07 12:15 UTC #2 lore raw

PATCH 1/1 tools: genboardscfg.py, use default terminal size if undetectable

The existing terminalsize detection raised an exception on build server. Removes the exception and return a default value.

Signed-off-by Roger Meier <[email protected]>

CC Masahiro Yamada <[email protected]>

CC Tom Rini <[email protected]>

---
Changes for v2:
- use 24 instead of 25 lines

---
 tools/genboardscfg.py |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

tools/genboardscfg.py +3 -5

index 734d90b..283c989 100755
@@ -58,11 +58,9 @@ def get_terminal_columns():
         try:
             ret = fcntl.ioctl(sys.stdout.fileno(), termios.TIOCGWINSZ, arg)
         except IOError as exception:
-            if exception.errno != errno.ENOTTY:
-                raise
             # If 'Inappropriate ioctl for device' error occurs,
-            # stdout is probably redirected. Return 0.
-            return 0
+            # stdout is probably redirected. Return default size.
+            return (24, 80)
         return struct.unpack('hhhh', ret)[1]
 
 def get_devnull():

@@ -408,7 +406,7 @@ def __gen_boards_cfg(jobs):
       jobs: The number of jobs to run simultaneously
 
     Note:
-      The incomplete boards.cfg is left over when an error (including 
+      The incomplete boards.cfg is left over when an error (including
       the termination by the keyboard interrupt) occurs on the halfway.
     """
     check_top_directory()

Signature · 1 line

1.7.10.4

Masahiro Yamada

2014-08-07 12:47 UTC #3 lore raw

PATCH 1/1 tools: genboardscfg.py, use default terminal size if undetectable

Hi Roger,

Quoted 42 lines · On Thu, 7 Aug 2014 14:15:09 +0200 Roger Meier <[email protected]> wrote:

The existing terminalsize detection raised an exception on build server. Removes the exception and return a default value.

Signed-off-by Roger Meier <[email protected]>

CC Masahiro Yamada <[email protected]>

CC Tom Rini <[email protected]>

---
Changes for v2:
- use 24 instead of 25 lines

---
 tools/genboardscfg.py |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

tools/genboardscfg.py +3 -5

index 734d90b..283c989 100755
@@ -58,11 +58,9 @@ def get_terminal_columns():
         try:
             ret = fcntl.ioctl(sys.stdout.fileno(), termios.TIOCGWINSZ, arg)
         except IOError as exception:
-            if exception.errno != errno.ENOTTY:
-                raise
             # If 'Inappropriate ioctl for device' error occurs,
-            # stdout is probably redirected. Return 0.
-            return 0
+            # stdout is probably redirected. Return default size.
+            return (24, 80)
         return struct.unpack('hhhh', ret)[1]
 
 def get_devnull():

@@ -408,7 +406,7 @@ def __gen_boards_cfg(jobs):
       jobs: The number of jobs to run simultaneously
 
     Note:
-      The incomplete boards.cfg is left over when an error (including 
+      The incomplete boards.cfg is left over when an error (including
       the termination by the keyboard interrupt) occurs on the halfway.
     """
     check_top_directory()

Signature · 1 line

1.7.10.4

I assume you fixed (25, 80) to (24, 80)

But the problem is not in it.

See my comment in v1.

BTW, this is some advice from me.

- Add "v2" prefix to the patch subject
- Describe which parts were changes

I recommend you to read
http://www.denx.de/wiki/U-Boot/Patches

Best Regards
Masahiro Yamada

Roger Meier

author

2014-08-07 14:19 UTC #4 lore raw

PATCH v3 1/1 tools: genboardscfg.py, no exception if columns undetectable

The existing terminalsize detection raised an exception on build server. Just removes the exception. This also deactivates the progress indicator.

Remove a trainling whitespace.

Signed-off-by Roger Meier <[email protected]>

CC Masahiro Yamada <[email protected]>

CC Tom Rini <[email protected]>

---
Changes for v2:
- use 24 instead of 25 lines
Changes for v3:
- return integer with 0 columns instead of tuple, this also deactivates the progress indicator

---
 tools/genboardscfg.py |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

tools/genboardscfg.py +1 -3

index 734d90b..cd9e69c 100755
@@ -58,8 +58,6 @@ def get_terminal_columns():
         try:
             ret = fcntl.ioctl(sys.stdout.fileno(), termios.TIOCGWINSZ, arg)
         except IOError as exception:
-            if exception.errno != errno.ENOTTY:
-                raise
             # If 'Inappropriate ioctl for device' error occurs,
             # stdout is probably redirected. Return 0.
             return 0

@@ -408,7 +406,7 @@ def __gen_boards_cfg(jobs):
       jobs: The number of jobs to run simultaneously
 
     Note:
-      The incomplete boards.cfg is left over when an error (including 
+      The incomplete boards.cfg is left over when an error (including
       the termination by the keyboard interrupt) occurs on the halfway.
     """
     check_top_directory()

Signature · 1 line

1.7.10.4

Masahiro YAMADA

2014-08-09 00:37 UTC #5 lore raw

PATCH v3 1/1 tools: genboardscfg.py, no exception if columns undetectable

Hi Roger,
(adding York to cc)

2014-08-07 23:19 GMT+09:00 Roger Meier <[email protected]>:

Quoted 9 lines

The existing terminalsize detection raised an exception on build server. Just removes the exception. This also deactivates the progress indicator.
Remove a trainling whitespace.
Signed-off-by Roger Meier <[email protected]>
CC Masahiro Yamada <[email protected]>
CC Tom Rini <[email protected]>

Acked-by Masahiro Yamada <[email protected]>

Tom,
This problem seems fatal. Please apply this patch asap.

Thanks!

Signature · 2 lines

Best Regards
Masahiro Yamada

Roger Meier

author

2014-08-17 16:07 UTC #6 lore raw

PATCH v3 1/1 tools: genboardscfg.py, no exception if columns undetectable

Quoting Masahiro YAMADA <[email protected]>:

Quoted 19 lines

Hi Roger,
(adding York to cc)
2014-08-07 23:19 GMT+09:00 Roger Meier <[email protected]>:
Quoted 9 lines
The existing terminalsize detection raised an exception on build server. Just removes the exception. This also deactivates the progress indicator.
Remove a trainling whitespace.
Signed-off-by Roger Meier <[email protected]>
CC Masahiro Yamada <[email protected]>
CC Tom Rini <[email protected]>
Acked-by Masahiro Yamada <[email protected]>

Quoted 8 lines

Tom,
This problem seems fatal. Please apply this patch asap.
Thanks!
--
Best Regards
Masahiro Yamada

Yes, building on a build server is still not possible without this patch. It does not matter if you use buildman or MAKEALL.

best!
-roger

Tom Rini

2014-08-23 12:42 UTC #7 lore raw

PATCH v3 1/1 tools: genboardscfg.py, no exception if columns undetectable

In reply to Roger Meier

Quoted 9 lines · Roger Meier

The existing terminalsize detection raised an exception on build server. Just removes the exception. This also deactivates the progress indicator.
Remove a trainling whitespace.
Signed-off-by Roger Meier <[email protected]>
CC Masahiro Yamada <[email protected]>
CC Tom Rini <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140823/99e16e07/attachment.pgp>

Masahiro Yamada

2014-08-07 12:47 UTC #8 lore raw

PATCH 1/1 tools: genboardscfg.py, use default terminal size if undetectable

In reply to Roger Meier

Hi Roger,

Quoted 15 lines · On Thu, 7 Aug 2014 12:46:53 +0200 Roger Meier <[email protected]> wrote:

The existing terminalsize detection raised an exception on build server. Removes the exception and return a default value.
Signed-off-by Roger Meier <[email protected]>
CC Masahiro Yamada <[email protected]>
---
 tools/genboardscfg.py |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
tools/genboardscfg.py +0 -0
index 734d90b..892c033 100755
@@ -58,11 +58,9 @@ def get_terminal_columns():
         try:

tools/genboardscfg.py def get_terminal_columns(): · line 64

Quoted 10 lines

tools/genboardscfg.py +2 -4

             ret = fcntl.ioctl(sys.stdout.fileno(), termios.TIOCGWINSZ, arg)
         except IOError as exception:
-            if exception.errno != errno.ENOTTY:
-                raise
             # If 'Inappropriate ioctl for device' error occurs,
-            # stdout is probably redirected. Return 0.
-            return 0
+            # stdout is probably redirected. Return default size.
+            return (25, 80)
         return struct.unpack('hhhh', ret)[1]

Wrong.
This function is supposed to return an integer. With your patch, it returns a tuple on the exception.

OK.
We can fix it easily.
(25, 80) --> 80

But I still see a problem.

When ENOTTY error happens, tty is not associated. No reason to display the progress indicator.

Assume this tool is called from an upper level script and the output is redirected to a log file.

In that case, I get an ugly log file as follows:

^M 1/1177 [> ]^M 2/1177 [> ]^M 3/1177 [> ]^M 4/1177 [> ]^M 5/1177 [> ]^M 6/1177 [> ]^M 7/1177 [> ]^M 8/1177 [> ]^M 9/1177 [> ]^M 10/1177 [> ]^M 11/1177 [> ]^M 12/1177 [> ]^M 13/1177 [> ]^M 14/1177 [> ]^M 15/1177 [> ]^M 16/1177 [> ]^M 17/1177 [> ]^M 18/1177 [> ]^M 19/1177 [> ]^M 20/1177 [> ]^M 21/1177 [> ]^M 22/1177 [=> ]^M 23/1177 [=> ]^M 24/1177 [=> ]^M 25/1177 [=> ]^M 26/1177 [=> ]^M 27/1177 [=> ]^M 28/1177 [=> ]^M 29/1177 [=> ]^M 30/117

tools/genboardscfg.py def __gen_boards_cfg(jobs): · line 412

Quoted 10 lines

tools/genboardscfg.py +1 -1

 def get_devnull():

@@ -408,7 +406,7 @@ def __gen_boards_cfg(jobs):
       jobs: The number of jobs to run simultaneously
 
     Note:
-      The incomplete boards.cfg is left over when an error (including 
+      The incomplete boards.cfg is left over when an error (including
       the termination by the keyboard interrupt) occurs on the halfway.
     """
     check_top_directory()

This fix is correct.
(Remove a trainling whitespace.)

Best Regards
Masahiro Yamada