Fix path behaviour, especially on Windows (#65)

Author: matterhorn103

avo_xtb/config.py

@@ -192,7 +192,7 @@ def update_config(avo_input: dict):
             }
             # Pass back to Avogadro to display to user
             print(json.dumps(output))
-        easyxtb.config["calcs_dir"] = str(easyxtb.CALCS_DIR)
+        easyxtb.config["calcs_dir"] = str(easyxtb.CALCS_DIR.as_posix())
 
     # Save any changes to binary paths
     for program in ["xtb", "crest"]:
@@ -204,7 +204,7 @@ def update_config(avo_input: dict):
                 easyxtb.CREST_BIN = bin_path
             else:
                 easyxtb.XTB_BIN = bin_path
-            easyxtb.config[f"{program}_bin"] = str(bin_path)
+            easyxtb.config[f"{program}_bin"] = str(bin_path.as_posix())
 
     # Update other options that don't need coercing
     for option in ["n_proc", "energy_units", "solvent", "opt_lvl"]:

avo_xtb/install_crest.py

@@ -11,7 +11,7 @@
 from pathlib import Path
 
 from support import easyxtb
-from install_xtb import get_bin, link_bin
+from install_xtb import get_bin
 
 
 logger = logging.getLogger(__name__)
@@ -103,12 +103,10 @@
             except PermissionError:
                 output = {"message": "Install directory is not writeable"}
             else:
-                crest_folder = get_bin(crest_urls[platform.system()], install_dir)
-                if crest_folder:
-                    link_bin(crest_folder/"crest")
+                crest_bin = get_bin(crest_urls[platform.system()], install_dir)
                 # Report success
                 output = {
-                    "message": "CREST was successfully installed.\nPlease restart Avogadro."
+                    "message": f"CREST was successfully installed to\n{crest_bin}\nPlease restart Avogadro."
                 }
 
         # Pass result back to Avogadro to display to user

avo_xtb/install_xtb.py

@@ -56,7 +56,7 @@ def extract_tar(archive, target_dir) -> Path:
     return extracted
 
 
-def get_bin(url, install_dir):
+def get_bin(url: str, install_dir: Path) -> Path:
     # Don't install if url not valid
     if url[0:5] != "https":
         return
@@ -66,35 +66,19 @@ def get_bin(url, install_dir):
         folder = extract_zip(archive, install_dir)
     else:
         folder = extract_tar(archive, install_dir)
+    # Remove archive
+    archive.unlink()
     # Rename unzipped folder to a non-versioned name we have chosen
+    # Store appropriate path to binary
     if "crest" in folder.name:
-        folder = folder.rename(folder.with_name("crest-dist"))
+        folder = folder.rename(folder.with_name("crest"))
+        bin_name = "crest.exe" if platform.system() == "Windows" else "crest"
+        bin_path = folder/bin_name
     else:
         folder = folder.rename(folder.with_name("xtb-dist"))
-    # Remove archive
-    archive.unlink()
-    return folder
-
-
-def link_bin(bin_path):
-    if "crest" in bin_path.name:
-        bin_name = "crest"
-    else:
-        bin_name = "xtb"
-    # Check Windows
-    if bin_path.with_suffix(".exe").exists():
-        bin_path = bin_path.with_suffix(".exe")
-    # Link
-    if bin_name == "xtb":
-        easyxtb.XTB_BIN = easyxtb.BIN_DIR / bin_path.name
-        easyxtb.XTB_BIN.symlink_to(bin_path)
-    elif bin_name == "crest":
-        easyxtb.CREST_BIN = easyxtb.BIN_DIR / bin_path.name
-        easyxtb.CREST_BIN.symlink_to(bin_path)
-    # Add to config
-    easyxtb.config[f"{bin_name}_bin"] = str(easyxtb.BIN_DIR / bin_path.name)
-    # Save config
-    easyxtb.configuration.save_config()
+        bin_name = "xtb.exe" if platform.system() == "Windows" else "xtb"
+        bin_path = folder/f"bin/{bin_name}"  
+    return bin_path
 
 
 if __name__ == "__main__":
@@ -175,12 +159,10 @@ def link_bin(bin_path):
             except PermissionError:
                 output = {"message": "Install directory is not writeable"}
             else:
-                xtb_folder = get_bin(xtb_urls[platform.system()], install_dir)
-                if xtb_folder:
-                    link_bin(xtb_folder/"bin/xtb")
+                xtb_bin = get_bin(xtb_urls[platform.system()], install_dir)
                 # Report success
                 output = {
-                    "message": "xtb was successfully installed.\nPlease restart Avogadro."
+                    "message": f"xtb was successfully installed to\n{xtb_bin}\nPlease restart Avogadro."
                 }
 
         # Pass result back to Avogadro to display to user

easyxtb/pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "easyxtb"
-version = "0.8.0"
+version = "0.8.1"
 description = "A Python API for xtb (and CREST)."
 readme = "README.md"
 requires-python = ">=3.10"

easyxtb/src/easyxtb/configuration.py

@@ -113,7 +113,7 @@ def save_config():
 
 # Current version of package
 # Hard code for now, obviously not ideal though
-easyxtb_VERSION = "0.8.0"
+easyxtb_VERSION = "0.8.1"
 
 def update_config():
     """Ensure that any config options added in later versions of the package are in
@@ -145,45 +145,23 @@ def update_config():
 BIN_DIR.mkdir(parents=True, exist_ok=True)
 
 
-def find_xtb() -> Path | None:
-    """Return path to xtb binary as Path object, or None."""
-    if (BIN_DIR/"xtb").exists():
-        # Normal binary or symlink to it
-        xtb = BIN_DIR/"xtb"
-    elif (BIN_DIR/"xtb.exe").exists():
-        # Windows
-        xtb = BIN_DIR/"xtb.exe"
-    elif (BIN_DIR/"xtb-dist").exists():
-        # Whole xtb distribution folder with nested binary directory
-        xtb = BIN_DIR/"xtb-dist/bin/xtb"
-        # Or, on Windows
-        if platform.system() == "Windows":
-            xtb = xtb.with_suffix(".exe")
-    elif which("xtb") is not None:
-        # Check PATH
-        xtb = Path(which("xtb"))
-    else:
-        xtb = None
-    logger.debug(f"xtb binary location determined to be: {xtb}")
-    return xtb
-
-
-def find_crest() -> Path | None:
-    """Return path to crest binary as Path object, or None"""
-    if (BIN_DIR/"crest").exists():
-        crest = BIN_DIR/"crest"
-    elif (BIN_DIR/"crest/crest").exists():
-        crest = BIN_DIR/"crest/crest"
-    # Currently there is no Windows binary for crest but let's assume there will be
-    elif (BIN_DIR/"crest.exe").exists():
-        crest = BIN_DIR/"crest.exe"
-    elif which("crest") is not None:
-        # Check PATH
-        crest = Path(which("crest"))
-    else:
-        crest = None
-    logger.debug(f"crest binary location determined to be: {crest}")
-    return crest
+def find_bin(program: str) -> Path | None:
+    """Return path to xtb or CREST binary as Path object, or None."""
+    bin_name = f"{program}.exe" if platform.system() == "Windows" else program
+    bin_path = None
+    for possible_location in [
+        BIN_DIR/bin_name,  # Normal binary or symlink to it
+        BIN_DIR/program/bin_name,  # Old layout for xtb, current for CREST
+        BIN_DIR/f"{program}-dist/bin/{bin_name}",  # Whole xtb distribution folder with nested binary directory
+    ]:
+        if possible_location.exists() and not possible_location.is_dir():
+            bin_path = possible_location
+            break
+    # Otherwise check the PATH
+    if not bin_path and which(bin_name) is not None:
+        bin_path = Path(which(bin_name))
+    logger.debug(f"{bin_name} binary location determined to be: {bin_path}")
+    return bin_path
 
 
 # Initialize and find the various binaries
@@ -194,15 +172,15 @@ def find_crest() -> Path | None:
 if "xtb_bin" in config:
     XTB_BIN = Path(config["xtb_bin"])
     if not XTB_BIN.exists():
-        XTB_BIN = find_xtb()
+        XTB_BIN = find_bin("xtb")
 else:
-    XTB_BIN = find_xtb()
+    XTB_BIN = find_bin("xtb")
 if "crest_bin" in config:
     CREST_BIN = Path(config["crest_bin"])
     if not CREST_BIN.exists():
-        CREST_BIN = find_crest()
+        CREST_BIN = find_bin("crest")
 else:
-    CREST_BIN = find_crest()
+    CREST_BIN = find_bin("crest")
 
 
 if XTB_BIN is not None:

plugin.json

@@ -1,6 +1,6 @@
 {
   "author": "Matthew J. Milner",
-  "version": "0.8.0",
+  "version": "0.8.1",
   "url": "https://github.com/matterhorn103/avo_xtb",
   "name": "avo_xtb",
   "description": "Access the power of xtb through the Avogadro interface.",

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "avo_xtb"
-version = "0.8.0"
+version = "0.8.1"
 description = "A convenient interface to xtb (and CREST) in Avogadro 2"
 readme = "README.md"
 requires-python = ">=3.10"