Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cache file length #4176

Merged
merged 4 commits into from Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.md
Expand Up @@ -18,6 +18,9 @@

<!-- Changes to how Black can be configured -->

- Shorten the length of the name of the cache file to fix crashes on file systems that
do not support long paths (#4176)

### Packaging

<!-- Changes to how Black is packaged, such as dependency requirements -->
Expand Down
9 changes: 8 additions & 1 deletion src/black/cache.py
Expand Up @@ -13,6 +13,7 @@

from _black_version import version as __version__
from black.mode import Mode
from black.output import err

if sys.version_info >= (3, 11):
from typing import Self
Expand Down Expand Up @@ -64,7 +65,13 @@ def read(cls, mode: Mode) -> Self:
resolve the issue.
"""
cache_file = get_cache_file(mode)
if not cache_file.exists():
try:
exists = cache_file.exists()
except OSError as e:
# Likely file too long; see #4172 and #4174
err(f"Unable to read cache file {cache_file} due to {e}")
return cls(mode, cache_file)
if not exists:
return cls(mode, cache_file)

with cache_file.open("rb") as fobj:
Expand Down
21 changes: 17 additions & 4 deletions src/black/mode.py
Expand Up @@ -192,6 +192,9 @@ class Deprecated(UserWarning):
"""Visible deprecation warning."""


_MAX_CACHE_KEY_PART_LENGTH: Final = 32


@dataclass
class Mode:
target_versions: Set[TargetVersion] = field(default_factory=set)
Expand Down Expand Up @@ -228,6 +231,19 @@ def get_cache_key(self) -> str:
)
else:
version_str = "-"
if len(version_str) > _MAX_CACHE_KEY_PART_LENGTH:
version_str = sha256(version_str.encode()).hexdigest()[
:_MAX_CACHE_KEY_PART_LENGTH
]
features_and_magics = (
",".join(sorted(f.name for f in self.enabled_features))
+ "@"
+ ",".join(sorted(self.python_cell_magics))
)
if len(features_and_magics) > _MAX_CACHE_KEY_PART_LENGTH:
features_and_magics = sha256(features_and_magics.encode()).hexdigest()[
:_MAX_CACHE_KEY_PART_LENGTH
]
parts = [
version_str,
str(self.line_length),
Expand All @@ -236,10 +252,7 @@ def get_cache_key(self) -> str:
str(int(self.is_ipynb)),
str(int(self.skip_source_first_line)),
str(int(self.magic_trailing_comma)),
sha256(
(",".join(sorted(f.name for f in self.enabled_features))).encode()
).hexdigest(),
str(int(self.preview)),
sha256((",".join(sorted(self.python_cell_magics))).encode()).hexdigest(),
features_and_magics,
]
return ".".join(parts)
25 changes: 25 additions & 0 deletions tests/test_black.py
Expand Up @@ -44,6 +44,7 @@
from black import re_compile_maybe_verbose as compile_pattern
from black.cache import FileData, get_cache_dir, get_cache_file
from black.debug import DebugVisitor
from black.mode import Mode, Preview
from black.output import color_diff, diff
from black.report import Report

Expand Down Expand Up @@ -2065,6 +2066,30 @@ def test_get_cache_dir(
monkeypatch.setenv("BLACK_CACHE_DIR", str(workspace2))
assert get_cache_dir().parent == workspace2

def test_cache_file_length(self) -> None:
cases = [
DEFAULT_MODE,
# all of the target versions
Mode(target_versions=set(TargetVersion)),
# all of the features
Mode(enabled_features=set(Preview)),
# all of the magics
Mode(python_cell_magics={f"magic{i}" for i in range(500)}),
# all of the things
Mode(
target_versions=set(TargetVersion),
enabled_features=set(Preview),
python_cell_magics={f"magic{i}" for i in range(500)},
),
]
for case in cases:
cache_file = get_cache_file(case)
# Some common file systems enforce a maximum path length
# of 143 (issue #4174). We can't do anything if the directory
# path is too long, but ensure the name of the cache file itself
# doesn't get too crazy.
assert len(cache_file.name) <= 96

def test_cache_broken_file(self) -> None:
mode = DEFAULT_MODE
with cache_dir() as workspace:
Expand Down