Merge remote-tracking branch 'upstream/master' into move-changelog

.travis.yml

@@ -54,7 +54,8 @@ env:
   - REPO_TYPE=nix
   - REPO_TYPE=dockerfile
   - REPO_TYPE=external/*
-  - REPO_TYPE=**/*.py
+  - REPO_TYPE=contentproviders/*.py
+  - REPO_TYPE=test_args.py
   global:
   - secure: gX7IOkbjlvcDwIH24sOLhutINx6TZRwujEusMWh1dqgYG2D69qQai/mTrRXO9PGRrsvQwIBk4RcILKAiZnk5O2Z1hLoIHk/oU2mNUmE44dDm4Xf/VTTdeYhjeOTR9B+KJ9NVwPxuSEDSND3lD7yFfvCqNXykipEhBtTliLupjWVxxXnaz0aZTYHUPJwanxdUc06AphSPwZjtm1m3qMUU8v7UdTGGAdW3NlgkKw0Xx2x5W31fW676vskC/GNQAbcRociYipuhSFWV4lu+6d8XF2xVO97xtzf54tBQzt6RgVfAKtiqkEIYSzJQBBpkQ6SM6yg+fQoQpOo8jPU9ZBjvaoopUG9vn8HRS/OtQrDcG3kEFnFAnaes8Iqtidp1deTn27LIlfCTl7kTFOp8yaaNlIMHJTJKTEMRhfdDlBYx7qiH8e9d/z37lupzY2loLHeNHdMRS1uYsfacZsmrnu9vAdpQmP1LuHivBPZEvgerinADaJiekelWOIEn956pDrno/YgnzP0i9LEBYnbbunqT8oEzLintNt5CXGdhkiG60j38McKCIn4sD6jbMMwgsqVFdClCBersyorKhOs7P8at5vX4xf8fMiKPC8LZPzYVIQYzCjmwSOFQ+Rzmz5gSj+DRTANKfHpzZCKZEF6amBYMGE1O5osF8m6M10vtW9ToK+s=
   - secure: Cfhb0BUT54JjEZD8n44Jj+o1lt5p32Lfg7W/euTyZ61YylDx0+XEYTzfWcwxOzH9fLpWr6dDrBMGHA/FPqsWA5BkoGdiBJ1OOVy2tmDRButctobWM3SVwa+Rhh8bZWlK8yKT2S3n6CtK4mesmjzdbUShL7YnKOSl8LBaTT5Y5oT8Oxsq51pfg8fJUImim8H20t8H7emaEzZorF4OSGRtajcAgukt5YoAqTEVDq+bFRBHZalxkcRqLhsGe3CCWa28kjGTL4MPZpCI6/AXIXHzihfG3rGq40ZT8jZ9GPP3MBgkiJWtFiTC9h16G34b/JI/TD40zCmoW9/9oVjRK4UlLGCAv6bgzFhCRof2abhB9NTZDniNzkO0T15uHs3VLbLCPYB0xYyClAFxm2P6e8WPChyENKfTNh+803IKFFo4JaTjOnKzi89N72v5+bT6ghP932nmjJr1AO65xjw63CeDmaLoHDY73n11DibybWQgEeiNzJuSzbIHyqMPhW5XqeroEjKKstdPHtVfOViI9ywjEMy0HCPsspaVI7Aow0Iv8E4Ajvd32W7z0h0fSCx/i25hEOAo2vhBsmQKJA7IquB3N88M11L874h/8J+oc/osW1EB5z7Ukke5YCq94Qh3qImSIhJULXMMc1QjEqYsqhLXtiMG2HUge0Y5hwwnnbEIRMQ=

docs/source/config_files.rst

@@ -113,13 +113,13 @@ used for your R installation.
 It is powered by the open file format `Dar <https://github.com/substance/dar>`_.
 
 If your repository contains a Stencila document, repo2docker detects it based on the file ``manifest.xml``.
-The required `execution contexts <https://stenci.la/learn/intro.html>` are extracted from a Dar article (i.e.
+The required `execution contexts <https://stenci.la/learn/intro.html>`_ are extracted from a Dar article (i.e.
 files named ``*.jats.xml``).
 
 You may also have a ``runtime.txt`` and/or an ``install.R`` to manually configure your R installation.
 
 To see example repositories, visit our
-`Stencila with R <https://github.com/binder-examples/stencila-r/>`_ and 
+`Stencila with R <https://github.com/binder-examples/stencila-r/>`_ and
 `Stencila with Python <https://github.com/binder-examples/stencila-py>`_ examples.
 
 .. _postBuild:
@@ -141,9 +141,9 @@ their demo for binder <https://github.com/jupyterlab/jupyterlab-demo/blob/master
 ====================================================
 
 A script that can contain simple commands to be run at runtime (as an
-`ENTRYPOINT <https://docs.docker.com/engine/reference/builder/#entrypoint>`
+`ENTRYPOINT <https://docs.docker.com/engine/reference/builder/#entrypoint>`_
 to the docker container). If you want this to be a shell script, make sure the
-first line is ```#!/bin/bash``. The last line must be ```exec "$@"```
+first line is ``#!/bin/bash``. The last line must be ``exec "$@"``
 equivalent.
 
 Use this to set environment variables that software installed in your container

docs/source/contributing/contributing.md

@@ -18,8 +18,8 @@ repo2docker merged.
 
 ## Guidelines to getting a Pull Request merged
 
-These are not hard rules to be enforced by :police_car: but instead guidelines
+These are not hard rules to be enforced by 🚓 but instead guidelines
-to help you make the most effictive / efficient contribution.
+to help you make a contribution.
 
 * prefix the title of your pull request with `[MRG]` if the contribution
   is complete and should be subjected to a detailed review;
@@ -31,6 +31,7 @@ to help you make the most effictive / efficient contribution.
 * describe why you are proposing the changes you are proposing;
 * try to not rush changes (the definition of rush depends on how big your
   changes are);
+* Enter your changes into the [change log](https://github.com/jupyter/repo2docker/blob/master/CHANGES.rst);
 * someone else has to merge your PR;
 * new code needs to come with a test;
 * apply [PEP8](https://www.python.org/dev/peps/pep-0008/) as much
@@ -67,7 +68,7 @@ make that your current directory with `cd repo2docker`.
 
 After cloning the repository (or your fork of the repository), you should set up an
 isolated environment to install libraries required for running / developing
-repo2docker. 
+repo2docker.
 
 There are many ways to do this but here we present you with two approaches: `virtual environment` or `pipenv`.
 
@@ -85,7 +86,7 @@ This should install all the libraries required for testing & running repo2docker
 
 - Using `pipenv`
 
-Note that you will need to install pipenv first using `pip3 install pipenv`. 
+Note that you will need to install pipenv first using `pip3 install pipenv`.
 Then from the root directory of this project you can use the following commands:
 
 ```bash
@@ -94,7 +95,7 @@ pipenv install --dev
 
 This should install both the dev and docs requirements at once!
 
-### Set up 
+### Set up
 
 ### Verify that docker is installed and running
 
@@ -129,7 +130,3 @@ Server:
 ```
 
 Then you are good to go!
-
-
-
-

repo2docker/__main__.py

@@ -1,11 +1,335 @@
+import argparse
+import sys
+import os
+import logging
+import docker
 from .app import Repo2Docker
+from . import __version__
+from .utils import validate_and_generate_port_mapping, is_valid_docker_image_name
+
+def validate_image_name(image_name):
+    """
+    Validate image_name read by argparse
+
+    Note: Container names must start with an alphanumeric character and
+    can then use _ . or - in addition to alphanumeric.
+    [a-zA-Z0-9][a-zA-Z0-9_.-]+
+
+    Args:
+        image_name (string): argument read by the argument parser
+
+    Returns:
+        unmodified image_name
+
+    Raises:
+        ArgumentTypeError: if image_name contains characters that do not
+                            meet the logic that container names must start
+                            with an alphanumeric character and can then
+                            use _ . or - in addition to alphanumeric.
+                            [a-zA-Z0-9][a-zA-Z0-9_.-]+
+    """
+    if not is_valid_docker_image_name(image_name):
+        msg = ("%r is not a valid docker image name. Image name"
+                "must start with an alphanumeric character and"
+                "can then use _ . or - in addition to alphanumeric." % image_name)
+        raise argparse.ArgumentTypeError(msg)
+    return image_name
+
+def get_argparser():
+    """Get arguments that may be used by repo2docker"""
+    argparser = argparse.ArgumentParser()
+
+    argparser.add_argument(
+        '--config',
+        default='repo2docker_config.py',
+        help="Path to config file for repo2docker"
+    )
+
+    argparser.add_argument(
+        '--json-logs',
+        default=False,
+        action='store_true',
+        help='Emit JSON logs instead of human readable logs'
+    )
+
+    argparser.add_argument(
+        'repo',
+        help=('Path to repository that should be built. Could be '
+                'local path or a git URL.')
+    )
+
+    argparser.add_argument(
+        '--image-name',
+        help=('Name of image to be built. If unspecified will be '
+                'autogenerated'),
+        type=validate_image_name
+    )
+
+    argparser.add_argument(
+        '--ref',
+        help=('If building a git url, which reference to check out. '
+                'E.g., `master`.')
+    )
+
+    argparser.add_argument(
+        '--debug',
+        help="Turn on debug logging",
+        action='store_true',
+    )
+
+    argparser.add_argument(
+        '--no-build',
+        dest='build',
+        action='store_false',
+        help=('Do not actually build the image. Useful in conjunction '
+                'with --debug.')
+    )
+
+    argparser.add_argument(
+        '--build-memory-limit',
+        help='Total Memory that can be used by the docker build process'
+    )
+
+    argparser.add_argument(
+        'cmd',
+        nargs=argparse.REMAINDER,
+        help='Custom command to run after building container'
+    )
+
+    argparser.add_argument(
+        '--no-run',
+        dest='run',
+        action='store_false',
+        help='Do not run container after it has been built'
+    )
+
+    argparser.add_argument(
+        '--publish', '-p',
+        dest='ports',
+        action='append',
+        help=('Specify port mappings for the image. Needs a command to '
+                'run in the container.')
+    )
+
+    argparser.add_argument(
+        '--publish-all', '-P',
+        dest='all_ports',
+        action='store_true',
+        help='Publish all exposed ports to random host ports.'
+    )
+
+    argparser.add_argument(
+        '--no-clean',
+        dest='clean',
+        action='store_false',
+        help="Don't clean up remote checkouts after we are done"
+    )
+
+    argparser.add_argument(
+        '--push',
+        dest='push',
+        action='store_true',
+        help='Push docker image to repository'
+    )
+
+    argparser.add_argument(
+        '--volume', '-v',
+        dest='volumes',
+        action='append',
+        help='Volumes to mount inside the container, in form src:dest',
+        default=[]
+    )
+
+    argparser.add_argument(
+        '--user-id',
+        help='User ID of the primary user in the image',
+        type=int
+    )
+
+    argparser.add_argument(
+        '--user-name',
+        help='Username of the primary user in the image',
+    )
+
+    argparser.add_argument(
+        '--env', '-e',
+        dest='environment',
+        action='append',
+        help='Environment variables to define at container run time',
+        default=[]
+    )
+
+    argparser.add_argument(
+        '--editable', '-E',
+        dest='editable',
+        action='store_true',
+        help='Use the local repository in edit mode',
+    )
+
+    argparser.add_argument(
+        '--appendix',
+        type=str,
+        #help=self.traits()['appendix'].help,
+    )
+
+    argparser.add_argument(
+        '--subdir',
+        type=str,
+        #help=self.traits()['subdir'].help,
+    )
+
+    argparser.add_argument(
+        '--version',
+        dest='version',
+        action='store_true',
+        help='Print the repo2docker version and exit.'
+    )
+
+    argparser.add_argument(
+        '--cache-from',
+        action='append',
+        default=[],
+        #help=self.traits()['cache_from'].help
+    )
+
+    return argparser
+
+
+def make_r2d(argv=None):
+    if argv is None:
+        argv = sys.argv[1:]
+
+
+    # version must be checked before parse, as repo/cmd are required and
+    # will spit out an error if allowed to be parsed first.
+    if '--version' in argv:
+        print(__version__)
+        sys.exit(0)
+
+    args = get_argparser().parse_args(argv)
+
+    r2d = Repo2Docker()
+
+    if args.debug:
+        r2d.log_level = logging.DEBUG
+
+    r2d.load_config_file(args.config)
+    if args.appendix:
+        r2d.appendix = args.appendix
+
+    r2d.repo = args.repo
+    r2d.ref = args.ref
+
+    # user wants to mount a local directory into the container for
+    # editing
+    if args.editable:
+        # the user has to point at a directory, not just a path for us
+        # to be able to mount it. We might have content providers that can
+        # provide content from a local `something.zip` file, which we
+        # couldn't mount in editable mode
+        if os.path.isdir(args.repo):
+            r2d.volumes[os.path.abspath(args.repo)] = '.'
+        else:
+            r2d.log.error('Can not mount "{}" in editable mode '
+                            'as it is not a directory'.format(args.repo),
+                            extra=dict(phase='failed'))
+            sys.exit(1)
+
+
+    if args.image_name:
+        r2d.output_image_spec = args.image_name
+
+    r2d.json_logs = args.json_logs
+
+    r2d.dry_run = not args.build
+
+    if r2d.dry_run:
+        # Can't push nor run if we aren't building
+        args.run = False
+        args.push = False
+
+    r2d.run = args.run
+    r2d.push = args.push
+
+    # check against r2d.run and not args.run as r2d.run is false on
+    # --no-build. Also r2d.volumes and not args.volumes since --editable
+    # modified r2d.volumes
+    if r2d.volumes and not r2d.run:
+        # Can't mount if we aren't running
+        print('To Mount volumes with -v, you also need to run the '
+                'container')
+        sys.exit(1)
+
+    for v in args.volumes:
+        src, dest = v.split(':')
+        r2d.volumes[src] = dest
+
+    r2d.run_cmd = args.cmd
+
+    if args.all_ports and not r2d.run:
+        print('To publish user defined port mappings, the container must '
+                'also be run')
+        sys.exit(1)
+
+    if args.ports and not r2d.run:
+        print('To publish user defined port mappings, the container must '
+                'also be run')
+        sys.exit(1)
+
+    if args.ports and not r2d.run_cmd:
+        print('To publish user defined port mapping, user must specify '
+                'the command to run in the container')
+        sys.exit(1)
+
+    r2d.ports = validate_and_generate_port_mapping(args.ports)
+    r2d.all_ports = args.all_ports
+
+    if args.user_id:
+        r2d.user_id = args.user_id
+    if args.user_name:
+        r2d.user_name = args.user_name
+
+    if args.build_memory_limit:
+        r2d.build_memory_limit = args.build_memory_limit
+
+    if args.environment and not r2d.run:
+        print('To specify environment variables, you also need to run '
+                'the container')
+        sys.exit(1)
+
+    if args.subdir:
+        r2d.subdir = args.subdir
+
+    if args.cache_from:
+        r2d.cache_from = args.cache_from
+
+    r2d.environment = args.environment
+
+    # if the source exists locally we don't want to delete it at the end
+    # FIXME: Find a better way to figure out if repo is 'local'. Push this into ContentProvider?
+    if os.path.exists(args.repo):
+        r2d.cleanup_checkout = False
+    else:
+        r2d.cleanup_checkout = args.clean
+
+    return r2d
 
 
 def main():
-    f = Repo2Docker()
+    r2d = make_r2d()
-    f.initialize()
+    r2d.initialize()
-    f.start()
+    try:
-
+        r2d.start()
+    except docker.errors.BuildError as e:
+        # This is only raised by us
+        if r2d.debug:
+            r2d.log.exception(e)
+        sys.exit(1)
+    except docker.errors.ImageLoadError as e:
+        # This is only raised by us
+        if r2d.debug:
+            r2d.log.exception(e)
+        sys.exit(1)
 
 if __name__ == '__main__':
     main()

repo2docker/app.py

@@ -25,7 +25,7 @@ from docker.errors import DockerException
 import escapism
 from pythonjsonlogger import jsonlogger
 
-from traitlets import Any, Dict, Int,  List, Unicode, default
+from traitlets import Any, Dict, Int,  List, Unicode, Bool, default
 from traitlets.config import Application
 
 from . import __version__
@@ -206,6 +206,127 @@ class Repo2Docker(Application):
         """
     )
 
+    json_logs = Bool(
+        False,
+        help="""
+        Log output in structured JSON format.
+
+        Useful when stdout is consumed by other tools
+        """,
+        config=True
+    )
+
+    repo = Unicode(
+        ".",
+        help="""
+        Specification of repository to build image for.
+
+        Could be local path or git URL.
+        """,
+        config=True
+    )
+
+    ref = Unicode(
+        None,
+        help="""
+        Git ref that should be built.
+
+        If repo is a git repository, this ref is checked out
+        in a local clone before repository is built.
+        """,
+        config=True,
+        allow_none=True
+    )
+
+    cleanup_checkout = Bool(
+        False,
+        help="""
+        Delete source repository after building is done.
+
+        Useful when repo2docker is doing the git cloning
+        """,
+        config=True
+    )
+
+    output_image_spec = Unicode(
+        "",
+        help="""
+        Docker Image name:tag to tag the built image with.
+
+        Required parameter.
+        """,
+        config=True
+    )
+
+    push = Bool(
+        False,
+        help="""
+        Set to true to push docker image after building
+        """,
+        config=True
+    )
+
+    run = Bool(
+        False,
+        help="""
+        Run docker image after building
+        """,
+        config=True
+    )
+
+    # FIXME: Refactor class to be able to do --no-build without needing
+    #        deep support for it inside other code
+    dry_run = Bool(
+        False,
+        help="""
+        Do not actually build the docker image, just simulate it.
+        """,
+        config=True
+    )
+
+    # FIXME: Refactor classes to separate build & run steps
+    run_cmd = List(
+        [],
+        help="""
+        Command to run when running the container
+
+        When left empty, a jupyter notebook is run.
+        """,
+        config=True
+    )
+
+    all_ports = Bool(
+        False,
+        help="""
+        Publish all declared ports from container whiel running.
+
+        Equivalent to -P option to docker run
+        """,
+        config=True
+    )
+
+    ports = Dict(
+        {},
+        help="""
+        Port mappings to establish when running the container.
+
+        Equivalent to -p {key}:{value} options to docker run.
+        {key} refers to port inside container, and {value}
+        refers to port / host:port in the host
+        """,
+        config=True
+    )
+
+    environment = List(
+        [],
+        help="""
+        Environment variables to set when running the built image.
+
+        Each item must be a string formatted as KEY=VALUE
+        """,
+        config=True
+    )
+
     def fetch(self, url, ref, checkout_path):
         """Fetch the contents of `url` and place it in `checkout_path`.
 
@@ -233,192 +354,7 @@ class Repo2Docker(Application):
                 spec, checkout_path, yield_output=self.json_logs):
             self.log.info(log_line, extra=dict(phase='fetching'))
 
-    def validate_image_name(self, image_name):
-        """
-        Validate image_name read by argparse
 
-        Note: Container names must start with an alphanumeric character and
-        can then use _ . or - in addition to alphanumeric.
-        [a-zA-Z0-9][a-zA-Z0-9_.-]+
-
-        Args:
-            image_name (string): argument read by the argument parser
-
-        Returns:
-            unmodified image_name
-
-        Raises:
-            ArgumentTypeError: if image_name contains characters that do not
-                               meet the logic that container names must start
-                               with an alphanumeric character and can then
-                               use _ . or - in addition to alphanumeric.
-                               [a-zA-Z0-9][a-zA-Z0-9_.-]+
-        """
-        if not is_valid_docker_image_name(image_name):
-            msg = ("%r is not a valid docker image name. Image name"
-                   "must start with an alphanumeric character and"
-                   "can then use _ . or - in addition to alphanumeric." % image_name)
-            raise argparse.ArgumentTypeError(msg)
-        return image_name
-
-    def get_argparser(self):
-        """Get arguments that may be used by repo2docker"""
-        argparser = argparse.ArgumentParser()
-
-        argparser.add_argument(
-            '--config',
-            default='repo2docker_config.py',
-            help="Path to config file for repo2docker"
-        )
-
-        argparser.add_argument(
-            '--json-logs',
-            default=False,
-            action='store_true',
-            help='Emit JSON logs instead of human readable logs'
-        )
-
-        argparser.add_argument(
-            'repo',
-            help=('Path to repository that should be built. Could be '
-                  'local path or a git URL.')
-        )
-
-        argparser.add_argument(
-            '--image-name',
-            help=('Name of image to be built. If unspecified will be '
-                  'autogenerated'),
-            type=self.validate_image_name
-        )
-
-        argparser.add_argument(
-            '--ref',
-            help=('If building a git url, which reference to check out. '
-                  'E.g., `master`.')
-        )
-
-        argparser.add_argument(
-            '--debug',
-            help="Turn on debug logging",
-            action='store_true',
-        )
-
-        argparser.add_argument(
-            '--no-build',
-            dest='build',
-            action='store_false',
-            help=('Do not actually build the image. Useful in conjunction '
-                  'with --debug.')
-        )
-
-        argparser.add_argument(
-            '--build-memory-limit',
-            help='Total Memory that can be used by the docker build process'
-        )
-
-        argparser.add_argument(
-            'cmd',
-            nargs=argparse.REMAINDER,
-            help='Custom command to run after building container'
-        )
-
-        argparser.add_argument(
-            '--no-run',
-            dest='run',
-            action='store_false',
-            help='Do not run container after it has been built'
-        )
-
-        argparser.add_argument(
-            '--publish', '-p',
-            dest='ports',
-            action='append',
-            help=('Specify port mappings for the image. Needs a command to '
-                  'run in the container.')
-        )
-
-        argparser.add_argument(
-            '--publish-all', '-P',
-            dest='all_ports',
-            action='store_true',
-            help='Publish all exposed ports to random host ports.'
-        )
-
-        argparser.add_argument(
-            '--no-clean',
-            dest='clean',
-            action='store_false',
-            help="Don't clean up remote checkouts after we are done"
-        )
-
-        argparser.add_argument(
-            '--push',
-            dest='push',
-            action='store_true',
-            help='Push docker image to repository'
-        )
-
-        argparser.add_argument(
-            '--volume', '-v',
-            dest='volumes',
-            action='append',
-            help='Volumes to mount inside the container, in form src:dest',
-            default=[]
-        )
-
-        argparser.add_argument(
-            '--user-id',
-            help='User ID of the primary user in the image',
-            type=int
-        )
-
-        argparser.add_argument(
-            '--user-name',
-            help='Username of the primary user in the image',
-        )
-
-        argparser.add_argument(
-            '--env', '-e',
-            dest='environment',
-            action='append',
-            help='Environment variables to define at container run time',
-            default=[]
-        )
-
-        argparser.add_argument(
-            '--editable', '-E',
-            dest='editable',
-            action='store_true',
-            help='Use the local repository in edit mode',
-        )
-
-        argparser.add_argument(
-            '--appendix',
-            type=str,
-            help=self.traits()['appendix'].help,
-        )
-
-        argparser.add_argument(
-            '--subdir',
-            type=str,
-            help=self.traits()['subdir'].help,
-        )
-
-        argparser.add_argument(
-            '--version',
-            dest='version',
-            action='store_true',
-            help='Print the repo2docker version and exit.'
-        )
-
-        argparser.add_argument(
-            '--cache-from',
-            action='append',
-            default=[],
-            help=self.traits()['cache_from'].help
-        )
-
-        return argparser
 
     def json_excepthook(self, etype, evalue, traceback):
         """Called on an uncaught exception when using json logging
@@ -429,50 +365,10 @@ class Repo2Docker(Application):
                        exc_info=(etype, evalue, traceback),
                        extra=dict(phase='failed'))
 
-    def initialize(self, argv=None):
+    def initialize(self):
         """Init repo2docker configuration before start"""
-        if argv is None:
+        # FIXME: Remove this function, move it to setters / traitlet reactors
-            argv = sys.argv[1:]
+        if self.json_logs:
-
-        # version must be checked before parse, as repo/cmd are required and
-        # will spit out an error if allowed to be parsed first.
-        if '--version' in argv:
-            print(self.version)
-            sys.exit(0)
-
-        args = self.get_argparser().parse_args(argv)
-
-        if args.debug:
-            self.log_level = logging.DEBUG
-
-        self.load_config_file(args.config)
-        if args.appendix:
-            self.appendix = args.appendix
-
-        self.repo = args.repo
-        self.ref = args.ref
-        # if the source exists locally we don't want to delete it at the end
-        if os.path.exists(args.repo):
-            self.cleanup_checkout = False
-        else:
-            self.cleanup_checkout = args.clean
-
-        # user wants to mount a local directory into the container for
-        # editing
-        if args.editable:
-            # the user has to point at a directory, not just a path for us
-            # to be able to mount it. We might have content providers that can
-            # provide content from a local `something.zip` file, which we
-            # couldn't mount in editable mode
-            if os.path.isdir(args.repo):
-                self.volumes[os.path.abspath(args.repo)] = '.'
-            else:
-                self.log.error('Can not mount "{}" in editable mode '
-                               'as it is not a directory'.format(args.repo),
-                               extra=dict(phase='failed'))
-                sys.exit(1)
-
-        if args.json_logs:
             # register JSON excepthook to avoid non-JSON output on errors
             sys.excepthook = self.json_excepthook
             # Need to reset existing handlers, or we repeat messages
@@ -493,9 +389,7 @@ class Repo2Docker(Application):
                 fmt='%(message)s'
             )
 
-        if args.image_name:
+        if self.output_image_spec == "":
-            self.output_image_spec = args.image_name
-        else:
             # Attempt to set a sane default!
             # HACK: Provide something more descriptive?
             self.output_image_spec = (
@@ -504,68 +398,11 @@ class Repo2Docker(Application):
                 str(int(time.time()))
             )
 
-        self.push = args.push
+        if self.dry_run and (self.run or self.push):
-        self.run = args.run
+            raise ValueError("Can not push or run image if we are not building it")
-        self.json_logs = args.json_logs
 
-        self.build = args.build
+        if self.volumes and not self.run:
-        if not self.build:
+            raise ValueError("Can not mount volumes if container is not run")
-            # Can't push nor run if we aren't building
-            self.run = False
-            self.push = False
-
-        # check against self.run and not args.run as self.run is false on
-        # --no-build
-        if args.volumes and not self.run:
-            # Can't mount if we aren't running
-            print('To Mount volumes with -v, you also need to run the '
-                  'container')
-            sys.exit(1)
-
-        for v in args.volumes:
-            src, dest = v.split(':')
-            self.volumes[src] = dest
-
-        self.run_cmd = args.cmd
-
-        if args.all_ports and not self.run:
-            print('To publish user defined port mappings, the container must '
-                  'also be run')
-            sys.exit(1)
-
-        if args.ports and not self.run:
-            print('To publish user defined port mappings, the container must '
-                  'also be run')
-            sys.exit(1)
-
-        if args.ports and not self.run_cmd:
-            print('To publish user defined port mapping, user must specify '
-                  'the command to run in the container')
-            sys.exit(1)
-
-        self.ports = validate_and_generate_port_mapping(args.ports)
-        self.all_ports = args.all_ports
-
-        if args.user_id:
-            self.user_id = args.user_id
-        if args.user_name:
-            self.user_name = args.user_name
-
-        if args.build_memory_limit:
-            self.build_memory_limit = args.build_memory_limit
-
-        if args.environment and not self.run:
-            print('To specify environment variables, you also need to run '
-                  'the container')
-            sys.exit(1)
-
-        if args.subdir:
-            self.subdir = args.subdir
-
-        if args.cache_from:
-            self.cache_from = args.cache_from
-
-        self.environment = args.environment
 
     def push_image(self):
         """Push docker image to registry"""
@@ -578,7 +415,7 @@ class Repo2Docker(Application):
             progress = json.loads(line.decode('utf-8'))
             if 'error' in progress:
                 self.log.error(progress['error'], extra=dict(phase='failed'))
-                sys.exit(1)
+                raise docker.errors.ImageLoadError(progress['error'])
             if 'id' not in progress:
                 continue
             if 'progressDetail' in progress and progress['progressDetail']:
@@ -698,20 +535,18 @@ class Repo2Docker(Application):
         s.close()
         return port
 
-    def start(self):
+    def build(self):
-        """Start execution of repo2docker""" # Check if r2d can connect to docker daemon
+        """
-        if self.build:
+        Build docker image
+        """
+        # Check if r2d can connect to docker daemon
+        if not self.dry_run:
             try:
                 api_client = docker.APIClient(version='auto',
                                               **kwargs_from_env())
             except DockerException as e:
-                print("Docker client initialization error. Check if docker is"
+                self.log.exception(e)
-                      " running on the host.")
+                raise
-                print(e)
-                if self.log_level == logging.DEBUG:
-                    raise e
-                sys.exit(1)
-
         # If the source to be executed is a directory, continue using the
         # directory. In the case of a local directory, it is used as both the
         # source and target. Reusing a local directory seems better than
@@ -733,7 +568,7 @@ class Repo2Docker(Application):
                 if not os.path.isdir(checkout_path):
                     self.log.error('Subdirectory %s does not exist',
                                    self.subdir, extra=dict(phase='failure'))
-                    sys.exit(1)
+                    raise FileNotFoundError(f'Could not find {checkout_path}')
 
             with chdir(checkout_path):
                 for BP in self.buildpacks:
@@ -754,7 +589,7 @@ class Repo2Docker(Application):
                 self.log.debug(picked_buildpack.render(),
                                extra=dict(phase='building'))
 
-                if self.build:
+                if not self.dry_run:
                     build_args = {
                         'NB_USER': self.user_name,
                         'NB_UID': str(self.user_id)
@@ -769,7 +604,7 @@ class Repo2Docker(Application):
                                           extra=dict(phase='building'))
                         elif 'error' in l:
                             self.log.info(l['error'], extra=dict(phase='failure'))
-                            sys.exit(1)
+                            raise docker.errors.BuildError(l['error'])
                         elif 'status' in l:
                                 self.log.info('Fetching base image...\r',
                                               extra=dict(phase='building'))
@@ -781,6 +616,9 @@ class Repo2Docker(Application):
             if self.cleanup_checkout:
                 shutil.rmtree(checkout_path, ignore_errors=True)
 
+    def start(self):
+        self.build()
+
         if self.push:
             self.push_image()
 

repo2docker/buildpacks/base.py

@@ -469,8 +469,8 @@ class BuildPack:
             # https://github.com/docker/docker-py/pull/1582 is related
             tar.uname = ''
             tar.gname = ''
-            tar.uid = 1000
+            tar.uid = int(build_args.get('NB_UID', 1000))
-            tar.gid = 1000
+            tar.gid = int(build_args.get('NB_UID', 1000))
             return tar
 
         for src in sorted(self.get_build_script_files()):

repo2docker/contentproviders/git.py

@@ -38,7 +38,7 @@ class Git(ContentProvider):
             if hash is None:
                 self.log.error('Failed to check out ref %s', ref,
                                extra=dict(phase='failed'))
-                sys.exit(1)
+                raise ValueError(f'Failed to check out ref {ref}')
             # If the hash is resolved above, we should be able to reset to it
             for line in execute_cmd(['git', 'reset', '--hard', hash],
                                     cwd=output_dir,

repo2docker/utils.py

@@ -122,8 +122,8 @@ def validate_and_generate_port_mapping(port_mapping):
         )$
         """, re.VERBOSE)
     ports = {}
-    if not port_mapping:
+    if port_mapping is None:
-        return None
+        return ports
     for p in port_mapping:
         if reg_regex.match(p) is None:
             raise Exception('Invalid port mapping ' + str(p))

tests/conftest.py

@@ -18,6 +18,7 @@ import pytest
 import yaml
 
 from repo2docker.app import Repo2Docker
+from repo2docker.__main__ import make_r2d
 
 
 def pytest_collect_file(parent, path):
@@ -30,8 +31,8 @@ def pytest_collect_file(parent, path):
 def make_test_func(args):
     """Generate a test function that runs repo2docker"""
     def test():
-        app = Repo2Docker()
+        app = make_r2d(args)
-        app.initialize(args)
+        app.initialize()
         if app.run_cmd:
             # verify test, run it
             app.start()

tests/contentproviders/test_git.py

@@ -1,6 +1,7 @@
 from contextlib import contextmanager
 import os
 import subprocess
+import pytest
 from tempfile import TemporaryDirectory
 from repo2docker.contentproviders import Git
 
@@ -34,6 +35,16 @@ def test_clone():
                 pass
             assert os.path.exists(os.path.join(clone_dir, 'test'))
 
+def test_bad_ref():
+    """
+    Test trying to checkout a ref that doesn't exist
+    """
+    with git_repo() as upstream:
+        with TemporaryDirectory() as clone_dir:
+            spec = {'repo': upstream, 'ref': 'does-not-exist'}
+            with pytest.raises(ValueError):
+                for _ in Git().fetch(spec, clone_dir):
+                    pass
 
 def test_always_accept():
     # The git content provider should always accept a spec

tests/test_args.py

@@ -0,0 +1,84 @@
+"""
+Test argument parsing and r2d construction
+"""
+import os
+import pytest
+import logging
+from repo2docker.__main__ import make_r2d
+from repo2docker import __version__
+
+
+def test_version(capsys):
+    """
+    Test passing '--version' to repo2docker
+    """
+    with pytest.raises(SystemExit):
+        make_r2d(['--version'])
+    assert capsys.readouterr().out == f"{__version__}\n"
+
+def test_simple():
+    """
+    Test simplest possible invocation to r2d
+    """
+    r2d = make_r2d(['.'])
+    assert r2d.repo == '.'
+
+def test_editable():
+    """
+    Test --editable behavior
+    """
+    r2d = make_r2d(['--editable', '.'])
+    assert r2d.repo == '.'
+    assert r2d.volumes[os.getcwd()] == '.'
+
+def test_dry_run():
+    """
+    Test passing --no-build implies --no-run and lack of --push
+    """
+    r2d = make_r2d(['--no-build', '.'])
+    assert r2d.dry_run
+    assert not r2d.run
+    assert not r2d.push
+
+def test_run_required():
+    """
+    Test all the things that should fail if we pass in --no-run
+    """
+    # Can't use volumes without running
+    with pytest.raises(SystemExit):
+        make_r2d(['--no-run', '--editable', '.'])
+
+    # Can't publish all ports without running
+    with pytest.raises(SystemExit):
+        make_r2d(['--no-run', '-P', '.'])
+
+    # Can't publish any ports without running
+    with pytest.raises(SystemExit):
+        make_r2d(['--no-run', '-p', '8000:8000', '.'])
+
+    # Can't publish any ports while running if we don't specify a command explicitly
+    with pytest.raises(SystemExit):
+        make_r2d(['-p', '8000:8000', '.'])
+
+def test_clean():
+    """
+    Test checkout is cleaned appropriately
+    """
+
+    # Don't clean when repo isn't local and we explicitly ask it to not clean
+    assert not make_r2d(['--no-clean', 'https://github.com/blah.git']).cleanup_checkout
+    # Do clean repo when repo isn't localj
+    assert make_r2d(['https://github.com/blah.git']).cleanup_checkout
+
+    # Don't clean by default when repo exists locally
+    assert not make_r2d(['.']).cleanup_checkout
+    # Don't clean when repo exists locally and we explicitly ask it to not clean
+    assert not make_r2d(['--no-clean', '.']).cleanup_checkout
+
+
+def test_invalid_image_name():
+    """
+    Test validating image names
+    """
+    with pytest.raises(SystemExit):
+        make_r2d(['--image-name', '_invalid', '.'])