Re: [PATCH] dtrace: Use deterministic temp file creation for all temp files

[Gioele Barabucci <gioele@svario.it>]

On 27/02/23 18:34, Florian Weimer wrote:
> * Gioele Barabucci:
> 
>> @@ -380,6 +373,7 @@ def main():
>>           usage()
>>           return 1
>>   
>> +    TEMP_DIR = mkdtemp()
>>       if s_filename != "" and use_cpp:
>>           (ignore, fname) = mktemp_determ(["use_cpp", s_filename], suffix=".d")
>>           cpp = os.environ.get("CPP", "cpp")
>> @@ -458,6 +452,9 @@ def main():
>>           else:
>>               print("cpp: " + s_filename)
>>   
>> +    if not keep_temps:
>> +        os.rmdir(TEMP_DIR)
> 
> This should probably use try:/finally:

You mean wrapping the whole function in a try:/finally: or just the 
`os.rmdir`?

The former implicates a quite invasive refactoring of the code 
(patch-wise a complete rewrite); the latter seems overkill given that 
the other `os.remove` calls are not checked and the only possibilities 
are either silencing the error (pass) or exit with an error code 
(similar to what the exception would do anyway).

> or a context manager with tempfile.TemporaryDirectory.

Wouldn't that greatly complicate the handling of `-k`/`keep_temps`? a 
context manager with `TemporaryDirectory` will unconditionally remove 
the directory and all its content on completion.

BTW, TemporaryDirectory is a Python 3 feature. I understood from the 
code that compatibility with 2.6 is still important in dtrace 
(`sys.version_info > (3,0)`...). Isn't that the case?

Regards,

-- 
Gioele Barabucci