Fixed a problem removing temp files

[PeteSteinfeld]

Summary

Before making this change, whenever I ran "check-flang", I'd get an
error message like:

llvm-lit: /mnt/c/GitHub/f18/c751/flang/build/bin/../../../llvm/utils/lit/lit/main.py:252: warning: Failed to delete temp directory '/tmp/lit_tmp_gOKUIh'

With this change, there's no such message in the output, and the temp
directory is successfully removed.

Note that my working environment is on Windows 10 running Windows
Subsystem for Linux using the Ubuntu app.

[yln]

Hi, thanks for tackling this!

I am not on Windows, so I can't reproduce this. Would you be able to debug this so we understand what the problem is? For example, printing the exception object or looking into the error handler of shutil.rmtree(... , onerror=..) might shed light on things [1].

The docs say that os.rmdir() raises when the directory isn't empty, so maybe it's not the best tool here.

Does the following work?

# Sleep and retry
time.sleep(2) # what would be a good value here?
shutil.rmtree(..., ignore_errors=True)

Thanks!

[PeteSteinfeld]

I am not on Windows, so I can't reproduce this. Would you be able to debug this so we understand what the problem is? For example, printing the exception object or looking into the error handler of shutil.rmtree(... , onerror=..) might shed light on things [1].

Here's what I've discovered.

Despite the FIXME comment, inserting a call to sleep() has no effect.

I printed out the exception object, and got the following:

'module' object has no attribute 'rmtree'

When I moved the call to shutil.rmtree() out of the try/exception block, I got a stack dump the message at the bottom:

NameError: global name 'shutil' is not defined

So the version of Python I'm using doesn't have shutil. When I look up the documentation, it looks like it's been around since at least version 2.3, and I'm running version 2.7.1. I also have version 3.6.7 on my machine, and when I upgraded to that version the problem went away.

I'm not sure what the best fix is here. The simplest thing for me would be to
remove the FIXME comment and add some text to the warning message recommending that the user upgrade their version of Python if they see the message.

I put in such code and tested it under both Python 2.7.1 and 3.6.7, and it works for me.

[yln]

In D79802#2033902, @PeteSteinfeld wrote:

'module' object has no attribute 'rmtree'

So on Windows in Python2, rmtree() is not available? This surprises me (it's not mentioned in the docs). It also means the FIXME drew the wrong conclusion.

When I moved the call to shutil.rmtree() out of the try/exception block, I got a stack dump the message at the bottom:
NameError: global name 'shutil' is not defined

I think this means that the import statement is missing. Try to also move the import shutil.

The simplest thing for me would be to remove the FIXME comment and add some text to the warning message recommending that the user upgrade their version of Python if they see the message.

I put in such code and tested it under both Python 2.7.1 and 3.6.7, and it works for me.

Cool, thanks!

llvm/utils/lit/lit/main.py

def execute_in_tmp_dir(run, lit_config):
     try:
         run.execute()
     finally:
         if tmp_dir:
             try:
                 import shutil
                 shutil.rmtree(tmp_dir)
             except:
-                # FIXME: Re-try after timeout on Windows.
+                try:
+                    os.rmdir(tmp_dir)
+                except:
                     lit_config.warning("Failed to delete temp directory '%s'" % tmp_dir)
 
 
 def print_histogram(tests):
     test_times = [(t.getFullName(), t.result.elapsed)
                   for t in tests if t.result.elapsed]
     if test_times:
         lit.util.printHistogram(test_times, title='Tests')