Merge remote-tracking branch 'origin/unienv'

* origin/unienv:
  docs: update Changelog.
  issue #600: /etc/environment may be non-ASCII in an unknown encoding
  docs: finished Changelog locking note
  Fix for sample in doc
  docs: break out install_app.py and fix API use.

ansible_mitogen/runner.py

@@ -52,7 +52,6 @@ import mitogen.core
 import ansible_mitogen.target  # TODO: circular import
 from mitogen.core import b
 from mitogen.core import bytes_partition
-from mitogen.core import str_partition
 from mitogen.core import str_rpartition
 from mitogen.core import to_text
 
@@ -104,12 +103,20 @@ iteritems = getattr(dict, 'iteritems', dict.items)
 LOG = logging.getLogger(__name__)
 
 
-if mitogen.core.PY3:
-    shlex_split = shlex.split
-else:
-    def shlex_split(s, comments=False):
-        return [mitogen.core.to_text(token)
-                for token in shlex.split(str(s), comments=comments)]
+def shlex_split_b(s):
+    """
+    Use shlex.split() to split characters in some single-byte encoding, without
+    knowing what that encoding is. The input is bytes, the output is a list of
+    bytes.
+    """
+    assert isinstance(s, mitogen.core.BytesType)
+    if mitogen.core.PY3:
+        return [
+            t.encode('latin1')
+            for t in shlex.split(s.decode('latin1'), comments=True)
+        ]
+
+    return [t for t in shlex.split(s, comments=True)]
 
 
 class TempFileWatcher(object):
@@ -165,13 +172,19 @@ class EnvironmentFileWatcher(object):
     A more robust future approach may simply be to arrange for the persistent
     interpreter to restart when a change is detected.
     """
+    # We know nothing about the character set of /etc/environment or the
+    # process environment.
+    environ = getattr(os, 'environb', os.environ)
+
     def __init__(self, path):
         self.path = os.path.expanduser(path)
         #: Inode data at time of last check.
         self._st = self._stat()
         #: List of inherited keys appearing to originated from this file.
-        self._keys = [key for key, value in self._load()
-                      if value == os.environ.get(key)]
+        self._keys = [
+            key for key, value in self._load()
+            if value == self.environ.get(key)
+        ]
         LOG.debug('%r installed; existing keys: %r', self, self._keys)
 
     def __repr__(self):
@@ -185,7 +198,7 @@ class EnvironmentFileWatcher(object):
 
     def _load(self):
         try:
-            fp = codecs.open(self.path, 'r', encoding='utf-8')
+            fp = open(self.path, 'rb')
             try:
                 return list(self._parse(fp))
             finally:
@@ -199,36 +212,36 @@ class EnvironmentFileWatcher(object):
         """
         for line in fp:
             # '   #export foo=some var  ' -> ['#export', 'foo=some var  ']
-            bits = shlex_split(line, comments=True)
-            if (not bits) or bits[0].startswith('#'):
+            bits = shlex_split_b(line)
+            if (not bits) or bits[0].startswith(b('#')):
                 continue
 
-            if bits[0] == u'export':
+            if bits[0] == b('export'):
                 bits.pop(0)
 
-            key, sep, value = str_partition(u' '.join(bits), u'=')
+            key, sep, value = bytes_partition(b(' ').join(bits), b('='))
             if key and sep:
                 yield key, value
 
     def _on_file_changed(self):
         LOG.debug('%r: file changed, reloading', self)
         for key, value in self._load():
-            if key in os.environ:
+            if key in self.environ:
                 LOG.debug('%r: existing key %r=%r exists, not setting %r',
-                          self, key, os.environ[key], value)
+                          self, key, self.environ[key], value)
             else:
                 LOG.debug('%r: setting key %r to %r', self, key, value)
                 self._keys.append(key)
-                os.environ[key] = value
+                self.environ[key] = value
 
     def _remove_existing(self):
         """
         When a change is detected, remove keys that existed in the old file.
         """
         for key in self._keys:
-            if key in os.environ:
+            if key in self.environ:
                 LOG.debug('%r: removing old key %r', self, key)
-                del os.environ[key]
+                del self.environ[key]
         self._keys = []
 
     def check(self):

docs/changelog.rst

@@ -49,11 +49,12 @@ Enhancements
 * `#419 <https://github.com/dw/mitogen/issues/419>`_: 2 network round-trips
   were removed from early connection setup.
 
-* `? <https://github.com/dw/mitogen/commit/7ae926b3>`_,
-  `? <https://github.com/dw/mitogen/commit/7ae926b3>`_,
-  `? <https://github.com/dw/mitogen/commit/7ae926b3>`_,
-  `? <https://github.com/dw/mitogen/commit/7ae926b3>`_: locking is avoided in
-  some hot paths, and locks that must be taken are held for less time.
+* `d6faff06 <https://github.com/dw/mitogen/commit/d6faff06>`_,
+  `807cbef9 <https://github.com/dw/mitogen/commit/807cbef9>`_,
+  `e93762b3 <https://github.com/dw/mitogen/commit/e93762b3>`_,
+  `50bfe4c7 <https://github.com/dw/mitogen/commit/50bfe4c7>`_: locking is
+  avoided on hot paths, and some locks are released earlier, before waking a
+  thread that must immediately take the same lock.
 
 
 Mitogen for Ansible
@@ -88,6 +89,11 @@ Mitogen for Ansible
   ``mitogen_ssh_keepalive_count`` variables, and the default timeout for an SSH
   server has been increased from `15*3` seconds to `30*10` seconds.
 
+* `#600 <https://github.com/dw/mitogen/issues/600>`_: functionality to reflect
+  changes to ``/etc/environment`` in the running interpreter did not account
+  for Unicode file contents. Now the file may contain data in any single byte
+  encoding.
+
 * `7ae926b3 <https://github.com/dw/mitogen/commit/7ae926b3>`_: the
   ``lineinfile`` module began leaking writable temporary file descriptors since
   Ansible 2.7.0. When ``lineinfile`` was used to create or modify a script, and
@@ -136,6 +142,9 @@ Core Library
   `closed` flag, preventing historical bugs where a double close could destroy
   descriptors belonging to unrelated streams.
 
+* `#606 <https://github.com/dw/mitogen/issues/606>`_: fix example code on the
+  documentation front page.
+
 * `a5536c35 <https://github.com/dw/mitogen/commit/a5536c35>`_: avoid quadratic
   buffer management when logging lines received from a child's redirected
   standard IO.
@@ -150,12 +159,14 @@ bug reports, testing, features and fixes in this release contributed by
 `Anton Markelov <https://github.com/strangeman>`_,
 `Nigel Metheringham <https://github.com/nigelm>`_,
 `Orion Poplawski <https://github.com/opoplawski>`_,
+`Pieter Voet <https://github.com/pietervoet/>`_,
+`Stefane Fermigier <https://github.com/sfermigier>`_,
 `Szabó Dániel Ernő <https://github.com/r3ap3rpy>`_,
 `Ulrich Schreiner <https://github.com/ulrichSchreiner>`_,
 `Yuki Nishida <https://github.com/yuki-nishida-exa>`_,
 `@ghp-rr <https://github.com/ghp-rr>`_,
-`Pieter Voet <https://github.com/pietervoet/>`_, and
-`@rizzly <https://github.com/rizzly>`_.
+`@rizzly <https://github.com/rizzly>`_, and
+`@tho86 <https://github.com/tho86>`_.
 
 
 v0.2.7 (2019-05-19)

docs/getting_started.rst

@@ -265,7 +265,7 @@ We must therefore continue by writing our code as a script::
         print(local.call(my_first_function))
 
     if __name__ == '__main__':
-        mitogen.utils.log_to_file(main)
+        mitogen.utils.log_to_file("mitogen.log")
         mitogen.utils.run_with_router(main)
 
 Let's try running it:

docs/index.rst

@@ -329,36 +329,7 @@ External contexts are configured such that any attempt to execute a function
 from the main Python script will correctly cause that script to be imported as
 usual into the slave process.
 
-.. code-block:: python
-
-    #!/usr/bin/env python
-    """
-    Install our application on a remote machine.
-
-    Usage:
-        install_app.py <hostname>
-
-    Where:
-        <hostname>  Hostname to install to.
-    """
-    import os
-    import sys
-
-    import mitogen
-
-
-    def install_app():
-        os.system('tar zxvf my_app.tar.gz')
-
-
-    @mitogen.main()
-    def main(broker):
-        if len(sys.argv) != 2:
-            print(__doc__)
-            sys.exit(1)
-
-        context = mitogen.ssh.connect(broker, sys.argv[1])
-        context.call(install_app)
+.. literalinclude:: ../examples/install_app.py
 
 
 Event-driven IO

examples/install_app.py

@@ -0,0 +1,28 @@
+#!/usr/bin/env python
+"""
+Install our application on a remote machine.
+
+Usage:
+    install_app.py <hostname>
+
+Where:
+    <hostname>  Hostname to install to.
+"""
+import os
+import sys
+
+import mitogen
+
+
+def install_app():
+    os.system('tar zxvf my_app.tar.gz')
+
+
+@mitogen.main()
+def main(router):
+    if len(sys.argv) != 2:
+        print(__doc__)
+        sys.exit(1)
+
+    context = router.ssh(hostname=sys.argv[1])
+    context.call(install_app)

tests/ansible/tests/env_file_watcher_test.py

@@ -0,0 +1,74 @@
+import os
+import sys
+import tempfile
+
+import mock
+import unittest2
+import testlib
+
+from mitogen.core import b
+import ansible_mitogen.runner
+
+
+klass = ansible_mitogen.runner.EnvironmentFileWatcher
+environb = getattr(os, 'environb', os.environ)
+
+
+class WatcherTest(testlib.TestCase):
+    def setUp(self):
+        self.original_env = environb.copy()
+        self.tf = tempfile.NamedTemporaryFile()
+
+    def tearDown(self):
+        self.tf.close()
+        environb.clear()
+        environb.update(self.original_env)
+
+    def test_missing_file(self):
+        # just ensure it doesn't crash
+        watcher = klass('/nonexistent')
+        watcher.check()
+
+    def test_file_becomes_missing(self):
+        # just ensure it doesn't crash
+        watcher = klass(self.tf.name)
+        watcher.check()
+        os.unlink(self.tf.name)
+        watcher.check()
+        open(self.tf.name,'wb').close()
+
+    def test_key_deleted(self):
+        environb[b('SOMEKEY')] = b('123')
+        self.tf.write(b('SOMEKEY=123\n'))
+        self.tf.flush()
+        watcher = klass(self.tf.name)
+        self.tf.seek(0)
+        self.tf.truncate(0)
+        watcher.check()
+        self.assertTrue(b('SOMEKEY') not in environb)
+
+    def test_key_added(self):
+        watcher = klass(self.tf.name)
+        self.tf.write(b('SOMEKEY=123\n'))
+        self.tf.flush()
+        watcher.check()
+        self.assertEqual(environb[b('SOMEKEY')], b('123'))
+
+    def test_key_shadowed_nuchange(self):
+        environb[b('SOMEKEY')] = b('234')
+        self.tf.write(b('SOMEKEY=123\n'))
+        self.tf.flush()
+        watcher = klass(self.tf.name)
+        watcher.check()
+        self.assertEqual(environb[b('SOMEKEY')], b('234'))
+
+    def test_binary_key_added(self):
+        watcher = klass(self.tf.name)
+        self.tf.write(b('SOMEKEY=\xff\xff\xff\n'))
+        self.tf.flush()
+        watcher.check()
+        self.assertEqual(environb[b('SOMEKEY')], b('\xff\xff\xff'))
+
+
+if __name__ == '__main__':
+    unittest2.main()

tests/image_prep/_container_setup.yml

@@ -91,6 +91,11 @@
       shell: locale-gen
       when: distro == "Debian"
 
+    - name: Write Unicode into /etc/environment
+      copy:
+        dest: /etc/environment
+        content: "UNICODE_SNOWMAN=\u2603\n"
+
     - name: Install prebuilt 'doas' binary
       unarchive:
         dest: /