[PATCH 0/3] knotty: Add automatic pager support

[PATCH 0/3] knotty: Add automatic pager support

[Rob Woolley]

The bitbake show_versions (-s) and show_environment (-e) commands provide
a lot of non-interactive output.  Even experienced users will sometimes
forget to page the output.

This series adds a feature to automatically page these commands when
using the knotty user interface.  It doesn't affect any other commands
and is designed to work in a similar fashion to the auto-paging in git.

This was tested with bitbake -s and bitbake -s | less (and -e) to ensure that
there were no regressions.  This includes testing broken pipe conditions
caused by quitting the pager before the full output has been sent through
the pipe.

knotty: Catch exceptions on broken pipes
 lib/bb/ui/knotty.py |   43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

knotty: Do not log show_versions output
 lib/bb/ui/knotty.py |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

knotty: Add automatic paging for bitbake -s and -e
 lib/bb/ui/knotty.py |   46 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

[PATCH 1/3] knotty: Catch exceptions on broken pipes

[Rob Woolley]

Any exceptions that occur in calls to logging methods are automatically 
suppressed, including exceptions due to broken pipes.

However, the knotty summary messages are printed directly to stdout, which
means that any broken pipes will cause an exception traceback in python.

By wrapping the summary section in a try / catch block we can check for
IOError exceptions caused by broken pipes and let them pass.

Signed-off-by: Rob Woolley <rob.woolley@windriver.com>
---
 lib/bb/ui/knotty.py | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/lib/bb/ui/knotty.py b/lib/bb/ui/knotty.py
index ea20ddc..09a4e0c 100644
--- a/lib/bb/ui/knotty.py
+++ b/lib/bb/ui/knotty.py
@@ -536,24 +536,29 @@ def main(server, eventHandler, params, tf = TerminalFilter):
             if not params.observe_only:
                 _, error = server.runCommand(["stateForceShutdown"])
             main.shutdown = 2
-    summary = ""
-    if taskfailures:
-        summary += pluralise("\nSummary: %s task failed:",
-                             "\nSummary: %s tasks failed:", len(taskfailures))
-        for failure in taskfailures:
-            summary += "\n  %s" % failure
-    if warnings:
-        summary += pluralise("\nSummary: There was %s WARNING message shown.",
-                             "\nSummary: There were %s WARNING messages shown.", warnings)
-    if return_value and errors:
-        summary += pluralise("\nSummary: There was %s ERROR message shown, returning a non-zero exit code.",
-                             "\nSummary: There were %s ERROR messages shown, returning a non-zero exit code.", errors)
-    if summary:
-        print(summary)
-
-    if interrupted:
-        print("Execution was interrupted, returning a non-zero exit code.")
-        if return_value == 0:
-            return_value = 1
+    try:
+         summary = ""
+         if taskfailures:
+             summary += pluralise("\nSummary: %s task failed:",
+                                  "\nSummary: %s tasks failed:", len(taskfailures))
+             for failure in taskfailures:
+                 summary += "\n  %s" % failure
+         if warnings:
+             summary += pluralise("\nSummary: There was %s WARNING message shown.",
+                                  "\nSummary: There were %s WARNING messages shown.", warnings)
+         if return_value and errors:
+             summary += pluralise("\nSummary: There was %s ERROR message shown, returning a non-zero exit code.",
+                                  "\nSummary: There were %s ERROR messages shown, returning a non-zero exit code.", errors)
+         if summary:
+             print(summary)
+
+         if interrupted:
+             print("Execution was interrupted, returning a non-zero exit code.")
+             if return_value == 0:
+                 return_value = 1
+    except IOError as e:
+        import errno
+        if e.errno == errno.EPIPE:
+            pass
 
     return return_value
-- 
1.8.3.1

[PATCH 2/3] knotty: Do not log show_versions output

[Rob Woolley]

Every time the bitbake show versions command (bitbake -s) is run it creates
a 100k log file.

The consolelogfile is disabled for show environment and disabling show
versions would make the behaviour match.

Signed-off-by: Rob Woolley <rob.woolley@windriver.com>
---
 lib/bb/ui/knotty.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bb/ui/knotty.py b/lib/bb/ui/knotty.py
index 09a4e0c..df0b854 100644
--- a/lib/bb/ui/knotty.py
+++ b/lib/bb/ui/knotty.py
@@ -271,7 +271,7 @@ def main(server, eventHandler, params, tf = TerminalFilter):
         server.terminateServer()
         return
 
-    if consolelogfile and not params.options.show_environment:
+    if consolelogfile and not params.options.show_environment and not params.options.show_versions:
         bb.utils.mkdirhier(os.path.dirname(consolelogfile))
         conlogformat = bb.msg.BBLogFormatter(format_str)
         consolelog = logging.FileHandler(consolelogfile)
-- 
1.8.3.1

[PATCH 3/3] knotty: Add automatic paging for bitbake -s and -e

[Rob Woolley]

The bitbake show environment and show versions commands provide a
large volume of output to the user.  The knotty user interface changes
the output it provides based on whether it detects a TTY.

By checking for the presense of a TTY, we can automatically pipe
the output to a child process that pages the output for the user.
The logic for the automatic paging is based on pager.c found in git.

Some users may have a preferred pager already set in the PAGER
environment variable.  If it is set, found in PATH, and executable
then we use the user's preferred pager.  If it is set to "cat", then
paging is disabled.  If no pager is set, then we set it to "less" as
the default.

The LESS and LV environment variables are also set with sane defaults
in the event that either of these pagers are used.

We cleanly close the pipe before the summary message is displayed.  This 
is done intentionally so that the user does not miss any of the summary 
information if they close the pipe early.

Tested with both bitbake -s and -e with and without an explicit pipe
to a pager.  This includes testing broken pipe conditions caused by 
quitting the pager before the full output has been sent through the pipe.

Pagers tested include: less, more, cat, more, lv, and vim less.sh

Signed-off-by: Rob Woolley <rob.woolley@windriver.com>
---
 lib/bb/ui/knotty.py | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

Index: b/lib/bb/ui/knotty.py
===================================================================
--- a/lib/bb/ui/knotty.py
+++ b/lib/bb/ui/knotty.py
@@ -32,6 +32,7 @@ import fcntl
 import struct
 import copy
 import atexit
+import subprocess
 from bb.ui import uihelper
 
 featureSet = [bb.cooker.CookerFeatures.SEND_SANITYEVENTS]
@@ -138,7 +139,7 @@ class TerminalFilter(object):
         self.helper = helper
         self.cuu = None
         self.stdinbackup = None
-        self.interactive = sys.stdout.isatty()
+        self.interactive = console.stream.isatty()
         self.footer_present = False
         self.lastpids = []
 
@@ -256,8 +257,42 @@ def main(server, eventHandler, params, t
 
     helper = uihelper.BBUIHelper()
 
-    console = logging.StreamHandler(sys.stdout)
-    errconsole = logging.StreamHandler(sys.stderr)
+    pager_bin = None
+    pager_process = None
+
+    if (os.getenv("PAGER")):
+        pager_bin = os.getenv("PAGER")
+    else:
+        pager_bin = "less"
+
+    if pager_bin == "cat":
+        pager_bin = None
+
+    if pager_bin and (not os.path.isfile(pager_bin) or not os.access(pager_bin, os.X_OK)):
+        found_pager = False
+        for path in os.environ["PATH"].split(os.pathsep):
+             pager_path = os.path.join(path,pager_bin)
+             if os.path.isfile(pager_path) and os.access(pager_path, os.X_OK):
+                 found_pager = True
+                 break
+
+        if not found_pager:
+            pager_bin = None
+
+    if (os.getenv("LESS") == None):
+        os.environ['LESS'] = 'FRX'
+
+    if (os.getenv("LV") == None):
+        os.environ['LV'] = '-c'
+
+    if interactive and pager_bin and (params.options.show_versions or params.options.show_environment):
+        pager_process = subprocess.Popen(pager_bin, stdin=subprocess.PIPE)
+        console = logging.StreamHandler(pager_process.stdin)
+        errconsole = logging.StreamHandler(pager_process.stdin)
+    else:
+        console = logging.StreamHandler(sys.stdout)
+        errconsole = logging.StreamHandler(sys.stderr)
+
     format_str = "%(levelname)s: %(message)s"
     format = bb.msg.BBLogFormatter(format_str)
     bb.msg.addDefaultlogFilter(console, bb.msg.BBLogFilterStdOut)
@@ -536,6 +571,11 @@ def main(server, eventHandler, params, t
             if not params.observe_only:
                 _, error = server.runCommand(["stateForceShutdown"])
             main.shutdown = 2
+
+    if pager_process:
+        pager_process.stdin.close()
+        pager_process.wait()
+
     try:
          summary = ""
          if taskfailures:

Re: [PATCH 0/3] knotty: Add automatic pager support

[Rob Woolley]

Hi,

Just wanted to send out a ping to see if this enhancement is desired.

It scratches an itch for me, but not sure how others feel about it.

If there are any changes I can make or extra tests I can run, please let 
me know.

Cheers,
Rob

On 02/27/2015 09:32 AM, Rob Woolley wrote:
> The bitbake show_versions (-s) and show_environment (-e) commands provide
> a lot of non-interactive output.  Even experienced users will sometimes
> forget to page the output.
>
> This series adds a feature to automatically page these commands when
> using the knotty user interface.  It doesn't affect any other commands
> and is designed to work in a similar fashion to the auto-paging in git.
>
> This was tested with bitbake -s and bitbake -s | less (and -e) to ensure that
> there were no regressions.  This includes testing broken pipe conditions
> caused by quitting the pager before the full output has been sent through
> the pipe.
>
> knotty: Catch exceptions on broken pipes
>   lib/bb/ui/knotty.py |   43 ++++++++++++++++++++++++-------------------
>   1 file changed, 24 insertions(+), 19 deletions(-)
>
> knotty: Do not log show_versions output
>   lib/bb/ui/knotty.py |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> knotty: Add automatic paging for bitbake -s and -e
>   lib/bb/ui/knotty.py |   46 +++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 43 insertions(+), 3 deletions(-)

Re: [PATCH 0/3] knotty: Add automatic pager support

[Richard Purdie]

On Thu, 2015-03-12 at 11:25 -0400, Rob Woolley wrote:
> Hi,
> 
> Just wanted to send out a ping to see if this enhancement is desired.
> 
> It scratches an itch for me, but not sure how others feel about it.
> 
> If there are any changes I can make or extra tests I can run, please let 
> me know.

Patches 1/2 look good, I've run into the EPIPE issue myself and I have
noticed the crazy logfile so those can likely be merged.

I'm less sure about 3, letting the user pipe the output to less when
needed would seem to be the better way to have this rather than adding
in specific pagination support?

Cheers,

Richard

Re: [PATCH 0/3] knotty: Add automatic pager support

[Woolley, Rob]

On 03/12/2015 12:30 PM, Richard Purdie wrote:
> On Thu, 2015-03-12 at 11:25 -0400, Rob Woolley wrote:
>> Hi,
>>
>> Just wanted to send out a ping to see if this enhancement is desired.
>>
>> It scratches an itch for me, but not sure how others feel about it.
>>
>> If there are any changes I can make or extra tests I can run, please let
>> me know.
> Patches 1/2 look good, I've run into the EPIPE issue myself and I have
> noticed the crazy logfile so those can likely be merged.
>
> I'm less sure about 3, letting the user pipe the output to less when
> needed would seem to be the better way to have this rather than adding
> in specific pagination support?
>
> Cheers,
>
> Richard
Sounds cool.  The exception was bugging me, so I'm glad to hear those 
ones look good.

As far as the 3rd patch is concerned: I remember when I first used 
bitbake, I was surprised by the flood of data coming from those 
options.  I would suggest that doing the automatic paging provides a 
better first impression and saves unnecessary typing.

I noticed that lib/image/help.py and lib/bsp/help.py have auto-paging 
for the help command.  I've CC'd Tom Zanussi for his opinion.

Regards,
Rob

Re: [PATCH 0/3] knotty: Add automatic pager support

[Bernhard Reutner-Fischer]

On March 13, 2015 8:41:37 PM GMT+01:00, "Woolley, Rob" <rob.woolley@windriver.com> wrote:
>
>On 03/12/2015 12:30 PM, Richard Purdie wrote:
>> On Thu, 2015-03-12 at 11:25 -0400, Rob Woolley wrote:
>>> Hi,
>>>
>>> Just wanted to send out a ping to see if this enhancement is
>desired.
>>>
>>> It scratches an itch for me, but not sure how others feel about it.
>>>
>>> If there are any changes I can make or extra tests I can run, please
>let
>>> me know.
>> Patches 1/2 look good, I've run into the EPIPE issue myself and I
>have
>> noticed the crazy logfile so those can likely be merged.
>>
>> I'm less sure about 3, letting the user pipe the output to less when
>> needed would seem to be the better way to have this rather than
>adding
>> in specific pagination support?
>>
>> Cheers,
>>
>> Richard
>Sounds cool.  The exception was bugging me, so I'm glad to hear those 
>ones look good.
>
>As far as the 3rd patch is concerned: I remember when I first used 
>bitbake, I was surprised by the flood of data coming from those 
>options.  I would suggest that doing the automatic paging provides a 
>better first impression and saves unnecessary typing.
>
>I noticed that lib/image/help.py and lib/bsp/help.py have auto-paging 
>for the help command.  I've CC'd Tom Zanussi for his opinion.

Using PAGER is a good thing IMO.
Not so setting LESS or LV nor outsmarting users PAGER=cat nor defaulting to less.
I would also not attempt to check access() et al, just let the user fix her eventually broken export.

YMMV.
HTH && cheers,

Re: [PATCH 0/3] knotty: Add automatic pager support

[Bernhard Reutner-Fischer]

On March 13, 2015 10:12:00 PM GMT+01:00, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>On March 13, 2015 8:41:37 PM GMT+01:00, "Woolley, Rob"
><rob.woolley@windriver.com> wrote:
>>
>>On 03/12/2015 12:30 PM, Richard Purdie wrote:
>>> On Thu, 2015-03-12 at 11:25 -0400, Rob Woolley wrote:
>>>> Hi,
>>>>
>>>> Just wanted to send out a ping to see if this enhancement is
>>desired.
>>>>
>>>> It scratches an itch for me, but not sure how others feel about it.
>>>>
>>>> If there are any changes I can make or extra tests I can run,
>please
>>let
>>>> me know.
>>> Patches 1/2 look good, I've run into the EPIPE issue myself and I
>>have
>>> noticed the crazy logfile so those can likely be merged.
>>>
>>> I'm less sure about 3, letting the user pipe the output to less when
>>> needed would seem to be the better way to have this rather than
>>adding
>>> in specific pagination support?
>>>
>>> Cheers,
>>>
>>> Richard
>>Sounds cool.  The exception was bugging me, so I'm glad to hear those 
>>ones look good.
>>
>>As far as the 3rd patch is concerned: I remember when I first used 
>>bitbake, I was surprised by the flood of data coming from those 
>>options.  I would suggest that doing the automatic paging provides a 
>>better first impression and saves unnecessary typing.
>>
>>I noticed that lib/image/help.py and lib/bsp/help.py have auto-paging 
>>for the help command.  I've CC'd Tom Zanussi for his opinion.
>
>Using PAGER is a good thing IMO.
>Not so setting LESS or LV nor outsmarting users PAGER=cat nor
>defaulting to less.
>I would also not attempt to check access() et al, just let the user fix
>her eventually broken export.

PS:
http://comments.gmane.org/gmane.comp.version-control.patchwork/1144
;)

Re: [PATCH 0/3] knotty: Add automatic pager support

[Woolley, Rob]

[-- Attachment #1: Type: text/plain, Size: 2235 bytes --]



On 3/13/2015 5:37 PM, Bernhard Reutner-Fischer wrote:
> On March 13, 2015 10:12:00 PM GMT+01:00, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>> On March 13, 2015 8:41:37 PM GMT+01:00, "Woolley, Rob"
>> <rob.woolley@windriver.com> wrote:
>>> On 03/12/2015 12:30 PM, Richard Purdie wrote:
>>>
>>>> Patches 1/2 look good, I've run into the EPIPE issue myself and I
>>> have
>>>> noticed the crazy logfile so those can likely be merged.
>>>>
>>>> I'm less sure about 3, letting the user pipe the output to less when
>>>> needed would seem to be the better way to have this rather than
>>> adding
>>>> in specific pagination support?
>>>>
>>>> Cheers,
>>>>
>>>> Richard
>>> Sounds cool.  The exception was bugging me, so I'm glad to hear those
>>> ones look good.
>>>
>>> As far as the 3rd patch is concerned: I remember when I first used
>>> bitbake, I was surprised by the flood of data coming from those
>>> options.  I would suggest that doing the automatic paging provides a
>>> better first impression and saves unnecessary typing.
>>>
>>> I noticed that lib/image/help.py and lib/bsp/help.py have auto-paging
>>> for the help command.  I've CC'd Tom Zanussi for his opinion.
>> Using PAGER is a good thing IMO.
>> Not so setting LESS or LV nor outsmarting users PAGER=cat nor
>> defaulting to less.
>> I would also not attempt to check access() et al, just let the user fix
>> her eventually broken export.
> PS:
> http://comments.gmane.org/gmane.comp.version-control.patchwork/1144
> ;)

Thanks for the feedback, Bernhard.

I set defaults for LESS and LV based on what git does for its automatic 
paging:
https://github.com/git/git/blob/master/pager.c#L77 
<https://github.com/git/git/blob/master/pager.c#L57>

The part with cat is also inspired by git:
https://github.com/git/git/blob/master/pager.c#L51 
<https://github.com/git/git/blob/master/pager.c#L34>
<https://github.com/git/git/blob/master/pager.c#L57>
I see the first 2 patches went in.  I'm going to give the automatic 
paging patch some more soak time.

If anyone is interested in trying it out, I've created a gist for it here:
https://gist.github.com/rcwoolley/c662b8e1494c58b1a24d

Regards,
Rob

[-- Attachment #2: Type: text/html, Size: 3676 bytes --]