Skip to content

Commit

Permalink
Lock CACHE_DIR while reading and writing
Browse files Browse the repository at this point in the history
  • Loading branch information
superatomic committed Sep 10, 2023
1 parent 6d7a4df commit b487466
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 16 deletions.
20 changes: 19 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ include = ["generate_completions.sh", "tldr-man.1"]
python = "^3.10.4"
click = "^8.1.7"
click-help-colors = "^0.9.2"
filelock = "^3.12.3"
requests = "^2.28.1"

[tool.poetry.group.dev.dependencies]
Expand Down
6 changes: 5 additions & 1 deletion src/tldr_man/languages.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@

from tldr_man.color import style_input
from tldr_man.errors import Fail
from tldr_man.pages import CACHE_DIR, language_directory_to_code, iter_dirs
from tldr_man.pages import CACHE_DIR, cache_dir_lock, language_directory_to_code, iter_dirs


@cache_dir_lock
def all_languages() -> Iterator[str]:
"""Returns an iterator of all languages directory names."""
return map(get_language_directory, all_language_codes())


@cache_dir_lock
def all_language_codes() -> Iterator[str]:
"""Returns an iterator of all language codes, based on all language directories."""
return (
Expand Down Expand Up @@ -76,6 +78,7 @@ def _language_code_as_parts(language_code: str) -> tuple[str, str]:
return language, region


@cache_dir_lock
def get_language_directory(language_code: str) -> str:
"""Get the name of the directory for a language code."""
language, region = _language_code_as_parts(language_code)
Expand All @@ -89,6 +92,7 @@ def get_language_directory(language_code: str) -> str:
return f'pages.{language}'


@cache_dir_lock
def get_locales(ctx: Context) -> list[str]:
"""Return an ordered list of the languages that the user specifies."""
language = ctx.params.get('language')
Expand Down
9 changes: 7 additions & 2 deletions src/tldr_man/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

from tldr_man import pages
from tldr_man.color import HELP_COLORS
from tldr_man.pages import cache_dir_lock
from tldr_man.shell_completion import page_shell_complete, language_shell_complete
from tldr_man.languages import get_locales
from tldr_man.platforms import get_page_sections, TLDR_PLATFORMS
Expand Down Expand Up @@ -82,6 +83,7 @@ def require_tldr_cache(func: Callable[Concatenate[list[str], list[str], P], T])
func(locales, platforms, ...) --> func(...)
"""
@wraps(func)
@cache_dir_lock
def wrapper(*args: P.args, **kwargs: P.kwargs) -> T:
pages.verify_tldr_cache_exists()

Expand Down Expand Up @@ -163,8 +165,11 @@ def subcommand_manpath(locales: list[str], page_sections: list[str]) -> None:
def cli(locales: list[str], page_sections: list[str], page: list[str]) -> None:
"""TLDR client that displays tldr-pages as manpages"""
page_name = '-'.join(page).strip().lower()
page_path = pages.find_page(page_name, locales, page_sections)
pages.display_page(page_path)
page_file = pages.find_page(page_name, locales, page_sections)
with temp_file(page_file.name) as temp_page_file:
temp_page_file.write_bytes(page_file.read_bytes())
cache_dir_lock.release(force=True)
pages.display_page(temp_page_file)


if __name__ == '__main__':
Expand Down
30 changes: 19 additions & 11 deletions src/tldr_man/pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import requests
from click import style, echo, secho, progressbar, format_filename
from filelock import FileLock

from tldr_man.color import style_command, style_path, style_url
from tldr_man.errors import Fail, NoPageCache, ExternalCommandNotFound, PageNotFound, eprint
Expand Down Expand Up @@ -79,6 +80,7 @@ def get_cache_dir() -> Path:


CACHE_DIR: Path = get_cache_dir()
cache_dir_lock = FileLock(CACHE_DIR.parent / f'.{CACHE_DIR.name}.lock', timeout=2)


def download_archive(location: Path, url: str = ZIP_ARCHIVE_URL) -> None:
Expand Down Expand Up @@ -172,12 +174,13 @@ def to_manpage(tldr_page: zipfile.Path) -> tuple[str, str]:
res_file.write_text(manpage)

# Log whether the file was created, updated, or unchanged:
if original_dir is None or not (original_file := original_dir / filename).exists():
created += 1
elif original_file.read_text() != manpage:
updated += 1
else:
unchanged += 1
with cache_dir_lock:
if original_dir is None or not (original_file := original_dir / filename).exists():
created += 1
elif original_file.read_text() != manpage:
updated += 1
else:
unchanged += 1
except:
# If an exception occurs, such as a KeyboardInterrupt or an actual Exception,
# shutdown the pool *without* waiting for any remaining futures to finish. This will prevent the
Expand All @@ -190,12 +193,13 @@ def to_manpage(tldr_page: zipfile.Path) -> tuple[str, str]:
# Now that the updated cache has been generated, remove the old cache, make sure the parent directory exists,
# and move the new cache into the correct directory from the temporary directory.

ensure_cache_dir_update_safety()
with suppress(FileNotFoundError):
rmtree(CACHE_DIR)
with cache_dir_lock:
ensure_cache_dir_update_safety()
with suppress(FileNotFoundError):
rmtree(CACHE_DIR)

makedirs(CACHE_DIR.parent, exist_ok=True)
move(temp_cache_dir, CACHE_DIR)
makedirs(CACHE_DIR.parent, exist_ok=True)
move(temp_cache_dir, CACHE_DIR)

# Display the details for the cache update:
echo(', '.join([
Expand All @@ -209,6 +213,7 @@ def to_manpage(tldr_page: zipfile.Path) -> tuple[str, str]:
EXPECTED_CACHE_CONTENT_PATTERN = re.compile(r'^pages(?:\.\w{2}(?:_\w{2})?)?$')


@cache_dir_lock
def ensure_cache_dir_update_safety() -> None:
"""Make sure not to overwrite directories with user data in them."""

Expand Down Expand Up @@ -280,6 +285,7 @@ def pandoc_exists() -> bool:
return False


@cache_dir_lock
def verify_tldr_cache_exists() -> None:
"""Display a specific message if the tldr manpage cache doesn't exist yet, and then exit."""
if not CACHE_DIR.exists():
Expand All @@ -296,6 +302,7 @@ def display_page(page: Path) -> None:
raise


@cache_dir_lock
def find_page(page_name: str, /, locales: Iterable[str], page_sections: Iterable[str]) -> Path:
for search_dir in get_dir_search_order(locales, page_sections):
page = search_dir / (page_name + '.' + MANPAGE_SECTION)
Expand All @@ -316,6 +323,7 @@ def unique(items: Iterable[T]) -> Iterator[T]:
yield item


@cache_dir_lock
def get_dir_search_order(locales: Iterable[str], page_sections: Iterable[str]) -> Iterator[Path]:
return unique(
CACHE_DIR / locale / section / ('man' + MANPAGE_SECTION)
Expand Down
4 changes: 3 additions & 1 deletion src/tldr_man/shell_completion.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
from click import Context, Parameter
from click.shell_completion import CompletionItem

from tldr_man.pages import CACHE_DIR, get_dir_search_order
from tldr_man.pages import CACHE_DIR, cache_dir_lock, get_dir_search_order
from tldr_man.languages import get_locales, all_language_codes
from tldr_man.platforms import get_page_sections


@cache_dir_lock
def page_shell_complete(ctx: Context, param: Parameter, incomplete: str) -> list[CompletionItem]:
if not CACHE_DIR.exists() or param.name is None: # the `param.name is None` check makes the type checker happy
return []
Expand All @@ -40,6 +41,7 @@ def page_shell_complete(ctx: Context, param: Parameter, incomplete: str) -> list
]


@cache_dir_lock
def language_shell_complete(_ctx: Context, _param: Parameter, _incomplete: str) -> list[CompletionItem]:
if not CACHE_DIR.exists():
return []
Expand Down

0 comments on commit b487466

Please sign in to comment.