Merge branch 'topic/christian/progressdots'

* topic/christian/progressdots:
  Add test to verify progress indicator behavior on TTYs
  Remove diff-remove-install-ticks canonifier
  Add InstallWorker.wait() method and switch progress-dot writers to it

CHANGES

@@ -1,3 +1,11 @@
+2.12.0-17 | 2022-04-25 10:36:42 -0700
+
+  * Add test to verify progress indicator behavior on TTYs (Christian Kreibich, Corelight)
+
+  * Remove diff-remove-install-ticks canonifier (Christian Kreibich, Corelight)
+
+  * Add InstallWorker.wait() method and switch progress-dot writers to it (Christian Kreibich, Corelight)
+
 2.12.0-13 | 2021-11-04 16:39:58 -0700
 
   * Request at least Sphinx 2.0 to avoid a dependency problem in RTD (Christian Kreibich, Corelight)

VERSION

@@ -1 +1 @@
-2.12.0-13
+2.12.0-17

testing/tests/dependency-management

@@ -26,7 +26,7 @@
 # @TEST-EXEC: echo '** Remove grault' >>out
 # @TEST-EXEC: zkg remove grault >>out
 
-# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-install-ticks btest-diff out
+# @TEST-EXEC: btest-diff out
 
 cd packages/foo
 echo 'depends = bar *' >> zkg.meta

testing/tests/install-on-tty

@@ -0,0 +1,27 @@
+# This test installs a package with a "slow" pseudo build command, once using a
+# pretend TTY, and once normally. It then verifies that we see progress dots
+# only in the TTY. This requires the "script" command for TTY fakery.
+
+# @TEST-REQUIRES: script --version
+# @TEST-EXEC: bash %INPUT
+
+# https://stackoverflow.com/questions/32910661/pretend-to-be-a-tty-in-bash-for-any-command
+faketty () {
+    script -qefc "$(printf "%q " "$@")" /dev/null
+}
+
+# Add a build command to the package that takes at least as long as it takes zkg
+# to produce progress dots.
+(
+    cd $(pwd)/packages/foo
+    echo 'build_command = sleep 2' >>zkg.meta
+    git add zkg.meta
+    git commit -am 'build slowly'
+)
+
+faketty zkg install foo >output.tty
+zkg uninstall --force foo
+zkg install foo >output.notty
+
+grep 'Installing' output.tty | grep -q '\.'
+grep 'Installing' output.notty | grep -v -q '\.'

testing/tests/user-mode

@@ -29,4 +29,4 @@
 # @TEST-EXEC: test -f home/testuser/.zkg/config
 # @TEST-EXEC: zkg-zeek --user config >>output
 
-# @TEST-EXEC: TEST_DIFF_CANONIFIER="$SCRIPTS/diff-remove-cwd | $SCRIPTS/diff-remove-install-ticks" btest-diff output
+# @TEST-EXEC: TEST_DIFF_CANONIFIER="$SCRIPTS/diff-remove-cwd" btest-diff output

zeekpkg/__init__.py

@@ -9,7 +9,7 @@
 
 import logging
 
-__version__ = "2.12.0-13"
+__version__ = "2.12.0-17"
 __all__ = ['manager', 'package', 'source', 'template', 'uservar']
 
 LOG = logging.getLogger(__name__)

zkg

@@ -471,6 +471,42 @@ class InstallWorker(threading.Thread):
         self.error = self.manager.install(
             self.package_name, self.package_version)
 
+    def wait(self, msg=None, out=sys.stdout, tty_only=True):
+        """Blocks until this thread ends, optionally writing liveness indicators.
+
+        This never returns until this thread dies (i.e., is_alive() is False).
+        When an output file object is provided, the method also indicates
+        progress by writing a dot character to it once per second. This happens
+        only when the file is a TTY, unless ``tty_only`` is False. When a
+        message is given, it gets written out first, regardless of TTY
+        status. Any output always terminates with a newline.
+
+        Args:
+            msg (str): a message to write first.
+
+            out (file-like object): the destination to write to.
+
+            tty_only (bool): whether to write progress dots also to non-TTYs.
+        """
+        if out is not None and msg:
+            out.write(msg)
+            out.flush()
+
+        is_tty = hasattr(out, 'isatty') and out.isatty()
+
+        while True:
+            self.join(1.0)
+            if not self.is_alive():
+                break
+
+            if out is not None and (is_tty or not tty_only):
+                out.write('.')
+                out.flush()
+
+        if out is not None and (msg or is_tty or not tty_only):
+            out.write('\n')
+            out.flush()
+
 
 def cmd_test(manager, args, config, configfile):
     if args.version and len(args.package) > 1:
@@ -689,19 +725,7 @@ def cmd_install(manager, args, config, configfile):
 
         worker = InstallWorker(manager, name, version)
         worker.start()
-        print('Installing "{}"'.format(name), end='')
-        sys.stdout.flush()
-
-        while True:
-            worker.join(1.0)
-
-            if not worker.is_alive():
-                break
-
-            print('.', end='')
-            sys.stdout.flush()
-
-        print('')
+        worker.wait('Installing "{}"'.format(name))
 
         if worker.error:
             print('Failed installing "{}": {}'.format(name, worker.error))
@@ -1392,23 +1416,7 @@ def cmd_upgrade(manager, args, config, configfile):
         tick_count = 0
         worker = InstallWorker(manager, name, version)
         worker.start()
-
-        while worker.is_alive():
-            worker.join(join_timeout)
-            time_accumulator += join_timeout
-
-            if time_accumulator >= tick_interval:
-                if tick_count == 0:
-                    print('Installing "{}"'.format(name), end='')
-                else:
-                    print('.', end='')
-
-                sys.stdout.flush()
-                tick_count += 1
-                time_accumulator -= tick_interval
-
-        if tick_count != 0:
-            print('')
+        worker.wait('Installing "{}"'.format(name))
 
         if worker.error:
             print('Failed installing "{}": {}'.format(name, worker.error))