search

google / renameio

Package renameio provides a way to atomically create or replace a file or symbolic link.
Apache License 2.0
609 stars 27 forks source link

Use device test to check if tempdir can be used #14

Closed twpayne closed 5 years ago

twpayne commented 5 years ago

I'm not sure if this is correct, but if it is then it's a simplification and speed improvement.

Basically, instead of creating a test temporary file and trying to rename it to determine if the return value of os.TempDir() can be used, we directly compare the device numbers and use the returned value if the device numbers are the same.

The original code has some behaviors that are not reflected in this approach. For example, the original code would fallback to filepath.Dir(dest) if it encountered any errors. Errors that might be encountered include not having permission to write to either directory (os.TempDir() or filepath.Dir(dest)) or disk full situations. Whether it is correct to fallback to filepath.Dir(dest) in these situations is debatable.

stapelberg commented 5 years ago

I’m not a fan of this change — the current code is more portable and determines eligibility by performing exactly the operations we need. I think that’s more elegant than checking whether files reside on the same device.

Given that performance-sensitive users are encouraged to explicitly call TempDir once and re-use the result, I don’t think performance is important here.

twpayne commented 5 years ago

OK, fair enough.