From a6fc7d1db802e09da6b4b09a27d976526ab93b43 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Fri, 18 Feb 2022 11:47:40 -0800 Subject: [PATCH] Improve SuperPMI script robustness 1. Be more robust to temp directory removal failure If we fail to remove a temporary directory, don't crash. Log the failure and the set of directories and files still remaining, and continue. We have seen this failure in at least one Linux x64 PMI coreclr_tests SuperPMI collection: ``` [Errno 39] Directory not empty: '/datadisks/disk1/work/B18E0979/t/tmpov3b4_qa' ``` 2. Limit the length of file names created by `make_safe_filename`. We were creating file names out of full path names, where those paths contained long temporary directory paths, and thus we were exceeding the maximum allowed file name component. --- src/coreclr/scripts/jitutil.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/coreclr/scripts/jitutil.py b/src/coreclr/scripts/jitutil.py index e6dcd648a5119c..3cc4f1610c7207 100644 --- a/src/coreclr/scripts/jitutil.py +++ b/src/coreclr/scripts/jitutil.py @@ -50,7 +50,20 @@ def __enter__(self): def __exit__(self, exc_type, exc_val, exc_tb): os.chdir(self.cwd) if not self._skip_cleanup: - shutil.rmtree(self.mydir) + try: + shutil.rmtree(self.mydir) + except Exception as ex: + logging.warning("Warning: failed to remove directory \"%s\": %s", self.mydir, ex) + # Print out all the remaining files and directories, in case that provides useful information + # for diagnosing the failure. If there is an exception doing this, ignore it. + try: + for dirpath, dirnames, filenames in os.walk(self.mydir): + for dir_name in dirnames: + logging.warning(" Remaining directory: \"%s\"", os.path.join(dirpath, dir_name)) + for file_name in filenames: + logging.warning(" Remaining file: \"%s\"", os.path.join(dirpath, file_name)) + except Exception: + pass class ChangeDir: """ Class to temporarily change to a given directory. Use with "with". @@ -255,6 +268,8 @@ def is_nonzero_length_file(fpath): def make_safe_filename(s): """ Turn a string into a string usable as a single file name component; replace illegal characters with underscores. + Also, limit the length of the file name to avoid creating illegally long file names. This is done by taking a + suffix of the name no longer than the maximum allowed file name length. Args: s (str) : string to convert to a file name @@ -267,7 +282,12 @@ def safe_char(c): return c else: return "_" - return "".join(safe_char(c) for c in s) + # Typically, a max filename length is 256, but let's limit it far below that, because callers typically add additional + # strings to this. + max_allowed_file_name_length = 150 + s = "".join(safe_char(c) for c in s) + s = s[-max_allowed_file_name_length:] + return s def find_in_path(name, pathlist, match_func=os.path.isfile):