Repository: materialsproject/custodian (python)

Issue:
[Bug]: The `error.tar.gz` files unpack in a weird way when a custom `directory` is set
### Code snippet

This is a continuation of #378.

When backing up files via an absolute path, the unpacking process is not ideal.


```python
from custodian.utils import backup

backup(["INCAR"], directory="/path/to/test")
```

Doing `tar -xvf error.1.tar.gz` will unpack into `path/to/test/INCAR` in the current working directory. This is due to an oddity of how `tar.add()` works. It adds the file to the archive with the full internal path (but for some reason without the `/` preceding it...). The `arcname` keyword argument has to be supplied to fix this.


### Version

2025.5.12


Pull request description:
Fix unpacking of `error.tar.gz` files when a custom `directory` is set
Closes #380.

**Fixes:**
- The `error.tar.gz` files now unpack without including the full file path of the original files that were tar'd.

**Changes**:
- I have made it so that doing `tar -xvf error1.tar.gz` will not accidentally overwrite files in the parent directory. The files will be written out to `error.1/*` instead.

**Maintenance**:
- I added a few missing tests and a regression test for #380.

Patch:
diff --git a/src/custodian/utils.py b/src/custodian/utils.py
index b081a040..19215a7d 100644
--- a/src/custodian/utils.py
+++ b/src/custodian/utils.py
@@ -21,12 +21,13 @@ def backup(filenames, prefix="error", directory="./") -> None:
         directory (str): directory where the files exist
     """
     num = max([0] + [int(file.split(".")[-3]) for file in glob(os.path.join(directory, f"{prefix}.*.tar.gz"))])
-    filename = os.path.join(directory, f"{prefix}.{num + 1}.tar.gz")
+    prefix = f"{prefix}.{num + 1}"
+    filename = os.path.join(directory, f"{prefix}.tar.gz")
     logging.info(f"Backing up run to {filename}.")
     with tarfile.open(filename, "w:gz") as tar:
         for fname in filenames:
             for file in glob(os.path.join(directory, fname)):
-                tar.add(file)
+                tar.add(file, arcname=os.path.join(prefix, os.path.basename(file)))
 
 
 def get_execution_host_info():


Test patch:
diff --git a/tests/test_utils.py b/tests/test_utils.py
index fb7df302..d8f97d3a 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -30,7 +30,37 @@ def test_cache_and_clear() -> None:
     assert n_calls == 3
 
 
-def test_backup(tmp_path) -> None:
+def test_backup(tmp_path, monkeypatch) -> None:
+    monkeypatch.chdir(tmp_path)
+    with open("INCAR", "w") as f:
+        f.write("This is a test file.")
+
+    backup(["INCAR"])
+
+    assert Path("error.1.tar.gz").exists()
+    with tarfile.open("error.1.tar.gz", "r:gz") as tar:
+        assert len(tar.getmembers()) == 1
+        assert tar.getnames() == ["error.1/INCAR"]
+
+
+def test_backup_with_glob(tmp_path, monkeypatch) -> None:
+    monkeypatch.chdir(tmp_path)
+    with open("INCAR", "w") as f:
+        f.write("This is a test file.")
+    with open("POSCAR", "w") as f:
+        f.write("This is a test file.")
+    with open("garbage", "w") as f:
+        f.write("This is a test file.")
+
+    backup(["*CAR"])
+
+    assert Path("error.1.tar.gz").exists()
+    with tarfile.open("error.1.tar.gz", "r:gz") as tar:
+        assert len(tar.getmembers()) == 2
+        assert tar.getnames() == ["error.1/INCAR", "error.1/POSCAR"]
+
+
+def test_backup_with_directory(tmp_path) -> None:
     with open(tmp_path / "INCAR", "w") as f:
         f.write("This is a test file.")
 
@@ -38,4 +68,5 @@ def test_backup(tmp_path) -> None:
 
     assert Path(tmp_path / "error.1.tar.gz").exists()
     with tarfile.open(tmp_path / "error.1.tar.gz", "r:gz") as tar:
-        assert len(tar.getmembers()) > 0
+        assert len(tar.getmembers()) == 1
+        assert tar.getnames() == ["error.1/INCAR"]