diff --git a/CHANGES b/CHANGES index de1562fe..76d5a78c 100644 --- a/CHANGES +++ b/CHANGES @@ -20,7 +20,8 @@ Changes - Make it possible to use a custom encoder when using config_changed with a dict. - Add configuration `DOIT_CONFIG` `action_string_formatting` to control command action formatter. - Fix `result_dep`, use result **after** its execution - - Fix #286: Cannot use functools.partial as title + - Fix #286: Support `functools.partial` on tasks' dict metadata `task.title` + - Fix #285: `clean` command, remove targets in reverse DFS order 0.31.1 (*2018-03-18*) diff --git a/doc/cmd_other.rst b/doc/cmd_other.rst index 14ef13cf..f06a3442 100644 --- a/doc/cmd_other.rst +++ b/doc/cmd_other.rst @@ -121,6 +121,12 @@ include a *clean* attribute. This attribute can be ``True`` to remove all of its target files. If there is a folder as a target it will be removed if the folder is empty, otherwise it will display a warning message. +.. note:: + + The targets' removal order will be the reverse of their lexical ordering. + This ensures that the directory structure formed by the targets is correctly + removed irrespective of their order in the ``targets`` array. + The *clean* attribute can be a list of actions. An action could be a string with a shell command or a tuple with a python callable. diff --git a/doit/task.py b/doit/task.py index 4859e33b..f2fbe029 100644 --- a/doit/task.py +++ b/doit/task.py @@ -573,26 +573,20 @@ def dict_to_task(task_dict): def clean_targets(task, dryrun): """remove all targets from a task""" - files = [path for path in task.targets if os.path.isfile(path)] - dirs = [path for path in task.targets if os.path.isdir(path)] - - # remove all files - for file_ in files: - print("%s - removing file '%s'" % (task.name, file_)) - if not dryrun: - os.remove(file_) - - # remove all directories (if empty) - for dir_ in dirs: - if os.listdir(dir_): - msg = "%s - cannot remove (it is not empty) '%s'" - print(msg % (task.name, dir_)) - else: - msg = "%s - removing dir '%s'" - print(msg % (task.name, dir_)) + for target in sorted(task.targets, reverse=True): + if os.path.isfile(target): + print("%s - removing file '%s'" % (task.name, target)) if not dryrun: - os.rmdir(dir_) - + os.remove(target) + elif os.path.isdir(target): + if os.listdir(target): + msg = "%s - cannot remove (it is not empty) '%s'" + print(msg % (task.name, target)) + else: + msg = "%s - removing dir '%s'" + print(msg % (task.name, target)) + if not dryrun: + os.rmdir(target) # uptodate diff --git a/tests/test_task.py b/tests/test_task.py index 8d63679d..b59d1e2e 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -374,8 +374,11 @@ class TestTaskClean(object): def tmpdir(self, request): tmpdir = {} tmpdir['dir'] = tempfile.mkdtemp(prefix='doit-') + tmpdir['subdir'] = tempfile.mkdtemp(dir=tmpdir['dir']) files = [os.path.join(tmpdir['dir'], fname) - for fname in ['a.txt', 'b.txt']] + for fname in ['a.txt', + 'b.txt', + os.path.join(tmpdir['subdir'], 'c.txt')]] tmpdir['files'] = files # create empty files for filename in tmpdir['files']: @@ -412,14 +415,15 @@ def test_clean_non_existent_targets(self): def test_clean_empty_dirs(self, tmpdir): # Remove empty directories listed in targets - targets = tmpdir['files'] + [tmpdir['dir']] + targets = tmpdir['files'] + [tmpdir['subdir']] t = task.Task("xxx", None, targets=targets, clean=True) assert True == t._remove_targets assert 0 == len(t.clean_actions) t.clean(StringIO(), False) for filename in tmpdir['files']: assert not os.path.exists(filename) - assert not os.path.exists(tmpdir['dir']) + assert not os.path.exists(tmpdir['subdir']) + assert os.path.exists(tmpdir['dir']) def test_keep_non_empty_dirs(self, tmpdir): # Keep non empty directories listed in targets @@ -433,6 +437,19 @@ def test_keep_non_empty_dirs(self, tmpdir): assert expected == os.path.exists(filename) assert os.path.exists(tmpdir['dir']) + def test_clean_in_reverse_lexical_order(self, tmpdir): + # Remove targets in reverse lexical order so that subdirectories' order + # in the targets array is irrelevant + targets = tmpdir['files'] + [tmpdir['dir'], tmpdir['subdir']] + t = task.Task("xxx", None, targets=targets, clean=True) + assert True == t._remove_targets + assert 0 == len(t.clean_actions) + t.clean(StringIO(), False) + for filename in tmpdir['files']: + assert not os.path.exists(filename) + assert not os.path.exists(tmpdir['dir']) + assert not os.path.exists(tmpdir['subdir']) + def test_clean_actions(self, tmpdir): # a clean action can be anything, it can even not clean anything! c_path = tmpdir['files'][0]