From f6ce444450db2681380dc93646d100814bf4c492 Mon Sep 17 00:00:00 2001 From: Tim Head Date: Tue, 24 Oct 2017 23:25:34 +0200 Subject: [PATCH 1/3] Use stdlib tempfile module The tempfile module can take care of finding a good temporary directory for us on paltforms where `/tmp` does not exist. --- repo2docker/app.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/repo2docker/app.py b/repo2docker/app.py index 82439edc..d6986700 100644 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -14,6 +14,7 @@ import logging import uuid import shutil import argparse +import tempfile from pythonjsonlogger import jsonlogger import escapism @@ -53,10 +54,10 @@ class Repo2Docker(Application): return logging.INFO git_workdir = Unicode( - "/tmp", + tempfile.mkdtemp(prefix='repo2docker'), config=True, help=""" - The directory to use to check out git repositories into. + Working directory to check out git repositories to. Should be somewhere ephemeral, such as /tmp """ @@ -315,7 +316,7 @@ class Repo2Docker(Application): if self.repo_type == 'local': checkout_path = self.repo else: - checkout_path = os.path.join(self.git_workdir, str(uuid.uuid4())) + checkout_path = self.git_workdir self.fetch( self.repo, self.ref, From 5242bd83399377710b20b17ae2c4e72a2ae514b5 Mon Sep 17 00:00:00 2001 From: Tim Head Date: Wed, 1 Nov 2017 21:15:27 +0100 Subject: [PATCH 2/3] Switch to using context manager for cleanup --- repo2docker/app.py | 81 ++++++++++++++++++++++++-------------------- repo2docker/utils.py | 14 ++++++-- 2 files changed, 57 insertions(+), 38 deletions(-) diff --git a/repo2docker/app.py b/repo2docker/app.py index d6986700..db1b635f 100644 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -11,26 +11,25 @@ import json import os import time import logging -import uuid -import shutil import argparse import tempfile from pythonjsonlogger import jsonlogger import escapism -from traitlets.config import Application, LoggingConfigurable -from traitlets import Type, Bool, Unicode, Dict, List, default, Tuple +from traitlets.config import Application +from traitlets import Unicode, List, default, Tuple import docker from docker.utils import kwargs_from_env import subprocess from .detectors import ( - BuildPack, PythonBuildPack, DockerBuildPack, LegacyBinderDockerBuildPack, + PythonBuildPack, DockerBuildPack, LegacyBinderDockerBuildPack, CondaBuildPack, JuliaBuildPack, Python2BuildPack, BaseImage ) from .utils import execute_cmd +from .utils import maybe_cleanup from . import __version__ @@ -54,12 +53,13 @@ class Repo2Docker(Application): return logging.INFO git_workdir = Unicode( - tempfile.mkdtemp(prefix='repo2docker'), + None, config=True, help=""" Working directory to check out git repositories to. - Should be somewhere ephemeral, such as /tmp + The default is to use the system's temporary directory. Should be + somewhere ephemeral, such as /tmp. """ ) @@ -248,7 +248,6 @@ class Repo2Docker(Application): self.run_cmd = args.cmd - def push_image(self): client = docker.APIClient(version='auto', **kwargs_from_env()) # Build a progress setup for each layer, and only emit per-layer info every 1.5s @@ -316,39 +315,49 @@ class Repo2Docker(Application): if self.repo_type == 'local': checkout_path = self.repo else: - checkout_path = self.git_workdir - self.fetch( - self.repo, - self.ref, - checkout_path - ) + if self.git_workdir is None: + checkout_path = tempfile.mkdtemp(prefix='repo2docker') + else: + checkout_path = self.git_workdir - os.chdir(checkout_path) - picked_buildpack = compose(self.default_buildpack, parent=self) + # keep as much as possible in the context manager to make sure we + # cleanup if things go wrong + with maybe_cleanup(checkout_path, self.cleanup_checkout): + if self.repo_type == 'remote': + self.fetch( + self.repo, + self.ref, + checkout_path + ) - for bp_spec in self.buildpacks: - bp = compose(bp_spec, parent=self) - if bp.detect(): - picked_buildpack = bp - break + os.chdir(checkout_path) + picked_buildpack = compose(self.default_buildpack, parent=self) - self.log.debug(picked_buildpack.render(), extra=dict(phase='building')) + for bp_spec in self.buildpacks: + bp = compose(bp_spec, parent=self) + if bp.detect(): + picked_buildpack = bp + break - if self.build: - self.log.info('Using %s builder\n', bp.name, extra=dict(phase='building')) - for l in picked_buildpack.build(self.output_image_spec): - if 'stream' in l: - self.log.info(l['stream'], extra=dict(phase='building')) - elif 'error' in l: - self.log.info(l['error'], extra=dict(phase='failure')) - sys.exit(1) - elif 'status' in l: - self.log.info('Fetching base image...\r', extra=dict(phase='building')) - else: - self.log.info(json.dumps(l), extra=dict(phase='building')) + self.log.debug(picked_buildpack.render(), + extra=dict(phase='building')) - if self.cleanup_checkout: - shutil.rmtree(checkout_path, ignore_errors=True) + if self.build: + self.log.info('Using %s builder\n', bp.name, + extra=dict(phase='building')) + for l in picked_buildpack.build(self.output_image_spec): + if 'stream' in l: + self.log.info(l['stream'], + extra=dict(phase='building')) + elif 'error' in l: + self.log.info(l['error'], extra=dict(phase='failure')) + sys.exit(1) + elif 'status' in l: + self.log.info('Fetching base image...\r', + extra=dict(phase='building')) + else: + self.log.info(json.dumps(l), + extra=dict(phase='building')) if self.push: self.push_image() diff --git a/repo2docker/utils.py b/repo2docker/utils.py index 9b6274a3..a0c4623e 100644 --- a/repo2docker/utils.py +++ b/repo2docker/utils.py @@ -1,6 +1,9 @@ +from contextlib import contextmanager from functools import partial +import shutil import subprocess + def execute_cmd(cmd, capture=False, **kwargs): """ Call given command, yielding output line by line if capture=True @@ -17,7 +20,7 @@ def execute_cmd(cmd, capture=False, **kwargs): if ret != 0: raise subprocess.CalledProcessError(ret, cmd) return - + # Capture output for logging. # Each line will be yielded as text. # This should behave the same as .readline(), but splits on `\r` OR `\n`, @@ -27,7 +30,7 @@ def execute_cmd(cmd, capture=False, **kwargs): line = b''.join(buf).decode('utf8', 'replace') buf[:] = [] return line - + c_last = '' try: for c in iter(partial(proc.stdout.read, 1), b''): @@ -41,3 +44,10 @@ def execute_cmd(cmd, capture=False, **kwargs): ret = proc.wait() if ret != 0: raise subprocess.CalledProcessError(ret, cmd) + + +@contextmanager +def maybe_cleanup(path, cleanup=False): + yield + if cleanup: + shutil.rmtree(path, ignore_errors=True) From 5ff228decaabc18fb5563862da6348025c9d0605 Mon Sep 17 00:00:00 2001 From: Tim Head Date: Thu, 9 Nov 2017 16:41:00 +0100 Subject: [PATCH 3/3] Fix argument definition --- repo2docker/app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/repo2docker/app.py b/repo2docker/app.py index db1b635f..bc582a43 100644 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -55,6 +55,7 @@ class Repo2Docker(Application): git_workdir = Unicode( None, config=True, + allow_none=True, help=""" Working directory to check out git repositories to.