Merge pull request #48 from heroku-python/python3

Cleanup for Python 3 compat

.travis.yml

@@ -14,9 +14,3 @@ script:
   # TODO: Replace with an actual test suite:
   # https://github.com/kennethreitz/bob-builder/issues/31
   - bob --help
-matrix:
-  allow_failures:
-    - python: "3.4"
-    - python: "3.5"
-    - python: "3.6"
-  fast_finish: true

bob/cli.py

@@ -13,8 +13,8 @@
 Configuration:
     Environment Variables: AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, S3_BUCKET, S3_PREFIX (optional), UPSTREAM_S3_BUCKET (optional), UPSTREAM_S3_PREFIX (optional)
 """
-from __future__ import print_function
-
+import os
+import signal
 import sys
 
 from docopt import docopt
@@ -28,7 +28,7 @@ def build(formula, name=None):
     try:
         assert f.exists
     except AssertionError:
-        print_stderr("Formula {} doesn't exist.".format(formula))
+        print_stderr("Formula {} doesn't exist.".format(formula), title='ERROR')
         sys.exit(1)
 
     # CLI lies ahead.
@@ -40,10 +40,10 @@ def build(formula, name=None):
 def deploy(formula, overwrite, name):
     f = build(formula, name)
 
-    print('Archiving.')
+    print_stderr('Archiving.')
     f.archive()
 
-    print('Deploying.')
+    print_stderr('Deploying.')
     f.deploy(allow_overwrite=overwrite)
 
 
@@ -63,9 +63,18 @@ def main():
         deploy(formula, overwrite=do_overwrite, name=do_name)
 
 
+def sigint_handler(signo, frame):
+    # when receiving a signal, a process must kill itself using the same signal
+    # sys.exit()ing 0, 1, 130, whatever will not signal to the calling program that we terminated in response to the signal
+    # best example: `for f in a b c; do bob deploy $f; done`, hitting Ctrl+C should interrupt Bob and stop the bash loop
+    # that's only possible if Bash knows that we exited in response to Ctrl+C (=SIGINT), then it'll also terminate the loop
+    # bash will report the exit status as 128+$signal, so 130 for SIGINT, but sys.exit(130) does not to the same thing - the value of 130 is simply bash's representation
+    # killing ourselves with the signal number that we are aborting in response to does all this correctly, and bash will see the right WIFSIGNALED() status of our program, not WIFEXITED()
+
+    # and finally, before we send ourselves the right signal, we must first restore the handler for it to the default
+    signal.signal(signo, signal.SIG_DFL)
+    os.kill(os.getpid(), signo)
+
 def dispatch():
-    try:
-        main()
-    except KeyboardInterrupt:
-        print('ool.')
-        sys.exit(130)
+    signal.signal(signal.SIGINT, sigint_handler)
+    main()

bob/models.py

@@ -1,16 +1,16 @@
 # -*- coding: utf-8 -*-
 
-from __future__ import print_function
-
 import os
 import re
 import shutil
+import signal
 import sys
 from tempfile import mkstemp, mkdtemp
+from subprocess import Popen
 
 from .utils import (
     archive_tree, extract_tree, get_with_wildcard, iter_marker_lines, mkdir_p,
-    pipe, print_stderr, process, S3ConnectionHandler)
+    print_stderr, S3ConnectionHandler)
 
 
 WORKSPACE = os.environ.get('WORKSPACE_DIR', 'workspace')
@@ -29,11 +29,6 @@
 DEPS_MARKER = '# Build Deps: '
 BUILD_PATH_MARKER = '# Build Path: '
 
-# Make stdin/out as unbuffered as possible via file descriptor modes.
-sys.stdout = os.fdopen(sys.stdout.fileno(), 'w', 0)
-sys.stderr = os.fdopen(sys.stderr.fileno(), 'w', 0)
-
-
 class Formula(object):
 
     def __init__(self, path, override_path=None):
@@ -42,7 +37,7 @@ def __init__(self, path, override_path=None):
         self.override_path = override_path
 
         if not S3_BUCKET:
-            print_stderr('The environment variable S3_BUCKET must be set to the bucket name.')
+            print_stderr('The environment variable S3_BUCKET must be set to the bucket name.', title='ERROR')
             sys.exit(1)
 
         s3 = S3ConnectionHandler()
@@ -95,22 +90,22 @@ def resolve_deps(self):
         deps = self.depends_on
 
         if deps:
-            print('Fetching dependencies... found {}:'.format(len(deps)))
+            print_stderr('Fetching dependencies... found {}:'.format(len(deps)))
 
             for dep in deps:
-                print('  - {}'.format(dep))
+                print_stderr('  - {}'.format(dep))
 
                 key_name = '{}{}.tar.gz'.format(S3_PREFIX, dep)
                 key = get_with_wildcard(self.bucket, key_name)
 
                 if not key and self.upstream:
-                    print('    Not found in S3_BUCKET, trying UPSTREAM_S3_BUCKET...')
+                    print_stderr('    Not found in S3_BUCKET, trying UPSTREAM_S3_BUCKET...')
                     key_name = '{}{}.tar.gz'.format(UPSTREAM_S3_PREFIX, dep)
                     key = get_with_wildcard(self.upstream, key_name)
 
                 if not key:
                     print_stderr('Archive {} does not exist.\n'
-                                 'Please deploy it to continue.'.format(key_name))
+                                 'Please deploy it to continue.'.format(key_name), title='ERROR')
                     sys.exit(1)
 
                 # Grab the Dep from S3, download it to a temp file.
@@ -120,7 +115,7 @@ def resolve_deps(self):
                 # Extract the Dep to the appropriate location.
                 extract_tree(archive, self.build_path)
 
-            print()
+            print_stderr()
 
     def build(self):
         # Prepare build directory.
@@ -133,38 +128,46 @@ def build(self):
         # Temporary directory where work will be carried out, because of David.
         cwd_path = mkdtemp(prefix='bob-')
 
-        print('Building formula {} in {}:\n'.format(self.path, cwd_path))
+        print_stderr('Building formula {} in {}:\n'.format(self.path, cwd_path))
 
         # Execute the formula script.
         args = ["/usr/bin/env", "bash", "--", self.full_path, self.build_path]
         if self.override_path != None:
             args.append(self.override_path)
 
-        p = process(args, cwd=cwd_path)
+        p = Popen(args, cwd=cwd_path, shell=False, stderr=sys.stdout.fileno()) # we have to pass sys.stdout.fileno(), because subprocess.STDOUT will not do what we want on older versions: https://bugs.python.org/issue22274
 
-        pipe(p.stdout, sys.stdout, indent=True)
         p.wait()
 
-        if p.returncode != 0:
-            print_stderr('Formula exited with return code {}.'.format(p.returncode))
+        if p.returncode > 0:
+            print_stderr('Formula exited with return code {}.'.format(p.returncode), title='ERROR')
             sys.exit(1)
-
-        print('\nBuild complete: {}'.format(self.build_path))
+        elif p.returncode < 0: # script was terminated by signal number abs(returncode)
+            signum = abs(p.returncode)
+            try:
+                # Python 3.5+
+                signame = signal.Signals(signum).name
+            except AttributeError:
+                signame = signum
+            print_stderr('Formula terminated by signal {}.'.format(signame), title='ERROR')
+            sys.exit(128+signum) # best we can do, given how we weren't terminated ourselves with the same signal (maybe we're PID 1, maybe another reason)
+
+        print_stderr('\nBuild complete: {}'.format(self.build_path))
 
     def archive(self):
         """Archives the build directory as a tar.gz."""
         archive = mkstemp(prefix='bob-build-', suffix='.tar.gz')[1]
         archive_tree(self.build_path, archive)
 
-        print('Created: {}'.format(archive))
+        print_stderr('Created: {}'.format(archive))
         self.archived_path = archive
 
     def deploy(self, allow_overwrite=False):
         """Deploys the formula's archive to S3."""
         assert self.archived_path
 
         if self.bucket.connection.anon:
-            print_stderr('Deploy requires valid AWS credentials.')
+            print_stderr('Deploy requires valid AWS credentials.', title='ERROR')
             sys.exit(1)
 
         if self.override_path != None:
@@ -179,16 +182,16 @@ def deploy(self, allow_overwrite=False):
         if key:
             if not allow_overwrite:
                 print_stderr('Archive {} already exists.\n'
-                             'Use the --overwrite flag to continue.'.format(key_name))
+                             'Use the --overwrite flag to continue.'.format(key_name), title='ERROR')
                 sys.exit(1)
         else:
             key = self.bucket.new_key(key_name)
 
         url = key.generate_url(0, query_auth=False)
-        print('Uploading to: {}'.format(url))
+        print_stderr('Uploading to: {}'.format(url))
 
         # Upload the archive, set permissions.
         key.set_contents_from_filename(self.archived_path)
         key.set_acl('public-read')
 
-        print('Upload complete!')
+        print_stderr('Upload complete!')

bob/utils.py

@@ -6,16 +6,15 @@
 import os
 import sys
 import tarfile
-from subprocess import Popen, PIPE, STDOUT
 
 import boto
 from boto.exception import NoAuthHandlerFound, S3ResponseError
 
 from distutils.version import LooseVersion
 from fnmatch import fnmatchcase
 
-def print_stderr(message, prefix='ERROR'):
-    print('\n{}: {}\n'.format(prefix, message), file=sys.stderr)
+def print_stderr(message='', title=''):
+    print(('\n{1}: {0}\n' if title else '{0}').format(message, title), file=sys.stderr)
 
 
 def iter_marker_lines(marker, formula, strip=True):
@@ -42,23 +41,6 @@ def mkdir_p(path):
             raise
 
 
-def process(cmd, cwd=None):
-    """A simple wrapper around the subprocess module; stderr is redirected to stdout."""
-    p = Popen(cmd, cwd=cwd, shell=False, stdout=PIPE, stderr=STDOUT)
-    return p
-
-
-def pipe(a, b, indent=True):
-    """Pipes stream A to stream B, with optional indentation."""
-
-    for line in iter(a.readline, b''):
-
-        if indent:
-            b.write('    ')
-
-        b.write(line)
-
-
 def archive_tree(dir, archive):
     """Creates a tar.gz archive from a given directory."""
     with tarfile.open(archive, 'w:gz') as tar:
@@ -104,16 +86,16 @@ def __init__(self):
             self.s3 = boto.connect_s3()
         except NoAuthHandlerFound:
             print_stderr('No AWS credentials found. Requests will be made without authentication.',
-                         prefix='WARNING')
+                         title='WARNING')
             self.s3 = boto.connect_s3(anon=True)
 
     def get_bucket(self, name):
         try:
             return self.s3.get_bucket(name)
         except S3ResponseError as e:
             if e.status == 403 and not self.s3.anon:
-                print('Access denied for bucket "{}" using found credentials. '
-                      'Retrying as an anonymous user.'.format(name))
+                print_stderr('Access denied for bucket "{}" using found credentials. '
+                             'Retrying as an anonymous user.'.format(name), title='NOTICE')
                 if not hasattr(self, 's3_anon'):
                     self.s3_anon = boto.connect_s3(anon=True)
                 return self.s3_anon.get_bucket(name)

setup.py

@@ -10,7 +10,7 @@
 
 setup(
     name='bob-builder',
-    version='0.0.17',
+    version='0.0.18',
     install_requires=deps,
     description='Binary Build Toolkit.',
     # long_description='Meh.',/