Create utils.delete_file and utils.delete_dir in place of tempfiles.try_delete. NFC

The code in `tempfiles.try_delete` went to great lengths to never fail
and change the permission bits of the things it was trying to delete.

However, in most cases is undesirable and overkill.  We don't want to
silently fail if the delete fails, and we don't want to delete read only
files in most cases.

The exception is the test suite were we do sometimes create read only
and we want to be able to clean them up.  So I created a test-only
helper called `force_delete_dir`, but even then we don't want to
silently fail so the try/catch was removed here.

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')
 
@@ -3044,7 +3044,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')
@@ -3532,7 +3532,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')
 
@@ -3570,7 +3570,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)
 
 
@@ -3768,7 +3768,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,35 @@ 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)
+  utils.delete_dir(dirname)
+
+
 def parameterized(parameters):
   """
   Mark a test as parameterized.
@@ -509,7 +531,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)
@@ -527,7 +549,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 = []
@@ -949,9 +971,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
@@ -1717,7 +1739,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('\\', '/'))
@@ -2560,8 +2560,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'])
@@ -4040,7 +4040,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.
@@ -5295,7 +5295,7 @@ def test_wasmfs_fetch_backend(self, args):
       return self.skipTest('ff hangs on the main_thread version. browser bug?')
     create_file('data.dat', 'hello, fetch')
     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
@@ -4064,7 +4064,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:
@@ -6380,7 +6380,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)