From c0937f1a49f1c07036d99ad17b4e831ccaa21273 Mon Sep 17 00:00:00 2001 From: David Trowbridge Date: Wed, 17 Feb 2016 14:26:57 -0800 Subject: [PATCH 1/2] Fix an UnboundLocalError if a compiler fails. Depending on where an exception is raised inside a compiler's `execute_command`, the `stdout` variable may not be assigned. In this case, the finally handler would fail with an `UnboundLocalError`, hiding the true cause of the exception. This change adds a new variable, `stdout_name`, which is populated once we've actually created the file. After this, the `CompilerError` is successfully raised with the contents of the true error. --- pipeline/compilers/__init__.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pipeline/compilers/__init__.py b/pipeline/compilers/__init__.py index 4610c098..65d9dcb7 100644 --- a/pipeline/compilers/__init__.py +++ b/pipeline/compilers/__init__.py @@ -112,9 +112,12 @@ def execute_command(self, command, cwd=None, stdout_captured=None): argument_list.extend(flattening_arg) try: + stdout_name = None + # 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: + stdout_name = stdout.name compiling = subprocess.Popen(argument_list, cwd=cwd, stdout=stdout, stderr=subprocess.PIPE) @@ -135,7 +138,8 @@ def execute_command(self, command, cwd=None, stdout_captured=None): 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) + if stdout_name: + if stdout_captured: + os.rename(stdout_name, os.path.join(cwd or os.curdir, stdout_captured)) + else: + os.remove(stdout_name) From d0d53a4e59cb5701029ba6246b609f251dd9dff7 Mon Sep 17 00:00:00 2001 From: David Trowbridge Date: Mon, 22 Feb 2016 09:14:08 -0800 Subject: [PATCH 2/2] Update to avoid merge conflicts with PR #531. Pull request 531, among many other things, includes a similar fix for the use of `stdout` before it's been assigned. That implementation is a nicer read, so I've switched over to it. --- pipeline/compilers/__init__.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pipeline/compilers/__init__.py b/pipeline/compilers/__init__.py index 65d9dcb7..d8203d1f 100644 --- a/pipeline/compilers/__init__.py +++ b/pipeline/compilers/__init__.py @@ -111,13 +111,11 @@ def execute_command(self, command, cwd=None, stdout_captured=None): else: argument_list.extend(flattening_arg) + stdout = None try: - stdout_name = None - # 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: - stdout_name = stdout.name compiling = subprocess.Popen(argument_list, cwd=cwd, stdout=stdout, stderr=subprocess.PIPE) @@ -138,8 +136,8 @@ def execute_command(self, command, cwd=None, stdout_captured=None): raise CompilerError(e) finally: # Decide what to do with captured stdout. - if stdout_name: + if stdout: if stdout_captured: - os.rename(stdout_name, os.path.join(cwd or os.curdir, stdout_captured)) + os.rename(stdout.name, os.path.join(cwd or os.curdir, stdout_captured)) else: - os.remove(stdout_name) + os.remove(stdout.name)