Reland "Create utils.delete_file and utils.delete_dir in place of tempfiles.try_delete. NFC (#17512)" (#17775)

Author: sbc100

emcc.py

@@ -51,7 +51,7 @@
 from tools import webassembly
 from tools import config
 from tools.settings import settings, MEM_SIZE_SETTINGS, COMPILE_TIME_SETTINGS
-from tools.utils import read_file, write_file, read_binary
+from tools.utils import read_file, write_file, read_binary, delete_file
 
 logger = logging.getLogger('emcc')
 
@@ -3070,7 +3070,7 @@ def phase_final_emitting(options, state, target, wasm_target, memfile):
     generate_worker_js(target, js_target, target_basename)
 
   if embed_memfile() and memfile:
-    shared.try_delete(memfile)
+    delete_file(memfile)
 
   if settings.SPLIT_MODULE:
     diagnostics.warning('experimental', 'The SPLIT_MODULE setting is experimental and subject to change')
@@ -3561,7 +3561,7 @@ def preprocess_wasm2js_script():
     if settings.WASM != 2:
       final_js = wasm2js
       # if we only target JS, we don't need the wasm any more
-      shared.try_delete(wasm_target)
+      delete_file(wasm_target)
 
     save_intermediate('wasm2js')
 
@@ -3599,7 +3599,7 @@ def preprocess_wasm2js_script():
       js = do_replace(js, '<<< WASM_BINARY_DATA >>>', base64_encode(read_binary(wasm_target)))
     else:
       js = do_replace(js, '<<< WASM_BINARY_FILE >>>', get_subresource_location(wasm_target))
-    shared.try_delete(wasm_target)
+    delete_file(wasm_target)
     write_file(final_js, js)
 
 
@@ -3797,7 +3797,7 @@ def generate_traditional_runtime_html(target, options, js_target, target_basenam
     js_contents = script.inline or ''
     if script.src:
       js_contents += read_file(js_target)
-    shared.try_delete(js_target)
+    delete_file(js_target)
     script.src = None
     script.inline = js_contents
 

test/common.py

@@ -31,9 +31,9 @@
 import jsrun
 from tools.shared import TEMP_DIR, EMCC, EMXX, DEBUG, EMCONFIGURE, EMCMAKE
 from tools.shared import EMSCRIPTEN_TEMP_DIR
-from tools.shared import get_canonical_temp_dir, try_delete, path_from_root
+from tools.shared import get_canonical_temp_dir, path_from_root
 from tools.utils import MACOS, WINDOWS, read_file, read_binary, write_file, write_binary, exit_with_error
-from tools import shared, line_endings, building, config
+from tools import shared, line_endings, building, config, utils
 
 logger = logging.getLogger('common')
 
@@ -84,13 +84,6 @@
 PYTHON = sys.executable
 
 
-def delete_contents(pathname):
-  for entry in os.listdir(pathname):
-    try_delete(os.path.join(pathname, entry))
-    # TODO(sbc): Should we make try_delete have a stronger guarantee?
-    assert not os.path.exists(os.path.join(pathname, entry))
-
-
 def test_file(*path_components):
   """Construct a path relative to the emscripten "tests" directory."""
   return str(Path(TEST_ROOT, *path_components))
@@ -308,6 +301,44 @@ def make_executable(name):
   Path(name).chmod(stat.S_IREAD | stat.S_IWRITE | stat.S_IEXEC)
 
 
+def make_dir_writeable(dirname):
+  # Ensure all files are readable and writable by the current user.
+  permission_bits = stat.S_IWRITE | stat.S_IREAD
+
+  def is_writable(path):
+    return (os.stat(path).st_mode & permission_bits) != permission_bits
+
+  def make_writable(path):
+    new_mode = os.stat(path).st_mode | permission_bits
+    os.chmod(path, new_mode)
+
+  # Some tests make files and subdirectories read-only, so rmtree/unlink will not delete
+  # them. Force-make everything writable in the subdirectory to make it
+  # removable and re-attempt.
+  if not is_writable(dirname):
+    make_writable(dirname)
+
+  for directory, subdirs, files in os.walk(dirname):
+    for item in files + subdirs:
+      i = os.path.join(directory, item)
+      if not os.path.islink(i):
+        make_writable(i)
+
+
+def force_delete_dir(dirname):
+  make_dir_writeable(dirname)
+  if WINDOWS:
+    # We have seen issues where windows was unable to cleanup the test directory
+    # See: https://github.com/emscripten-core/emscripten/pull/17512
+    # TODO: Remove this try/catch.
+    try:
+      utils.delete_dir(dirname)
+    except IOError as e:
+      logger.warning(f'failed to remove directory: {dirname} ({e})')
+  else:
+    utils.delete_dir(dirname)
+
+
 def parameterized(parameters):
   """
   Mark a test as parameterized.
@@ -505,7 +536,7 @@ def setUp(self):
           # expect this.  --no-clean can be used to keep the old contents for the new test
           # run. This can be useful when iterating on a given test with extra files you want to keep
           # around in the output directory.
-          delete_contents(self.working_dir)
+          utils.delete_contents(self.working_dir)
       else:
         print('Creating new test output directory')
         ensure_dir(self.working_dir)
@@ -523,7 +554,7 @@ def tearDown(self):
     if not EMTEST_SAVE_DIR:
       # rmtree() fails on Windows if the current working directory is inside the tree.
       os.chdir(os.path.dirname(self.get_dir()))
-      try_delete(self.get_dir())
+      force_delete_dir(self.get_dir())
 
       if EMTEST_DETECT_TEMPFILE_LEAKS and not DEBUG:
         temp_files_after_run = []
@@ -981,9 +1012,9 @@ def get_library(self, name, generated_libs, configure=['sh', './configure'],  #
                          cache_name, env_init=env_init, native=native)
 
   def clear(self):
-    delete_contents(self.get_dir())
+    utils.delete_contents(self.get_dir())
     if EMSCRIPTEN_TEMP_DIR:
-      delete_contents(EMSCRIPTEN_TEMP_DIR)
+      utils.delete_contents(EMSCRIPTEN_TEMP_DIR)
 
   def run_process(self, cmd, check=True, **args):
     # Wrapper around shared.run_process.  This is desirable so that the tests
@@ -1753,7 +1784,7 @@ def btest(self, filename, expected=None, reference=None,
     outfile = 'test.html'
     args += [filename, '-o', outfile]
     # print('all args:', args)
-    try_delete(outfile)
+    utils.delete_file(outfile)
     self.compile_btest(args, reporting=reporting)
     self.assertExists(outfile)
     if post_build:

test/parallel_testsuite.py

@@ -10,7 +10,8 @@
 import time
 import queue
 
-from tools.tempfiles import try_delete
+import common
+
 
 NUM_CORES = None
 
@@ -95,7 +96,7 @@ def collect_results(self):
       else:
         self.clear_finished_processes()
     for temp_dir in self.dedicated_temp_dirs:
-      try_delete(temp_dir)
+      common.force_delete_dir(temp_dir)
     return buffered_results
 
   def clear_finished_processes(self):

test/test_benchmark.py

@@ -21,8 +21,8 @@
 import common
 from tools.shared import CLANG_CC, CLANG_CXX
 from common import TEST_ROOT, test_file, read_file, read_binary
-from tools.shared import run_process, PIPE, try_delete, EMCC, config
-from tools import building
+from tools.shared import run_process, PIPE, EMCC, config
+from tools import building, utils
 
 # standard arguments for timing:
 # 0: no runtime, just startup
@@ -197,7 +197,7 @@ def build(self, parent, filename, args, shared_args, emcc_args, native_args, nat
       emcc_args += lib_builder('js_' + llvm_root, native=False, env_init=env_init)
     final = os.path.dirname(filename) + os.path.sep + self.name + ('_' if self.name else '') + os.path.basename(filename) + '.js'
     final = final.replace('.cpp', '')
-    try_delete(final)
+    utils.delete_file(final)
     cmd = [
       EMCC, filename,
       OPTIMIZATIONS,
@@ -317,7 +317,7 @@ def build(self, parent, filename, args, shared_args, emcc_args, native_args, nat
       cheerp_args += ['-cheerp-pretty-code'] # get function names, like emcc --profiling
     final = os.path.dirname(filename) + os.path.sep + self.name + ('_' if self.name else '') + os.path.basename(filename) + '.js'
     final = final.replace('.cpp', '')
-    try_delete(final)
+    utils.delete_file(final)
     dirs_to_delete = []
     cheerp_args += ['-cheerp-preexecute']
     try:
@@ -339,7 +339,7 @@ def build(self, parent, filename, args, shared_args, emcc_args, native_args, nat
         run_binaryen_opts(final.replace('.js', '.wasm'), self.binaryen_opts)
     finally:
       for dir_ in dirs_to_delete:
-        try_delete(dir_)
+        utils.delete_dir(dir_)
 
   def run(self, args):
     return jsrun.run_js(self.filename, engine=self.engine, args=args, stderr=PIPE)

test/test_browser.py

@@ -26,7 +26,7 @@
 from tools import shared
 from tools import ports
 from tools.shared import EMCC, WINDOWS, FILE_PACKAGER, PIPE
-from tools.shared import try_delete
+from tools.utils import delete_file, delete_dir
 
 
 def test_chunked_synchronous_xhr_server(support_byte_ranges, chunkSize, data, checksum, port):
@@ -245,8 +245,8 @@ def test_zzz_html_source_map(self):
       ''')
     # use relative paths when calling emcc, because file:// URIs can only load
     # sourceContent when the maps are relative paths
-    try_delete(html_file)
-    try_delete(html_file + '.map')
+    delete_file(html_file)
+    delete_file(html_file + '.map')
     self.compile_btest(['src.cpp', '-o', 'src.html', '-gsource-map'])
     self.assertExists(html_file)
     self.assertExists('src.wasm.map')
@@ -339,7 +339,7 @@ def make_main(path):
     self.btest_exit('main.cpp', args=['--preload-file', absolute_src_path])
 
     # Test subdirectory handling with asset packaging.
-    try_delete('assets')
+    delete_dir('assets')
     ensure_dir('assets/sub/asset1/'.replace('\\', '/'))
     ensure_dir('assets/sub/asset1/.git'.replace('\\', '/')) # Test adding directory that shouldn't exist.
     ensure_dir('assets/sub/asset2/'.replace('\\', '/'))
@@ -2559,8 +2559,8 @@ def test_uuid(self):
     print(out)
 
     # Tidy up files that might have been created by this test.
-    try_delete(test_file('uuid/test.js'))
-    try_delete(test_file('uuid/test.js.map'))
+    delete_file(test_file('uuid/test.js'))
+    delete_file(test_file('uuid/test.js.map'))
 
     # Now run test in browser
     self.btest_exit(test_file('uuid/test.c'), args=['-luuid'])
@@ -4039,7 +4039,7 @@ def test_pthread_custom_pthread_main_url(self):
     # Test that it is possible to define "Module.locateFile(foo)" function to locate where worker.js will be loaded from.
     create_file('shell2.html', read_file(path_from_root('src/shell.html')).replace('var Module = {', 'var Module = { locateFile: function(filename) { if (filename == "test.worker.js") return "cdn/test.worker.js"; else return filename; }, '))
     self.compile_btest(['main.cpp', '--shell-file', 'shell2.html', '-sWASM=0', '-sIN_TEST_HARNESS', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE', '-o', 'test2.html'], reporting=Reporting.JS_ONLY)
-    try_delete('test.worker.js')
+    delete_file('test.worker.js')
     self.run_browser('test2.html', '/report_result?exit:0')
 
   # Test that if the main thread is performing a futex wait while a pthread needs it to do a proxied operation (before that pthread would wake up the main thread), that it's not a deadlock.
@@ -5305,7 +5305,7 @@ def test_wasmfs_fetch_backend(self, args):
     create_file('data.dat', 'hello, fetch')
     create_file('small.dat', 'hello')
     create_file('test.txt', 'fetch 2')
-    try_delete('subdir')
+    delete_dir('subdir')
     ensure_dir('subdir')
     create_file('subdir/backendfile', 'file 1')
     create_file('subdir/backendfile2', 'file 2')

test/test_core.py

@@ -19,9 +19,9 @@
 if __name__ == '__main__':
   raise Exception('do not run this file directly; do something like: test/runner')
 
-from tools.shared import try_delete, PIPE
+from tools.shared import PIPE
 from tools.shared import EMCC, EMAR
-from tools.utils import WINDOWS, MACOS, write_file
+from tools.utils import WINDOWS, MACOS, write_file, delete_file
 from tools import shared, building, config, webassembly
 import common
 from common import RunnerCore, path_from_root, requires_native_clang, test_file, create_file
@@ -4061,7 +4061,7 @@ def dylink_testf(self, main, side=None, expected=None, force_c=False, main_emcc_
 
     if isinstance(main, list):
       # main is just a library
-      try_delete('main.js')
+      delete_file('main.js')
       self.run_process([EMCC] + main + self.get_emcc_args() + ['-o', 'main.js'])
       self.do_run('main.js', expected, no_build=True, **kwargs)
     else:
@@ -6402,7 +6402,7 @@ def test_dlmalloc(self):
     if self.emcc_args == []:
       # emcc should build in dlmalloc automatically, and do all the sign correction etc. for it
 
-      try_delete('src.js')
+      delete_file('src.js')
       self.run_process([EMCC, test_file('dlmalloc_test.c'), '-sINITIAL_MEMORY=128MB', '-o', 'src.js'], stdout=PIPE, stderr=self.stderr_redirect)
 
       self.do_run(None, '*1,0*', ['200', '1'], no_build=True)