code review

driver/portfolio_runner.py

@@ -63,8 +63,8 @@ def adapt_args(args, search_cost_type, heuristic_cost_type, plan_manager):
             break
 
 
-def run_search(executable, args, sas_file, plan_manager, time, memory):
-    complete_args = [executable] + args + [
+def run_search(command, args, sas_file, plan_manager, time, memory):
+    complete_args = command + args + [
         "--internal-plan-file", plan_manager.get_plan_prefix()]
     print("args: %s" % complete_args)
 
@@ -91,7 +91,7 @@ def compute_run_time(timeout, configs, pos):
 
 
 def run_sat_config(configs, pos, search_cost_type, heuristic_cost_type,
-                   executable, sas_file, plan_manager, timeout, memory):
+                   command, sas_file, plan_manager, timeout, memory):
     run_time = compute_run_time(timeout, configs, pos)
     if run_time <= 0:
         return None
@@ -102,12 +102,12 @@ def run_sat_config(configs, pos, search_cost_type, heuristic_cost_type,
         args.extend([
             "--internal-previous-portfolio-plans",
             str(plan_manager.get_plan_counter())])
-    result = run_search(executable, args, sas_file, plan_manager, run_time, memory)
+    result = run_search(command, args, sas_file, plan_manager, run_time, memory)
     plan_manager.process_new_plans()
     return result
 
 
-def run_sat(configs, executable, sas_file, plan_manager, final_config,
+def run_sat(configs, command, sas_file, plan_manager, final_config,
             final_config_builder, timeout, memory):
     # If the configuration contains S_COST_TYPE or H_COST_TRANSFORM and the task
     # has non-unit costs, we start by treating all costs as one. When we find
@@ -120,7 +120,7 @@ def run_sat(configs, executable, sas_file, plan_manager, final_config,
         for pos, (relative_time, args) in enumerate(configs):
             exitcode = run_sat_config(
                 configs, pos, search_cost_type, heuristic_cost_type,
-                executable, sas_file, plan_manager, timeout, memory)
+                command, sas_file, plan_manager, timeout, memory)
             if exitcode is None:
                 continue
 
@@ -140,7 +140,7 @@ def run_sat(configs, executable, sas_file, plan_manager, final_config,
                     heuristic_cost_type = "plusone"
                     exitcode = run_sat_config(
                         configs, pos, search_cost_type, heuristic_cost_type,
-                        executable, sas_file, plan_manager, timeout, memory)
+                        command, sas_file, plan_manager, timeout, memory)
                     if exitcode is None:
                         return
 
@@ -162,18 +162,18 @@ def run_sat(configs, executable, sas_file, plan_manager, final_config,
         print("Abort portfolio and run final config.")
         exitcode = run_sat_config(
             [(1, final_config)], 0, search_cost_type,
-            heuristic_cost_type, executable, sas_file, plan_manager,
+            heuristic_cost_type, command, sas_file, plan_manager,
             timeout, memory)
         if exitcode is not None:
             yield exitcode
 
 
-def run_opt(configs, executable, sas_file, plan_manager, timeout, memory):
+def run_opt(configs, command, sas_file, plan_manager, timeout, memory):
     for pos, (relative_time, args) in enumerate(configs):
         run_time = compute_run_time(timeout, configs, pos)
         if run_time <= 0:
             return
-        exitcode = run_search(executable, args, sas_file, plan_manager,
+        exitcode = run_search(command, args, sas_file, plan_manager,
                               run_time, memory)
         yield exitcode
 
@@ -202,7 +202,7 @@ def get_portfolio_attributes(portfolio: Path):
     return attributes
 
 
-def run(portfolio: Path, executable, sas_file, plan_manager, time, memory):
+def run(portfolio: Path, command, sas_file, plan_manager, time, memory):
     """
     Run the configs in the given portfolio file.
 
@@ -231,9 +231,9 @@ def run(portfolio: Path, executable, sas_file, plan_manager, time, memory):
 
     if optimal:
         exitcodes = run_opt(
-            configs, executable, sas_file, plan_manager, timeout, memory)
+            configs, command, sas_file, plan_manager, timeout, memory)
     else:
         exitcodes = run_sat(
-            configs, executable, sas_file, plan_manager, final_config,
+            configs, command, sas_file, plan_manager, final_config,
             final_config_builder, timeout, memory)
     return returncodes.generate_portfolio_exitcode(list(exitcodes))

driver/run_components.py

@@ -30,15 +30,21 @@
 _VALIDATE_PATH = Path(_VALIDATE_NAME) if _VALIDATE_NAME else None
 
 
-def get_search_executable(build: str, exit_on_failure=True):
-    return _get_executable(build, _REL_SEARCH_PATH, exit_on_failure)
+class MissingBuildError(Exception):
+    pass
 
 
-def get_translate_executable(build: str, exit_on_failure=True):
-    return _get_executable(build, _REL_TRANSLATE_PATH, exit_on_failure)
+def get_search_command(build: str):
+    return [_get_executable(build, _REL_SEARCH_PATH)]
 
 
-def _get_executable(build: str, rel_path: Path, exit_on_failure=True):
+def get_translate_command(build: str):
+    assert sys.executable, "Path to interpreter could not be found"
+    abs_path = _get_executable(build, _REL_TRANSLATE_PATH)
+    return [sys.executable, abs_path]
+
+
+def _get_executable(build: str, rel_path: Path):
     # First, consider 'build' to be a path directly to the binaries.
     # The path can be absolute or relative to the current working
     # directory.
@@ -50,21 +56,15 @@ def _get_executable(build: str, rel_path: Path, exit_on_failure=True):
         #   '<repo-root>/builds/<buildname>/bin'.
         build_dir = util.BUILDS_DIR / build / "bin"
         if not build_dir.exists():
-            if exit_on_failure:
-                returncodes.exit_with_driver_input_error(
-                    f"Could not find build '{build}' at {build_dir}. "
-                    f"Please run './build.py {build}'.")
-            else:
-                return None
+            raise MissingBuildError(
+                f"Could not find build '{build}' at {build_dir}. "
+                f"Please run './build.py {build}'.")
 
     abs_path = build_dir / rel_path
     if not abs_path.exists():
-        if exit_on_failure:
-            returncodes.exit_with_driver_input_error(
-                f"Could not find '{rel_path}' in build '{build}'. "
-                f"Please run './build.py {build}'.")
-        else:
-            return None
+        raise MissingBuildError(
+            f"Could not find '{rel_path}' in build '{build}'. "
+            f"Please run './build.py {build}'.")
 
     return abs_path
 
@@ -75,13 +75,14 @@ def run_translate(args):
         args.translate_time_limit, args.overall_time_limit)
     memory_limit = limits.get_memory_limit(
         args.translate_memory_limit, args.overall_memory_limit)
-    translate = get_translate_executable(args.build)
-    assert sys.executable, "Path to interpreter could not be found"
-    cmd = [sys.executable] + [translate] + args.translate_inputs + args.translate_options
+    try:
+        translate_command = get_translate_command(args.build)
+    except MissingBuildError as e:
+        returncodes.exit_with_driver_input_error(e)
 
     stderr, returncode = call.get_error_output_and_returncode(
         "translator",
-        cmd,
+        translate_command + args.translate_inputs + args.translate_options,
         time_limit=time_limit,
         memory_limit=memory_limit)
 
@@ -121,7 +122,10 @@ def run_search(args):
         args.search_time_limit, args.overall_time_limit)
     memory_limit = limits.get_memory_limit(
         args.search_memory_limit, args.overall_memory_limit)
-    executable = get_search_executable(args.build)
+    try:
+        search_command = get_search_command(args.build)
+    except MissingBuildError as e:
+        returncodes.exit_with_driver_input_error(e)
 
     plan_manager = PlanManager(
         args.plan_file,
@@ -133,7 +137,7 @@ def run_search(args):
         assert not args.search_options
         logging.info(f"search portfolio: {args.portfolio}")
         return portfolio_runner.run(
-            args.portfolio, executable, args.search_input, plan_manager,
+            args.portfolio, search_command, args.search_input, plan_manager,
             time_limit, memory_limit)
     else:
         if not args.search_options:
@@ -144,7 +148,7 @@ def run_search(args):
         try:
             call.check_call(
                 "search",
-                [executable] + args.search_options,
+                search_command + args.search_options,
                 stdin=args.search_input,
                 time_limit=time_limit,
                 memory_limit=memory_limit)

driver/tab_completion.py

@@ -1,12 +1,10 @@
 import os
-from pathlib import Path
 import subprocess
 import sys
 import tempfile
 
-from . import returncodes
 from . import util
-from .run_components import get_search_executable, get_translate_executable
+from .run_components import get_search_command, get_translate_command, MissingBuildError
 
 try:
     import argcomplete
@@ -16,9 +14,10 @@
 
 
 def complete_build_arg(prefix, parsed_args, **kwargs):
-    if not util.BUILDS_DIR.exists():
+    try:
+        return [p.name for p in util.BUILDS_DIR.iterdir() if p.is_dir()]
+    except OSError:
         return []
-    return [p.name for p in util.BUILDS_DIR.iterdir() if p.is_dir()]
 
 
 def complete_planner_args(prefix, parsed_args, **kwargs):
@@ -43,18 +42,12 @@ def complete_planner_args(prefix, parsed_args, **kwargs):
     if parsed_args.filenames or double_dash_in_options:
         if current_mode == "search":
             completions["--translate-options"] = ""
-
-            downward = get_search_executable(parsed_args.build, exit_on_failure=False)
-            if downward and downward.exists():
-                completions.update(_get_completions_from_downward(
-                    downward, parsed_args.search_options, prefix))
+            completions.update(_get_completions_from_downward(
+                parsed_args.build, parsed_args.search_options, prefix))
         else:
             completions["--search-options"] = ""
-
-            translator = get_translate_executable(parsed_args.build, exit_on_failure=False)
-            if translator and translator.exists():
-                completions.update(_get_completions_from_translator(
-                    translator, parsed_args.translate_options, prefix))
+            completions.update(_get_completions_from_translator(
+                parsed_args.build, parsed_args.translate_options, prefix))
 
     if has_only_filename_options and len(parsed_args.filenames) < 2:
         file_completer = argcomplete.FilesCompleter()
@@ -84,35 +77,65 @@ def _split_argcomplete_ouput(content, entry_separator, help_separator):
     return suggestions
 
 
-def _call_argcomplete(python_file, comp_line, comp_point):
+def _get_bash_completion_args(cmd, options, prefix):
+    """
+    Return values for four environment variables, bash uses as part of tab
+    completion when cmd is called with parsed arguments args, and the unparsed
+    prefix of a word to be completed prefix.
+    COMP_POINT: the cursor position within COMP_LINE
+    COMP_LINE: the full command line as a string
+    COMP_CWORD: an index into COMP_WORDS to the word under the cursor.
+    COMP_WORDS: the command line as list of words
+    """
+    comp_words = [str(x) for x in cmd] + options + [prefix]
+    comp_line = " ".join(comp_words)
+    comp_point = str(len(comp_line))
+    comp_cword = str(len(comp_words) - 1)
+    return comp_point, comp_line, comp_cword, comp_words
+
+
+def _call_argcomplete(cmd, comp_line, comp_point):
     with tempfile.NamedTemporaryFile(mode="r") as f:
         env = os.environ.copy()
         env["COMP_LINE"] = comp_line
-        env["COMP_POINT"] = str(comp_point)
+        env["COMP_POINT"] = comp_point
         env["_ARGCOMPLETE"] = "1"
         env["_ARGCOMPLETE_STDOUT_FILENAME"] = f.name
-        subprocess.check_call([sys.executable, python_file], env=env)
+        subprocess.check_call(cmd, env=env)
         entry_separator, help_separator = _get_field_separators(env)
         return _split_argcomplete_ouput(f.read(), entry_separator, help_separator)
 
-def _get_completions_from_downward(downward, options, prefix):
+def _get_completions_from_downward(build, options, prefix):
+    try:
+        search_command = get_search_command(build)
+    except MissingBuildError:
+        return {}
+
     entry_separator, help_separator = _get_field_separators(os.environ)
     help_separator = help_separator or ":"
-    simulated_commandline = [str(downward)] + options + [prefix]
-    comp_line = " ".join(simulated_commandline)
-    comp_point = str(len(comp_line))
-    comp_cword = str(len(simulated_commandline) - 1)
-    cmd = [str(downward), "--bash-complete", entry_separator, help_separator,
-           comp_point, comp_line, comp_cword] + simulated_commandline
+    comp_point, comp_line, comp_cword, comp_words = _get_bash_completion_args(
+        search_command, options, prefix)
+    cmd = [str(x) for x in search_command] + ["--bash-complete", entry_separator, help_separator,
+           comp_point, comp_line, comp_cword] + comp_words
     output = subprocess.check_output(cmd, text=True)
     return _split_argcomplete_ouput(output, entry_separator, help_separator)
 
 
-def _get_completions_from_translator(translator, options, prefix):
-    simulated_commandline = [str(translator)] + options + [prefix]
-    comp_line = " ".join(simulated_commandline)
-    comp_point = len(comp_line)
-    return _call_argcomplete(translator, comp_line, comp_point)
+def _get_completions_from_translator(build, options, prefix):
+    try:
+        translate_command = get_translate_command(build)
+    except MissingBuildError:
+        return {}
+
+    # We add domain and problem as dummy file names because otherwise, the
+    # translators tab completion will suggest filenames which we don't want at
+    # this point. Technically, file names should come after any options we
+    # complete but to consider them in the completion, they have to be to the
+    # left of the cursor position and to the left of any options that take
+    # parameters.
+    comp_point, comp_line, _, _ = _get_bash_completion_args(
+        translate_command, ["domain", "problem"] + options, prefix)
+    return _call_argcomplete(translate_command, comp_line, comp_point)
 
 
 def enable(parser):