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

SCons 4.9.0 test suite failures with python 3.7 #4694

Closed
jcbrill opened this issue Mar 10, 2025 · 3 comments · Fixed by #4695
Closed

SCons 4.9.0 test suite failures with python 3.7 #4694

jcbrill opened this issue Mar 10, 2025 · 3 comments · Fixed by #4695

Comments

@jcbrill
Copy link
Contributor

jcbrill commented Mar 10, 2025

There are a number of test suite failures with SCons 4.9.0 and python 3.7. These failures do not appear in python 3.8 and later.

The following tests fail on linux with python 3.7

SCons/CacheDirTests.py
SCons/EnvironmentTests.py
test/CacheDir/CACHEDIR_CLASS.py
test/CacheDir/CacheDir.py
test/CacheDir/CacheDir_TryCompile.py
test/CacheDir/CustomCacheDir.py
test/CacheDir/DoubleCachedirClass.py
test/CacheDir/NoCache.py
test/CacheDir/SideEffect.py
test/CacheDir/VariantDir.py
test/CacheDir/debug.py
test/CacheDir/environment.py
test/CacheDir/multi-targets.py
test/CacheDir/multiple-targets.py
test/CacheDir/option--cd.py
test/CacheDir/option--cf.py
test/CacheDir/option--cr.py
test/CacheDir/option--cs.py
test/CacheDir/readonly-cache.py
test/CacheDir/scanner-target.py
test/CacheDir/source-scanner.py
test/CacheDir/symlink.py
test/CacheDir/timestamp-match.py
test/CacheDir/timestamp-newer.py
test/CacheDir/up-to-date-q.py
test/CacheDir/value_dependencies.py
test/Interactive/cache-debug.py
test/Interactive/cache-disable.py
test/Interactive/cache-force.py
test/Interactive/cache-show.py

The same tests fail on Windows with python 3.7 with the exception that there is no result for test test/CacheDir/symlink.py.

The root cause of the failures appears to be the CacheDir method _mkdir_atomic. Python 3.7 evidently behaves differently than later versions of Python.

scons/SCons/CacheDir.py

Lines 193 to 227 in cd43bf7

def _mkdir_atomic(self, path: str) -> bool:
"""Create cache directory at *path*.
Uses directory renaming to avoid races. If we are actually
creating the dir, populate it with the metadata files at the
same time as that's the safest way. But it's not illegal to point
CacheDir at an existing directory that wasn't a cache previously,
so we may have to do that elsewhere, too.
Returns:
``True`` if it we created the dir, ``False`` if already existed,
Raises:
SConsEnvironmentError: if we tried and failed to create the cache.
"""
directory = os.path.abspath(path)
if os.path.exists(directory):
return False
try:
tempdir = tempfile.TemporaryDirectory(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

Sample test output:

  • 1/30 (3.33%) /home/jbrill/.venv/3.7.17/scons-dev/bin/python SCons/CacheDirTests.py
    .E....E.E.
    ======================================================================
    ERROR: test_cachepath (__main__.CacheDirTestCase)
    Test the cachepath() method
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "SCons/CacheDirTests.py", line 71, in setUp
        self._CacheDir = SCons.CacheDir.CacheDir('cache')
      File "/mnt/s/SCons/scons-4.9.0-jcb/SCons/CacheDir.py", line 158, in __init__
        self._readconfig(path)
      File "/mnt/s/SCons/scons-4.9.0-jcb/SCons/CacheDir.py", line 235, in _readconfig
        created = self._mkdir_atomic(path)
      File "/mnt/s/SCons/scons-4.9.0-jcb/SCons/CacheDir.py", line 227, in _mkdir_atomic
        raise SCons.Errors.SConsEnvironmentError(msg) from e
      File "/home/jbrill/.pyenv/versions/3.7.17/lib/python3.7/tempfile.py", line 807, in __exit__
        self.cleanup()
      File "/home/jbrill/.pyenv/versions/3.7.17/lib/python3.7/tempfile.py", line 811, in cleanup
        _shutil.rmtree(self.name)
      File "/home/jbrill/.pyenv/versions/3.7.17/lib/python3.7/shutil.py", line 485, in rmtree
        onerror(os.lstat, path, sys.exc_info())
      File "/home/jbrill/.pyenv/versions/3.7.17/lib/python3.7/shutil.py", line 483, in rmtree
        orig_st = os.lstat(path)
    FileNotFoundError: [Errno 2] No such file or directory: '/mnt/s/SCons/scons-4.9.0-jcb/tmpo5oj9yn_'
    

A test file for the failed tests when using the file option to runtest.py (e.g., -f cachedir.txt) is cachedir.txt

Required information

  • Windows/VM:
    • link to mailing list: n/a
    • SCons 4.9.0
    • Python 3.7.7
    • pyenv-win
    • scons source distribution (scons-4.9.0)
    • Windows 11 Pro (23H2)
    • python runtest.py --all --time --passed
      • 29 tests failed
  • Linux/WSL:
    • link to mailing list: n/a
    • SCons 4.9.0
    • Python 3.7.17
    • pyenv
    • scons source distribution (scons-4.9.0)
    • Ubuntu 24.04.2 LTS (noble)
    • python runtest.py --all --time --passed
      • 30 tests failed
@mwichmann
Copy link
Collaborator

Confirmed. It's not limited to Windows, so something has crept in that is too new. Our test matrix is constrained, to avoid eating too many "free" resources, but should always include the oldest Python supported, but at the moment 3.8 is the oldest tested.

@mwichmann
Copy link
Collaborator

CacheDir initialization is the problem. An odd one... it seems when the temporary directory object created when calling tempfile.TemporaryDirectory is used as a context manager, and the code block renames the directory (as it does), then the context manager fails on exit because the original directory no longer exists. For all the versions from 3.8 on, it doesn't fail in this case. I don't see any documentation that this behavior has changed between 3.7 and 3.8. Since 3.10, you can set an ignore_cleanup_errors when creating the TemporaryDirectory, but that defaults to false, we don't set it, and if that was the reason, 3.8 and 3.9 would also be failing.

Anyway, a workaround isn't too complicated.

@bdbaddog
Copy link
Contributor

@mwichmann - if you PR it, we can roll 4.9.1 and release that fairly quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants