From 67d2e7a828808f691d66427a8ee1b382add438f2 Mon Sep 17 00:00:00 2001
From: whitequark <whitequark@whitequark.org>
Date: Sat, 16 Jan 2016 01:28:26 +0000
Subject: [PATCH] worker: display compile warnings and errors nicely (#227).

---
 artiq/coredevice/core.py    |  9 +++++----
 artiq/master/worker_impl.py | 33 ++++++++++++++++++++++++++-------
 2 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/artiq/coredevice/core.py b/artiq/coredevice/core.py
index b7f1f7276..ae0cb51a9 100644
--- a/artiq/coredevice/core.py
+++ b/artiq/coredevice/core.py
@@ -13,15 +13,16 @@ from artiq.compiler.targets import OR1KTarget
 # Import for side effects (creating the exception classes).
 from artiq.coredevice import exceptions
 
-def _render_diagnostic(diagnostic):
+
+def _render_diagnostic(diagnostic, colored):
     def shorten_path(path):
         return path.replace(os.path.normpath(os.path.join(__file__, "..", "..")), "<artiq>")
-    lines = [shorten_path(path) for path in diagnostic.render(colored=True)]
+    lines = [shorten_path(path) for path in diagnostic.render(colored)]
     return "\n".join(lines)
 
 class _DiagnosticEngine(diagnostic.Engine):
     def render_diagnostic(self, diagnostic):
-        sys.stderr.write(_render_diagnostic(diagnostic) + "\n")
+        sys.stderr.write(_render_diagnostic(diagnostic, colored=True) + "\n")
 
 class CompileError(Exception):
     def __init__(self, diagnostic):
@@ -30,7 +31,7 @@ class CompileError(Exception):
     def __str__(self):
         # Prepend a newline so that the message shows up on after
         # exception class name printed by Python.
-        return "\n" + _render_diagnostic(self.diagnostic)
+        return "\n" + _render_diagnostic(self.diagnostic, colored=True)
 
 
 @syscall
diff --git a/artiq/master/worker_impl.py b/artiq/master/worker_impl.py
index da697e1dc..2ff5f3f56 100644
--- a/artiq/master/worker_impl.py
+++ b/artiq/master/worker_impl.py
@@ -4,12 +4,13 @@ import os
 import logging
 from collections import OrderedDict
 
+import artiq
 from artiq.protocols import pyon
 from artiq.tools import file_import
 from artiq.master.worker_db import DeviceManager, DatasetManager, get_hdf5_output
 from artiq.language.environment import is_experiment
 from artiq.language.core import set_watchdog_factory, TerminationRequested
-from artiq.coredevice.core import CompileError, host_only
+from artiq.coredevice.core import CompileError, host_only, _render_diagnostic
 from artiq import __version__ as artiq_version
 
 
@@ -169,6 +170,26 @@ def string_to_hdf5(f, key, value):
     dataset[()] = value.encode()
 
 
+def setup_diagnostics(experiment_file, repository_path):
+    def render_diagnostic(self, diagnostic):
+        message = "Cannot compile {}\n".format(experiment_file) + \
+                    _render_diagnostic(diagnostic, colored=False)
+        if repository_path is not None:
+            message = message.replace(repository_path, "<repository>")
+        logging.error(message)
+
+    # This is kind of gross, but 1) we do not have any explicit connection
+    # between the worker and a coredevice.core.Core instance at all,
+    # and 2) the diagnostic engine really ought to be per-Core, since
+    # that's what uses it and the repository path is per-Core.
+    # So I don't know how to implement this properly for now.
+    #
+    # This hack is as good or bad as any other solution that involves
+    # putting inherently local objects (the diagnostic engine) into
+    # global slots, and there isn't any point in making it prettier by
+    # wrapping it in layers of indirection.
+    artiq.coredevice.core._DiagnosticEngine.render_diagnostic = render_diagnostic
+
 def main():
     sys.stdout = LogForwarder()
     sys.stderr = LogForwarder()
@@ -199,6 +220,8 @@ def main():
                     repository_path = obj["wd"]
                 else:
                     experiment_file = expid["file"]
+                    repository_path = None
+                setup_diagnostics(experiment_file, repository_path)
                 exp = get_exp(experiment_file, expid["class_name"])
                 device_mgr.virtual_devices["scheduler"].set_run_info(
                     rid, obj["pipeline_name"], expid, obj["priority"])
@@ -230,12 +253,8 @@ def main():
                 put_object({"action": "completed"})
             elif action == "terminate":
                 break
-    except CompileError as exc:
-        # TODO: This should be replaced with a proper DiagnosticEngine.
-        message = "Cannot compile {}\n".format(experiment_file) + exc.render_string()
-        if repository_path is not None:
-            message = message.replace(repository_path, "<repository>")
-        logging.error(message)
+    except CompileError:
+        pass
     except Exception as exc:
         short_exc_info = type(exc).__name__
         exc_str = str(exc)