Skip to content

Commit

Permalink
Improve SuperPMI script robustness (#65586)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
BruceForstall authored Feb 18, 2022
1 parent ac4ec9e commit cf6dbde
Showing 1 changed file with 22 additions and 2 deletions.
24 changes: 22 additions & 2 deletions src/coreclr/scripts/jitutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down

0 comments on commit cf6dbde

Please sign in to comment.