diff --git a/pipeline/compilers/__init__.py b/pipeline/compilers/__init__.py index 491bc217..3b713c73 100644 --- a/pipeline/compilers/__init__.py +++ b/pipeline/compilers/__init__.py @@ -1,16 +1,14 @@ from __future__ import unicode_literals import os - -try: - from shlex import quote -except ImportError: - from pipes import quote +import subprocess +from tempfile import NamedTemporaryFile from django.contrib.staticfiles import finders from django.contrib.staticfiles.storage import staticfiles_storage from django.core.files.base import ContentFile from django.utils.encoding import smart_bytes +from django.utils.six import string_types from pipeline.conf import settings from pipeline.exceptions import CompilerError @@ -40,8 +38,8 @@ def _compile(input_path): infile = finders.find(input_path) outfile = self.output_path(infile, compiler.output_extension) outdated = compiler.is_outdated(input_path, output_path) - compiler.compile_file(quote(infile), quote(outfile), - outdated=outdated, force=force) + compiler.compile_file(infile, outfile, + outdated=outdated, force=force) return output_path else: return input_path @@ -90,18 +88,55 @@ def is_outdated(self, infile, outfile): class SubProcessCompiler(CompilerBase): - def execute_command(self, command, content=None, cwd=None): - import subprocess - pipe = subprocess.Popen(command, shell=True, cwd=cwd, - stdout=subprocess.PIPE, stdin=subprocess.PIPE, - stderr=subprocess.PIPE) - if content: - content = smart_bytes(content) - stdout, stderr = pipe.communicate(content) - if stderr.strip(): - raise CompilerError(stderr) - if self.verbose: - print(stderr) - if pipe.returncode != 0: - raise CompilerError("Command '{0}' returned non-zero exit status {1}".format(command, pipe.returncode)) - return stdout + def execute_command(self, command, cwd=None, stdout_captured=None): + """Execute a command at cwd, saving its normal output at + stdout_captured. Errors, defined as nonzero return code or a failure + to start execution, will raise a CompilerError exception with a + description of the cause. They do not write output. + + This is file-system safe (any valid file names are allowed, even with + spaces or crazy characters) and OS agnostic (existing and future OSes + that Python supports should already work). + + The only thing weird here is that any incoming command arg item may + itself be a tuple. This allows compiler implementations to look clean + while supporting historical string config settings and maintaining + backwards compatibility. Thus, we flatten one layer deep. + ((env, foocomp), infile, (-arg,)) -> (env, foocomp, infile, -arg) + """ + argument_list = [] + for flattening_arg in command: + if isinstance(flattening_arg, string_types): + argument_list.append(flattening_arg) + else: + argument_list.extend(flattening_arg) + + try: + # We always catch stdout in a file, but we may not have a use for it. + temp_file_container = cwd or os.path.dirname(stdout_captured or "") or os.getcwd() + with NamedTemporaryFile(delete=False, dir=temp_file_container) as stdout: + compiling = subprocess.Popen(argument_list, cwd=cwd, + stdout=stdout, + stderr=subprocess.PIPE) + _, stderr = compiling.communicate() + + if compiling.returncode != 0: + stdout_captured = None # Don't save erroneous result. + raise CompilerError( + "{0!r} exit code {1}\n{2}".format(argument_list, compiling.returncode, stderr)) + + # User wants to see everything that happened. + if self.verbose: + with open(stdout.name) as out: + print(out.read()) + print(stderr) + except OSError as e: + stdout_captured = None # Don't save erroneous result. + raise CompilerError(e) + finally: + # Decide what to do with captured stdout. + if stdout_captured: + os.rename(stdout.name, os.path.join(cwd or os.curdir, stdout_captured)) + else: + os.remove(stdout.name) + diff --git a/pipeline/compilers/coffee.py b/pipeline/compilers/coffee.py index 4c1fa642..8260ede6 100644 --- a/pipeline/compilers/coffee.py +++ b/pipeline/compilers/coffee.py @@ -13,10 +13,10 @@ def match_file(self, path): def compile_file(self, infile, outfile, outdated=False, force=False): if not outdated and not force: return # File doesn't need to be recompiled - command = "%s -cp %s %s > %s" % ( + command = ( settings.COFFEE_SCRIPT_BINARY, + "-cp", settings.COFFEE_SCRIPT_ARGUMENTS, infile, - outfile ) - return self.execute_command(command) + return self.execute_command(command, stdout_captured=outfile) diff --git a/pipeline/compilers/es6.py b/pipeline/compilers/es6.py index 1e81d57e..a5cedd77 100644 --- a/pipeline/compilers/es6.py +++ b/pipeline/compilers/es6.py @@ -13,10 +13,11 @@ def match_file(self, path): def compile_file(self, infile, outfile, outdated=False, force=False): if not outdated and not force: return # File doesn't need to be recompiled - command = "%s %s %s -o %s" % ( + command = ( settings.BABEL_BINARY, settings.BABEL_ARGUMENTS, infile, + "-o", outfile ) return self.execute_command(command) diff --git a/pipeline/compilers/less.py b/pipeline/compilers/less.py index 709cd5ae..fa81747b 100644 --- a/pipeline/compilers/less.py +++ b/pipeline/compilers/less.py @@ -14,10 +14,9 @@ def match_file(self, filename): def compile_file(self, infile, outfile, outdated=False, force=False): # Pipe to file rather than provide outfile arg due to a bug in lessc - command = "%s %s %s > %s" % ( + command = ( settings.LESS_BINARY, settings.LESS_ARGUMENTS, infile, - outfile ) - return self.execute_command(command, cwd=dirname(infile)) + return self.execute_command(command, cwd=dirname(infile), stdout_captured=outfile) diff --git a/pipeline/compilers/livescript.py b/pipeline/compilers/livescript.py index 35c5e812..f72f896b 100644 --- a/pipeline/compilers/livescript.py +++ b/pipeline/compilers/livescript.py @@ -13,10 +13,10 @@ def match_file(self, path): def compile_file(self, infile, outfile, outdated=False, force=False): if not outdated and not force: return # File doesn't need to be recompiled - command = "%s -cp %s %s > %s" % ( + command = ( settings.LIVE_SCRIPT_BINARY, + "-cp", settings.LIVE_SCRIPT_ARGUMENTS, infile, - outfile ) - return self.execute_command(command) + return self.execute_command(command, stdout_captured=outfile) diff --git a/pipeline/compilers/sass.py b/pipeline/compilers/sass.py index 495f130a..d05c87ec 100644 --- a/pipeline/compilers/sass.py +++ b/pipeline/compilers/sass.py @@ -13,7 +13,7 @@ def match_file(self, filename): return filename.endswith(('.scss', '.sass')) def compile_file(self, infile, outfile, outdated=False, force=False): - command = "%s %s %s %s" % ( + command = ( settings.SASS_BINARY, settings.SASS_ARGUMENTS, infile, diff --git a/pipeline/compilers/stylus.py b/pipeline/compilers/stylus.py index c70d7ee0..320efd9e 100644 --- a/pipeline/compilers/stylus.py +++ b/pipeline/compilers/stylus.py @@ -13,7 +13,7 @@ def match_file(self, filename): return filename.endswith('.styl') def compile_file(self, infile, outfile, outdated=False, force=False): - command = "%s %s %s" % ( + command = ( settings.STYLUS_BINARY, settings.STYLUS_ARGUMENTS, infile diff --git a/pipeline/conf.py b/pipeline/conf.py index bdb4a614..151f489c 100644 --- a/pipeline/conf.py +++ b/pipeline/conf.py @@ -1,6 +1,8 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals +import shlex + from django.conf import settings as _settings DEFAULTS = { @@ -77,12 +79,26 @@ class PipelineSettings(object): - ''' + """ Container object for pipeline settings - ''' + """ def __init__(self, wrapped_settings): DEFAULTS.update(wrapped_settings) self.__dict__ = DEFAULTS + def __getattr__(self, name): + if hasattr(self, name): + value = getattr(self, name) + elif name in self: + value = DEFAULTS[name] + else: + raise AttributeError("'%s' setting not found" % name) + + if name.endswith(("_BINARY", "_ARGUMENTS")): + if isinstance(value, (type(u""), type(b""))): + return tuple(shlex.split(value)) + return tuple(value) + + return value settings = PipelineSettings(_settings.PIPELINE) diff --git a/tests/tests/test_compiler.py b/tests/tests/test_compiler.py index 15b07821..372496c4 100644 --- a/tests/tests/test_compiler.py +++ b/tests/tests/test_compiler.py @@ -1,15 +1,70 @@ from __future__ import unicode_literals +import os + from django.test import TestCase from pipeline.conf import settings -from pipeline.compilers import Compiler, CompilerBase +from pipeline.compilers import Compiler, CompilerBase, SubProcessCompiler from pipeline.collector import default_collector - +from pipeline.exceptions import CompilerError from tests.utils import _ +class FailingCompiler(SubProcessCompiler): + output_extension = 'junk' + + def match_file(self, path): + return path.endswith('.coffee') + + def compile_file(self, infile, outfile, outdated=False, force=False): + command = (("/usr/bin/env", "false",),) + return self.execute_command(command) + + +class InvalidCompiler(SubProcessCompiler): + output_extension = 'junk' + + def match_file(self, path): + return path.endswith('.coffee') + + def compile_file(self, infile, outfile, outdated=False, force=False): + command = ( + ("this-exists-nowhere-as-a-command-and-should-fail",), + infile, + outfile + ) + return self.execute_command(command) + + +class CopyingCompiler(SubProcessCompiler): + output_extension = 'junk' + + def match_file(self, path): + return path.endswith('.coffee') + + def compile_file(self, infile, outfile, outdated=False, force=False): + command = ( + ("cp",), + ("--no-dereference", "--preserve=links",), + infile, + outfile + ) + return self.execute_command(command) + + +class LineNumberingCompiler(SubProcessCompiler): + output_extension = 'junk' + + def match_file(self, path): + return path.endswith('.coffee') + + def compile_file(self, infile, outfile, outdated=False, force=False): + command = (("/usr/bin/env", "cat"), ("-n",), infile,) + return self.execute_command(command, stdout_captured=outfile) + + class DummyCompiler(CompilerBase): output_extension = 'js' @@ -20,7 +75,7 @@ def compile_file(self, infile, outfile, outdated=False, force=False): return -class CompilerTest(TestCase): +class DummyCompilerTest(TestCase): def setUp(self): default_collector.collect() self.compiler = Compiler() @@ -45,3 +100,76 @@ def test_compile(self): def tearDown(self): default_collector.clear() settings.COMPILERS = self.old_compilers + + +class CompilerStdoutTest(TestCase): + def setUp(self): + default_collector.collect() + self.compiler = Compiler() + self.old_compilers = settings.COMPILERS + settings.PIPELINE_COMPILERS = ['tests.tests.test_compiler.LineNumberingCompiler'] + + def test_output_path(self): + output_path = self.compiler.output_path("js/helpers.coffee", "js") + self.assertEqual(output_path, "js/helpers.js") + + def test_compile(self): + paths = self.compiler.compile([_('pipeline/js/dummy.coffee')]) + self.assertEqual([_('pipeline/js/dummy.junk')], list(paths)) + + def tearDown(self): + default_collector.clear() + settings.PIPELINE_COMPILERS = self.old_compilers + + +class CompilerSelfWriterTest(TestCase): + def setUp(self): + default_collector.collect() + self.compiler = Compiler() + self.old_compilers = settings.PIPELINE_COMPILERS + settings.PIPELINE_COMPILERS = ['tests.tests.test_compiler.CopyingCompiler'] + + def test_output_path(self): + output_path = self.compiler.output_path("js/helpers.coffee", "js") + self.assertEqual(output_path, "js/helpers.js") + + def test_compile(self): + paths = self.compiler.compile([_('pipeline/js/dummy.coffee')]) + default_collector.collect() + self.assertEqual([_('pipeline/js/dummy.junk')], list(paths)) + + def tearDown(self): + default_collector.clear() + settings.COMPILERS = self.old_compilers + + +class InvalidCompilerTest(TestCase): + def setUp(self): + default_collector.collect() + self.compiler = Compiler() + self.old_compilers = settings.COMPILERS + settings.COMPILERS = ['tests.tests.test_compiler.InvalidCompiler'] + + def test_compile(self): + self.assertRaises(CompilerError, + self.compiler.compile, [_('pipeline/js/dummy.coffee')]) + + def tearDown(self): + default_collector.clear() + settings.COMPILERS = self.old_compilers + + +class FailingCompilerTest(TestCase): + def setUp(self): + default_collector.collect() + self.compiler = Compiler() + self.old_compilers = settings.COMPILERS + settings.COMPILERS = ['tests.tests.test_compiler.FailingCompiler'] + + def test_compile(self): + self.assertRaises(CompilerError, + self.compiler.compile, [_('pipeline/js/dummy.coffee'),]) + + def tearDown(self): + default_collector.clear() + settings.COMPILERS = self.old_compilers diff --git a/tests/tests/test_conf.py b/tests/tests/test_conf.py new file mode 100644 index 00000000..b9ad05c3 --- /dev/null +++ b/tests/tests/test_conf.py @@ -0,0 +1,32 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.test import TestCase + +from pipeline.conf import PipelineSettings + +class TestSettings(TestCase): + + def test_3unicode(self): + s = PipelineSettings(dict(), DEFAULTS={ "PIPELINE_FOO_BINARY": "env actualprogram" }) + self.assertEqual(s.PIPELINE_FOO_BINARY, ('env', 'actualprogram')) + + def test_2unicode(self): + s = PipelineSettings(dict(), DEFAULTS={ "PIPELINE_FOO_BINARY": u"env actualprogram" }) + self.assertEqual(s.PIPELINE_FOO_BINARY, ('env', 'actualprogram')) + + def test_2bytes(self): + s = PipelineSettings(dict(), DEFAULTS={ "PIPELINE_FOO_BINARY": "env actualprogram" }) + self.assertEqual(s.PIPELINE_FOO_BINARY, ('env', 'actualprogram')) + + def test_expected_splitting(self): + s = PipelineSettings(dict(), DEFAULTS={ "PIPELINE_FOO_BINARY": "env actualprogram" }) + self.assertEqual(s.PIPELINE_FOO_BINARY, ('env', 'actualprogram')) + + def test_expected_preservation(self): + s = PipelineSettings(dict(), DEFAULTS={ "PIPELINE_FOO_BINARY": r"actual\ program" }) + self.assertEqual(s.PIPELINE_FOO_BINARY, ('actual program',)) + + def test_tuples_are_normal(self): + s = PipelineSettings(dict(), DEFAULTS={ "PIPELINE_FOO_ARGUMENTS": ("explicit", "with", "args") }) + self.assertEqual(s.PIPELINE_FOO_ARGUMENTS, ('explicit', 'with', 'args'))