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 CacheDir initialization so works on 3.7 also #4695

Merged
merged 4 commits into from
Mar 13, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER
From Mats Wichmann:
- Fix typos in CCFLAGS test. Didn't affect the test itself, but
didn't correctly apply the DefaultEnvironment speedup.
- New CacheDir initialization code failed on Python 3.7 for unknown
reason (worked on 3.8+). Adjusted the approach a bit. Fixes #4694.


RELEASE 4.9.0 - Sun, 02 Mar 2025 17:22:20 -0700
Expand Down
3 changes: 3 additions & 0 deletions RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ FIXES

- List fixes of outright bugs

- New CacheDir initialization code failed on Python 3.7 for unknown
reason (worked on 3.8+). Adjusted the approach a bit. Fixes #4694.

IMPROVEMENTS
------------

Expand Down
32 changes: 19 additions & 13 deletions SCons/CacheDir.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import atexit
import json
import os
import shutil
import stat
import sys
import tempfile
Expand Down Expand Up @@ -76,7 +77,6 @@ def CacheRetrieveFunc(target, source, env) -> int:

def CacheRetrieveString(target, source, env) -> str:
t = target[0]
fs = t.fs
cd = env.get_CacheDir()
cachedir, cachefile = cd.cachepath(t)
if t.fs.exists(cachefile):
Expand Down Expand Up @@ -209,22 +209,28 @@ def _mkdir_atomic(self, path: str) -> bool:
if os.path.exists(directory):
return False

# TODO: tried to use TemporaryDirectory() here and the result as a
# context manager, but that fails on Python 3.7 (works on 3.8+) after
# the directory has successfully been renamed. Not sure why.
try:
tempdir = tempfile.TemporaryDirectory(dir=os.path.dirname(directory))
tempdir = tempfile.mkdtemp(dir=os.path.dirname(directory))
except OSError as e:
msg = "Failed to create cache directory " + path
raise SCons.Errors.SConsEnvironmentError(msg) from e
self._add_config(tempdir.name)
with tempdir:
try:
os.rename(tempdir.name, directory)
return True
except Exception as e:
# did someone else get there first?
if os.path.isdir(directory):
return False
msg = "Failed to create cache directory " + path
raise SCons.Errors.SConsEnvironmentError(msg) from e
self._add_config(tempdir)
try:
os.replace(tempdir, directory)
return True
except OSError as e:
# did someone else get there first? attempt cleanup.
if os.path.isdir(directory):
try:
shutil.rmtree(tempdir)
except Exception: # we tried, don't worry about it
pass
return False
msg = "Failed to create cache directory " + path
raise SCons.Errors.SConsEnvironmentError(msg) from e

def _readconfig(self, path: str) -> None:
"""Read the cache config from *path*.
Expand Down
Loading