From 2b934470b47bc3c66f0fc688e4fe7b1c16813e9d Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Wed, 13 Nov 2019 14:12:01 -0800 Subject: [PATCH 1/6] makedirs: fix mode flag is being ignored starting from Python 3.7 --- dvc/utils/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 864c9e5f56..350a6c3bc9 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -152,9 +152,9 @@ def makedirs(path, exist_ok=False, mode=None): _makedirs(path, exist_ok=exist_ok) return - umask = os.umask(0) + umask = os.umask(0o777 - mode) try: - _makedirs(path, exist_ok=exist_ok, mode=mode) + _makedirs(path, exist_ok=exist_ok) finally: os.umask(umask) From eec7618f49c9180338af02362b0b44ea7044dd4c Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Wed, 13 Nov 2019 16:58:51 -0800 Subject: [PATCH 2/6] tests: makedirs permissions on intermediate dirs --- tests/func/test_utils.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/func/test_utils.py b/tests/func/test_utils.py index 73f489e252..3ea25eed5f 100644 --- a/tests/func/test_utils.py +++ b/tests/func/test_utils.py @@ -1,6 +1,9 @@ import filecmp import os import shutil +import stat + +import pytest from dvc import utils from tests.basic_env import TestDvc @@ -69,3 +72,16 @@ def test_boxify(self): ) assert expected == utils.boxify("message") + + +@pytest.mark.skipif(os.name == "nt", reason="Not supported for Windows.") +def test_makedirs_permissions(): + dir_mode = 0o755 + intermediate_dir = "testdir" + test_dir = os.path.join(intermediate_dir, "data") + utils.makedirs(test_dir, mode=dir_mode) + + assert stat.S_IMODE(os.stat(test_dir).st_mode) == dir_mode + assert stat.S_IMODE(os.stat(intermediate_dir).st_mode) == dir_mode + + shutil.rmtree(test_dir) From 303d52e4acdc33dea6e3e89ffd2213cbbc8a4505 Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Thu, 14 Nov 2019 14:32:52 -0800 Subject: [PATCH 3/6] tests: fix mkdirs permissions test to cleanup after failure --- tests/func/test_utils.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/func/test_utils.py b/tests/func/test_utils.py index 3ea25eed5f..c764a80e45 100644 --- a/tests/func/test_utils.py +++ b/tests/func/test_utils.py @@ -1,3 +1,4 @@ +# encoding: utf-8 import filecmp import os import shutil @@ -77,11 +78,14 @@ def test_boxify(self): @pytest.mark.skipif(os.name == "nt", reason="Not supported for Windows.") def test_makedirs_permissions(): dir_mode = 0o755 - intermediate_dir = "testdir" + intermediate_dir = "тестовая-директория" test_dir = os.path.join(intermediate_dir, "data") - utils.makedirs(test_dir, mode=dir_mode) - assert stat.S_IMODE(os.stat(test_dir).st_mode) == dir_mode - assert stat.S_IMODE(os.stat(intermediate_dir).st_mode) == dir_mode + assert not os.path.exists(intermediate_dir) - shutil.rmtree(test_dir) + try: + utils.makedirs(test_dir, mode=dir_mode) + assert stat.S_IMODE(os.stat(test_dir).st_mode) == dir_mode + assert stat.S_IMODE(os.stat(intermediate_dir).st_mode) == dir_mode + finally: + shutil.rmtree(intermediate_dir) From ecfbc3a6de136bf5881891c2f0a03265ba423c2d Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Thu, 14 Nov 2019 14:33:36 -0800 Subject: [PATCH 4/6] compat: fix py2 mkdirs implementation, bytes do not exist on py2 --- dvc/utils/compat.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/dvc/utils/compat.py b/dvc/utils/compat.py index 7d66dd090c..4863e4a565 100644 --- a/dvc/utils/compat.py +++ b/dvc/utils/compat.py @@ -85,8 +85,6 @@ def _makedirs(name, mode=0o777, exist_ok=False): if e.errno != errno.EEXIST: raise cdir = os.curdir - if isinstance(tail, bytes): - cdir = bytes(os.curdir, "ASCII") if tail == cdir: return try: From aa6a77285d7173c13c01ca12636a47dff919d1da Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Tue, 19 Nov 2019 00:17:27 -0800 Subject: [PATCH 5/6] makedirs: add comment on why do have to use umask --- dvc/utils/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 350a6c3bc9..a7e9ab5f42 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -152,6 +152,9 @@ def makedirs(path, exist_ok=False, mode=None): _makedirs(path, exist_ok=exist_ok) return + # utilize umask to set proper permissions since Python 3.7 the `mode` + # `makedirs` argument no longer affects the file permission bits of + # newly-created intermediate-level directories. umask = os.umask(0o777 - mode) try: _makedirs(path, exist_ok=exist_ok) From 1c752a9548b922f3207fdb4991130acb190a81f0 Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Tue, 19 Nov 2019 00:32:09 -0800 Subject: [PATCH 6/6] tests: makedirs permissions test simplify with tmpdir fixture --- tests/func/test_utils.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/func/test_utils.py b/tests/func/test_utils.py index c764a80e45..594fdc332d 100644 --- a/tests/func/test_utils.py +++ b/tests/func/test_utils.py @@ -76,16 +76,15 @@ def test_boxify(self): @pytest.mark.skipif(os.name == "nt", reason="Not supported for Windows.") -def test_makedirs_permissions(): +def test_makedirs_permissions(tmpdir): dir_mode = 0o755 + os.chdir(str(tmpdir)) intermediate_dir = "тестовая-директория" test_dir = os.path.join(intermediate_dir, "data") assert not os.path.exists(intermediate_dir) - try: - utils.makedirs(test_dir, mode=dir_mode) - assert stat.S_IMODE(os.stat(test_dir).st_mode) == dir_mode - assert stat.S_IMODE(os.stat(intermediate_dir).st_mode) == dir_mode - finally: - shutil.rmtree(intermediate_dir) + utils.makedirs(test_dir, mode=dir_mode) + + assert stat.S_IMODE(os.stat(test_dir).st_mode) == dir_mode + assert stat.S_IMODE(os.stat(intermediate_dir).st_mode) == dir_mode