Include compiler and env in engine name. Retire clear_binaries.

Currently the worker deletes cached binaries on restart. The
reason is that the user may have changed compilers, so that new
engine binaries would be matched against old engine binaries,
built with a different compiler.

In this PR we add compiler+version and a hash of selected
environment variables to the engine name so that
there is no risk of matching engine binaries built with
different compilers, or with  different environment variables.

It makes little sense to hash PATH and TEMP but they should be
included in the sandboxed environment used by the make command.

A possible enhancement (not in this PR) would be to also add
the full build command to the engine name so that engine binaries
could even be preserved across worker updates (a worker update
may change the build command).

Raise worker version to 251 (also server side)

server/fishtest/api.py

@@ -34,7 +34,7 @@
 according to the route/URL mapping defined in `__init__.py`.
 """
 
-WORKER_VERSION = 250
+WORKER_VERSION = 251
 
 
 @exception_view_config(HTTPException)

worker/games.py

@@ -721,6 +721,7 @@ def setup_engine(
     concurrency,
     compiler,
     global_cache,
+    env,
 ):
     """Download and build sources in a temporary directory then move exe to destination"""
     tmp_dir = Path(tempfile.mkdtemp(dir=worker_dir))
@@ -768,10 +769,6 @@ def setup_engine(
             f"COMP={comp}",
         ]
 
-        # append -DNNUE_EMBEDDING_OFF to existing CXXFLAGS environment variable, if any
-        cxx = os.environ.get("CXXFLAGS", "") + " -DNNUE_EMBEDDING_OFF"
-        env = dict(os.environ, CXXFLAGS=cxx.strip())
-
         with subprocess.Popen(
             cmd,
             env=env,
@@ -1208,7 +1205,6 @@ def run_games(
     run,
     task_id,
     pgn_file,
-    clear_binaries,
     global_cache,
 ):
     # This is the main fastchess driver.
@@ -1298,7 +1294,7 @@ def parse_options(s):
     # Clean up old engines (keeping the num_bkps most recent).
     worker_dir = Path(__file__).resolve().parent
     testing_dir = worker_dir / "testing"
-    num_bkps = 0 if clear_binaries else 50
+    num_bkps = 50
     try:
         engines = sorted(
             testing_dir.glob("stockfish_*" + EXE_SUFFIX),
@@ -1323,11 +1319,36 @@ def parse_options(s):
                     sep="",
                     file=sys.stderr,
                 )
+
+    def create_environment():
+        # OS and TEMP are necessary for msys2
+        white_set = {"PATH", "OS", "TEMP"}
+        env = {k: v for k, v in os.environ.items() if k in white_set}
+        env["CXXFLAGS"] = "-DNNUE_EMBEDDING_OFF"
+
+        # Do not hash directories such as PATH and TEMP
+        hash_set = {"OS", "CXXFLAGS"}
+        hashed_env = {k: v for k, v in env.items() if k in hash_set}
+
+        env_hash = hashlib.sha256(str(hashed_env).encode()).hexdigest()[0:10]
+        return env, env_hash
+
+    env, env_hash = create_environment()
+
+    # build signature
+    bs = (
+        worker_info["compiler"]
+        + "_"
+        + str(".".join([str(s) for s in worker_info["gcc_version"]]))
+        + "_env_"
+        + env_hash
+    )
     # Create new engines.
     sha_new = run["args"]["resolved_new"]
     sha_base = run["args"]["resolved_base"]
-    new_engine_name = "stockfish_" + sha_new
-    base_engine_name = "stockfish_" + sha_base
+
+    new_engine_name = "stockfish_" + sha_new + "_" + bs
+    base_engine_name = "stockfish_" + sha_base + "_" + bs
 
     new_engine = testing_dir / (new_engine_name + EXE_SUFFIX)
     base_engine = testing_dir / (base_engine_name + EXE_SUFFIX)
@@ -1344,6 +1365,7 @@ def parse_options(s):
             worker_info["concurrency"],
             worker_info["compiler"],
             global_cache,
+            env,
         )
     if not base_engine.exists():
         setup_engine(
@@ -1356,6 +1378,7 @@ def parse_options(s):
             worker_info["concurrency"],
             worker_info["compiler"],
             global_cache,
+            env,
         )
 
     os.chdir(testing_dir)
@@ -1565,7 +1588,7 @@ def make_player(arg):
                 "-engine",
                 "name=New-" + run["args"]["resolved_new"],
                 "tc={}".format(scaled_new_tc),
-                "cmd=./{}".format(new_engine_name),
+                "cmd={}".format(new_engine),
                 "dir=.",
             ]
             + new_options
@@ -1574,7 +1597,7 @@ def make_player(arg):
                 "-engine",
                 "name=Base-" + run["args"]["resolved_base"],
                 "tc={}".format(scaled_tc),
-                "cmd=./{}".format(base_engine_name),
+                "cmd={}".format(base_engine),
                 "dir=.",
             ]
             + base_options

worker/sri.txt

@@ -1 +1 @@
-{"__version": 250, "updater.py": "Mg+pWOgGA0gSo2TuXuuLCWLzwGwH91rsW1W3ixg3jYauHQpRMtNdGnCfuD1GqOhV", "worker.py": "rRWsXlFkv+whgfQS9IIJVEWnbquhlDa0DYRkkJ43/L7NTRR9pt2+sA2hqHH0x9M/", "games.py": "iYN33XDHqgfXnQheUtghu9GsuYhL0XU85/wUijSu87QtuwbyeAzJf2/Roqfatwgf"}
+{"__version": 251, "updater.py": "Mg+pWOgGA0gSo2TuXuuLCWLzwGwH91rsW1W3ixg3jYauHQpRMtNdGnCfuD1GqOhV", "worker.py": "mfLxjM3Pq6448qSPwbLq5lTqX1nxMVBZbz8iv4Pkn9jPzhwHULsNCtFWw7ThcPAy", "games.py": "IZmJeczQV7IDMOvWS1NqR/tPe+Zg3rI8D1QDYIz/dvRE8auUU66PEjyfhWf9nbNe"}

worker/worker.py

@@ -69,7 +69,7 @@
 MIN_CLANG_MAJOR = 8
 MIN_CLANG_MINOR = 0
 
-WORKER_VERSION = 250
+WORKER_VERSION = 251
 FILE_LIST = ["updater.py", "worker.py", "games.py"]
 HTTP_TIMEOUT = 30.0
 INITIAL_RETRY_TIME = 15.0
@@ -1298,7 +1298,6 @@ def fetch_and_handle_task(
     password,
     remote,
     current_state,
-    clear_binaries,
     global_cache,
     worker_lock,
 ):
@@ -1391,7 +1390,6 @@ def fetch_and_handle_task(
             run,
             task_id,
             pgn_file,
-            clear_binaries,
             global_cache,
         )
         success = True
@@ -1609,14 +1607,12 @@ def worker():
     # Start the main loop.
     delay = INITIAL_RETRY_TIME
     fish_exit = False
-    clear_binaries = True
     while current_state["alive"]:
         success = fetch_and_handle_task(
             worker_info,
             options.password,
             remote,
             current_state,
-            clear_binaries,
             options.global_cache,
             worker_lock,
         )
@@ -1637,7 +1633,6 @@ def worker():
                 safe_sleep(delay)
                 delay = min(MAX_RETRY_TIME, delay * 2)
         else:
-            clear_binaries = False
             delay = INITIAL_RETRY_TIME
 
     if fish_exit: