diff --git a/fsspec/implementations/local.py b/fsspec/implementations/local.py index ee2995d63..70dfd901e 100644 --- a/fsspec/implementations/local.py +++ b/fsspec/implementations/local.py @@ -6,6 +6,7 @@ import shutil import stat import tempfile +from functools import lru_cache from fsspec import AbstractFileSystem from fsspec.compression import compr @@ -354,6 +355,19 @@ def trailing_sep(path): return path.endswith(os.sep) or (os.altsep is not None and path.endswith(os.altsep)) +@lru_cache(maxsize=1) +def get_umask(mask: int = 0o666) -> int: + """Get the current umask. + + Follows https://stackoverflow.com/a/44130549 to get the umask. + Temporarily sets the umask to the given value, and then resets it to the + original value. + """ + value = os.umask(mask) + os.umask(value) + return value + + class LocalFileOpener(io.IOBase): def __init__( self, path, mode, autocommit=True, fs=None, compression=None, **kwargs @@ -417,6 +431,11 @@ def commit(self): if self.autocommit: raise RuntimeError("Can only commit if not already set to autocommit") shutil.move(self.temp, self.path) + try: + mask = 0o666 + os.chmod(self.path, mask & ~get_umask(mask)) + except RuntimeError: + pass def discard(self): if self.autocommit: diff --git a/fsspec/implementations/tests/test_local.py b/fsspec/implementations/tests/test_local.py index 2820c14f5..2e4c07c9d 100644 --- a/fsspec/implementations/tests/test_local.py +++ b/fsspec/implementations/tests/test_local.py @@ -15,7 +15,7 @@ import fsspec from fsspec import compression from fsspec.core import OpenFile, get_fs_token_paths, open_files -from fsspec.implementations.local import LocalFileSystem, make_path_posix +from fsspec.implementations.local import LocalFileSystem, get_umask, make_path_posix from fsspec.tests.test_utils import WIN files = { @@ -508,6 +508,60 @@ def test_commit_discard(tmpdir): assert not fs.exists(tmpdir + "/bfile") +def test_same_permissions_with_and_without_transaction(tmpdir): + tmpdir = str(tmpdir) + + with fsspec.open(tmpdir + "/afile", "wb") as f: + f.write(b"data") + + fs, urlpath = fsspec.core.url_to_fs(tmpdir + "/bfile") + with fs.transaction, fs.open(urlpath, "wb") as f: + f.write(b"data") + + assert fs.info(tmpdir + "/afile")["mode"] == fs.info(tmpdir + "/bfile")["mode"] + + +def test_get_umask_uses_cache(): + get_umask.cache_clear() # Clear the cache before the test for testing purposes. + get_umask() # Retrieve the current umask value. + get_umask() # Retrieve the umask again; this should result in a cache hit. + assert get_umask.cache_info().hits == 1 + + +def test_multiple_file_transaction_use_umask_cache(tmpdir): + get_umask.cache_clear() # Clear the cache before the test for testing purposes. + fs = LocalFileSystem() + with fs.transaction: + with fs.open(tmpdir + "/afile", "wb") as f: + f.write(b"data") + with fs.open(tmpdir + "/bfile", "wb") as f: + f.write(b"data") + assert get_umask.cache_info().hits == 1 + + +def test_multiple_transactions_use_umask_cache(tmpdir): + get_umask.cache_clear() # Clear the cache before the test for testing purposes. + fs = LocalFileSystem() + with fs.transaction: + with fs.open(tmpdir + "/afile", "wb") as f: + f.write(b"data") + with fs.transaction: + with fs.open(tmpdir + "/bfile", "wb") as f: + f.write(b"data") + assert get_umask.cache_info().hits == 1 + + +def test_multiple_filesystems_use_umask_cache(tmpdir): + get_umask.cache_clear() # Clear the cache before the test for testing purposes. + fs1 = LocalFileSystem() + fs2 = LocalFileSystem() + with fs1.transaction, fs1.open(tmpdir + "/afile", "wb") as f: + f.write(b"data") + with fs2.transaction, fs2.open(tmpdir + "/bfile", "wb") as f: + f.write(b"data") + assert get_umask.cache_info().hits == 1 + + def test_make_path_posix(): cwd = os.getcwd() if WIN: