Fixes for #454
This commit is contained in:
@@ -1,3 +1,3 @@
|
|||||||
""" version info """
|
""" version info """
|
||||||
|
|
||||||
__version__ = "0.42.25"
|
__version__ = "0.42.26"
|
||||||
|
|||||||
@@ -23,12 +23,14 @@ EXIFTOOL_STAYOPEN_EOF_LEN = len(EXIFTOOL_STAYOPEN_EOF)
|
|||||||
# list of exiftool processes to cleanup when exiting or when terminate is called
|
# list of exiftool processes to cleanup when exiting or when terminate is called
|
||||||
EXIFTOOL_PROCESSES = []
|
EXIFTOOL_PROCESSES = []
|
||||||
|
|
||||||
|
|
||||||
@atexit.register
|
@atexit.register
|
||||||
def terminate_exiftool():
|
def terminate_exiftool():
|
||||||
"""Terminate any running ExifTool subprocesses; call this to cleanup when done using ExifTool """
|
"""Terminate any running ExifTool subprocesses; call this to cleanup when done using ExifTool """
|
||||||
for proc in EXIFTOOL_PROCESSES:
|
for proc in EXIFTOOL_PROCESSES:
|
||||||
proc._stop_proc()
|
proc._stop_proc()
|
||||||
|
|
||||||
|
|
||||||
@lru_cache(maxsize=1)
|
@lru_cache(maxsize=1)
|
||||||
def get_exiftool_path():
|
def get_exiftool_path():
|
||||||
""" return path of exiftool, cache result """
|
""" return path of exiftool, cache result """
|
||||||
@@ -70,15 +72,14 @@ class _ExifToolProc:
|
|||||||
self._exiftool = exiftool or get_exiftool_path()
|
self._exiftool = exiftool or get_exiftool_path()
|
||||||
self._start_proc()
|
self._start_proc()
|
||||||
|
|
||||||
EXIFTOOL_PROCESSES.append(self)
|
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def process(self):
|
def process(self):
|
||||||
""" return the exiftool subprocess """
|
""" return the exiftool subprocess """
|
||||||
if self._process_running:
|
if self._process_running:
|
||||||
return self._process
|
return self._process
|
||||||
else:
|
else:
|
||||||
raise ValueError("exiftool process is not running")
|
self._start_proc()
|
||||||
|
return self._process
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def pid(self):
|
def pid(self):
|
||||||
@@ -116,15 +117,21 @@ class _ExifToolProc:
|
|||||||
)
|
)
|
||||||
self._process_running = True
|
self._process_running = True
|
||||||
|
|
||||||
|
EXIFTOOL_PROCESSES.append(self)
|
||||||
|
|
||||||
def _stop_proc(self):
|
def _stop_proc(self):
|
||||||
""" stop the exiftool process if it's running, otherwise, do nothing """
|
""" stop the exiftool process if it's running, otherwise, do nothing """
|
||||||
|
|
||||||
if not self._process_running:
|
if not self._process_running:
|
||||||
return
|
return
|
||||||
|
|
||||||
self._process.stdin.write(b"-stay_open\n")
|
try:
|
||||||
self._process.stdin.write(b"False\n")
|
self._process.stdin.write(b"-stay_open\n")
|
||||||
self._process.stdin.flush()
|
self._process.stdin.write(b"False\n")
|
||||||
|
self._process.stdin.flush()
|
||||||
|
except Exception as e:
|
||||||
|
pass
|
||||||
|
|
||||||
try:
|
try:
|
||||||
self._process.communicate(timeout=5)
|
self._process.communicate(timeout=5)
|
||||||
except subprocess.TimeoutExpired:
|
except subprocess.TimeoutExpired:
|
||||||
@@ -134,9 +141,6 @@ class _ExifToolProc:
|
|||||||
del self._process
|
del self._process
|
||||||
self._process_running = False
|
self._process_running = False
|
||||||
|
|
||||||
def __del__(self):
|
|
||||||
self._stop_proc()
|
|
||||||
|
|
||||||
|
|
||||||
class ExifTool:
|
class ExifTool:
|
||||||
""" Basic exiftool interface for reading and writing EXIF tags """
|
""" Basic exiftool interface for reading and writing EXIF tags """
|
||||||
@@ -162,9 +166,12 @@ class ExifTool:
|
|||||||
# if running as a context manager, self._context_mgr will be True
|
# if running as a context manager, self._context_mgr will be True
|
||||||
self._context_mgr = False
|
self._context_mgr = False
|
||||||
self._exiftoolproc = _ExifToolProc(exiftool=exiftool)
|
self._exiftoolproc = _ExifToolProc(exiftool=exiftool)
|
||||||
self._process = self._exiftoolproc.process
|
|
||||||
self._read_exif()
|
self._read_exif()
|
||||||
|
|
||||||
|
@property
|
||||||
|
def _process(self):
|
||||||
|
return self._exiftoolproc.process
|
||||||
|
|
||||||
def setvalue(self, tag, value):
|
def setvalue(self, tag, value):
|
||||||
"""Set tag to value(s); if value is None, will delete tag
|
"""Set tag to value(s); if value is None, will delete tag
|
||||||
|
|
||||||
|
|||||||
@@ -3,9 +3,10 @@ import os
|
|||||||
import pathlib
|
import pathlib
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from applescript import AppleScript
|
|
||||||
from photoscript.utils import ditto
|
from photoscript.utils import ditto
|
||||||
|
|
||||||
|
import osxphotos
|
||||||
|
from applescript import AppleScript
|
||||||
from osxphotos.exiftool import _ExifToolProc
|
from osxphotos.exiftool import _ExifToolProc
|
||||||
|
|
||||||
|
|
||||||
@@ -63,7 +64,9 @@ def pytest_collection_modifyitems(config, items):
|
|||||||
# --addalbum given in cli: do not skip addalbum tests (these require interactive test)
|
# --addalbum given in cli: do not skip addalbum tests (these require interactive test)
|
||||||
return
|
return
|
||||||
|
|
||||||
skip_addalbum = pytest.mark.skip(reason="need --addalbum option and MacOS Catalina to run")
|
skip_addalbum = pytest.mark.skip(
|
||||||
|
reason="need --addalbum option and MacOS Catalina to run"
|
||||||
|
)
|
||||||
for item in items:
|
for item in items:
|
||||||
if "addalbum" in item.keywords:
|
if "addalbum" in item.keywords:
|
||||||
item.add_marker(skip_addalbum)
|
item.add_marker(skip_addalbum)
|
||||||
|
|||||||
@@ -34,9 +34,9 @@ UUID_BURST_ALBUM = {
|
|||||||
"TestBurst2/IMG_9814.JPG",
|
"TestBurst2/IMG_9814.JPG",
|
||||||
],
|
],
|
||||||
"38F8F30C-FF6D-49DA-8092-18497F1D6628": [
|
"38F8F30C-FF6D-49DA-8092-18497F1D6628": [
|
||||||
"TestBurst/IMG_9812.JPG",
|
"TestBurst/IMG_9812.JPG",
|
||||||
"TestBurst/IMG_9813.JPG",
|
"TestBurst/IMG_9813.JPG",
|
||||||
"TestBurst/IMG_9814.JPG", # in my personal library, "38F8F30C-FF6D-49DA-8092-18497F1D6628"
|
"TestBurst/IMG_9814.JPG", # in my personal library, "38F8F30C-FF6D-49DA-8092-18497F1D6628"
|
||||||
"TestBurst/IMG_9815.JPG",
|
"TestBurst/IMG_9815.JPG",
|
||||||
"TestBurst/IMG_9816.JPG",
|
"TestBurst/IMG_9816.JPG",
|
||||||
"TestBurst2/IMG_9814.JPG",
|
"TestBurst2/IMG_9814.JPG",
|
||||||
@@ -1229,16 +1229,12 @@ def test_export_exiftool_path():
|
|||||||
import shutil
|
import shutil
|
||||||
import tempfile
|
import tempfile
|
||||||
from osxphotos.cli import export
|
from osxphotos.cli import export
|
||||||
from osxphotos.exiftool import ExifTool, get_exiftool_path
|
from osxphotos.exiftool import ExifTool
|
||||||
|
|
||||||
runner = CliRunner()
|
runner = CliRunner()
|
||||||
cwd = os.getcwd()
|
cwd = os.getcwd()
|
||||||
# pylint: disable=not-context-manager
|
# pylint: disable=not-context-manager
|
||||||
with runner.isolated_filesystem():
|
with runner.isolated_filesystem():
|
||||||
tempdir = tempfile.TemporaryDirectory()
|
|
||||||
exiftool_source = get_exiftool_path()
|
|
||||||
exiftool_path = os.path.join(tempdir.name, "myexiftool")
|
|
||||||
shutil.copy2(exiftool_source, exiftool_path)
|
|
||||||
for uuid in CLI_EXIFTOOL:
|
for uuid in CLI_EXIFTOOL:
|
||||||
result = runner.invoke(
|
result = runner.invoke(
|
||||||
export,
|
export,
|
||||||
@@ -1250,11 +1246,11 @@ def test_export_exiftool_path():
|
|||||||
"--uuid",
|
"--uuid",
|
||||||
f"{uuid}",
|
f"{uuid}",
|
||||||
"--exiftool-path",
|
"--exiftool-path",
|
||||||
exiftool_path,
|
exiftool,
|
||||||
],
|
],
|
||||||
)
|
)
|
||||||
assert result.exit_code == 0
|
assert result.exit_code == 0
|
||||||
assert f"exiftool path: {exiftool_path}" in result.output
|
assert f"exiftool path: {exiftool}" in result.output
|
||||||
files = glob.glob("*")
|
files = glob.glob("*")
|
||||||
assert sorted(files) == sorted([CLI_EXIFTOOL[uuid]["File:FileName"]])
|
assert sorted(files) == sorted([CLI_EXIFTOOL[uuid]["File:FileName"]])
|
||||||
|
|
||||||
@@ -1290,9 +1286,6 @@ def test_export_exiftool_path_render_template():
|
|||||||
cwd = os.getcwd()
|
cwd = os.getcwd()
|
||||||
# pylint: disable=not-context-manager
|
# pylint: disable=not-context-manager
|
||||||
with runner.isolated_filesystem():
|
with runner.isolated_filesystem():
|
||||||
tempdir = tempfile.TemporaryDirectory()
|
|
||||||
exiftool_path = os.path.join(tempdir.name, "myexiftool")
|
|
||||||
shutil.copy2(exiftool_source, exiftool_path)
|
|
||||||
for uuid in CLI_EXIFTOOL:
|
for uuid in CLI_EXIFTOOL:
|
||||||
result = runner.invoke(
|
result = runner.invoke(
|
||||||
export,
|
export,
|
||||||
@@ -1305,7 +1298,7 @@ def test_export_exiftool_path_render_template():
|
|||||||
"--uuid",
|
"--uuid",
|
||||||
f"{uuid}",
|
f"{uuid}",
|
||||||
"--exiftool-path",
|
"--exiftool-path",
|
||||||
exiftool_path,
|
exiftool,
|
||||||
],
|
],
|
||||||
)
|
)
|
||||||
assert result.exit_code == 0
|
assert result.exit_code == 0
|
||||||
|
|||||||
@@ -425,8 +425,17 @@ def test_exiftool_terminate():
|
|||||||
import subprocess
|
import subprocess
|
||||||
|
|
||||||
exif1 = osxphotos.exiftool.ExifTool(TEST_FILE_ONE_KEYWORD)
|
exif1 = osxphotos.exiftool.ExifTool(TEST_FILE_ONE_KEYWORD)
|
||||||
|
|
||||||
|
ps = subprocess.run(["ps"], capture_output=True)
|
||||||
|
stdout = ps.stdout.decode("utf-8")
|
||||||
|
assert "exiftool -stay_open" in stdout
|
||||||
|
|
||||||
osxphotos.exiftool.terminate_exiftool()
|
osxphotos.exiftool.terminate_exiftool()
|
||||||
|
|
||||||
ps = subprocess.run(["ps"], capture_output=True)
|
ps = subprocess.run(["ps"], capture_output=True)
|
||||||
stdout = ps.stdout.decode("utf-8")
|
stdout = ps.stdout.decode("utf-8")
|
||||||
assert "exiftool -stay_open" not in stdout
|
assert "exiftool -stay_open" not in stdout
|
||||||
|
|
||||||
|
# verify we can create a new instance after termination
|
||||||
|
exif2 = osxphotos.exiftool.ExifTool(TEST_FILE_ONE_KEYWORD)
|
||||||
|
assert exif2.asdict()["IPTC:Keywords"] == "wedding"
|
||||||
|
|||||||
Reference in New Issue
Block a user