From 65ac3090c500ce7a7654df993959ccec30c9eebf Mon Sep 17 00:00:00 2001 From: Till Hoffmann Date: Thu, 30 Jun 2022 13:54:46 -0400 Subject: [PATCH 1/2] Use `list` for `file_dep` (fixes #254). --- doit/control.py | 2 +- doit/dependency.py | 7 ++++--- doit/task.py | 6 +++--- tests/test_action.py | 6 +++--- tests/test_control.py | 12 ++++++------ tests/test_dependency.py | 2 +- tests/test_loader.py | 4 ++-- tests/test_runner.py | 4 ++-- tests/test_task.py | 8 ++++---- 9 files changed, 26 insertions(+), 25 deletions(-) diff --git a/doit/control.py b/doit/control.py index 3e35ac66..2cf7705d 100644 --- a/doit/control.py +++ b/doit/control.py @@ -483,7 +483,7 @@ def _add_task(self, node): # remove file_dep since generated tasks are not required # to really create the target (support multiple matches) if regex_group: - this_task.file_dep = {} + this_task.file_dep = [] if regex_group.target in self.targets: regex_group.found = True else: diff --git a/doit/dependency.py b/doit/dependency.py index cd1befb4..b45a7fc2 100644 --- a/doit/dependency.py +++ b/doit/dependency.py @@ -679,10 +679,11 @@ def get_status(self, task, tasks_dict, get_log=False): # check for modified file_dep previous = self._get(task.name, 'deps:') previous_set = set(previous) if previous else None - if previous_set and previous_set != task.file_dep: + set_file_dep = set(task.file_dep) + if previous_set and previous_set != set_file_dep: if get_log: - added_files = sorted(list(task.file_dep - previous_set)) - removed_files = sorted(list(previous_set - task.file_dep)) + added_files = sorted(list(set_file_dep - previous_set)) + removed_files = sorted(list(previous_set - set_file_dep)) result.set_reason('added_file_dep', added_files) result.set_reason('removed_file_dep', removed_files) result.status = 'run' diff --git a/doit/task.py b/doit/task.py index f864d1a7..3a18f8ec 100644 --- a/doit/task.py +++ b/doit/task.py @@ -266,7 +266,7 @@ def _init_deps(self, file_dep, task_dep, calc_dep): self.dep_changed = None # file_dep - self.file_dep = set() + self.file_dep = [] self._expand_file_dep(file_dep) # task_dep @@ -327,9 +327,9 @@ def _expand_file_dep(self, file_dep): """put input into file_dep""" for dep in file_dep: if isinstance(dep, str): - self.file_dep.add(dep) + self.file_dep.append(dep) elif isinstance(dep, PurePath): - self.file_dep.add(str(dep)) + self.file_dep.append(str(dep)) else: msg = ("%s. file_dep must be a str or Path from pathlib. " "Got '%r' (%s)") diff --git a/tests/test_action.py b/tests/test_action.py index f4df6159..6032b618 100644 --- a/tests/test_action.py +++ b/tests/test_action.py @@ -214,7 +214,7 @@ def test_task_meta_reference(self): assert my_action.execute() is None got = my_action.out.split('-') - assert task.file_dep == set(got[0].split()) + assert task.file_dep == got[0].split() assert task.dep_changed == got[1].split() assert targets == got[2].split() @@ -896,7 +896,7 @@ def py_callable(targets, dependencies, changed, task): targets.append('new_target') dependencies.append('new_dependency') changed.append('new_changed') - task.file_dep.add('dep2') + task.file_dep.append('dep2') my_task = Task('Fake', [py_callable], file_dep=['dependencies'], targets=['targets']) my_task.dep_changed = ['changed'] @@ -904,7 +904,7 @@ def py_callable(targets, dependencies, changed, task): my_action = my_task.actions[0] my_action.execute() - assert my_task.file_dep == set(['dependencies', 'dep2']) + assert my_task.file_dep == ['dependencies', 'dep2'] assert my_task.targets == ['targets'] assert my_task.dep_changed == ['changed'] diff --git a/tests/test_control.py b/tests/test_control.py index 175e65c5..48a84edb 100644 --- a/tests/test_control.py +++ b/tests/test_control.py @@ -127,7 +127,7 @@ def test_filter_delayed_regex_single(self): assert len(selected) == 1 assert selected[0] == '_regex_target_abc:taskY' sel_task = control.tasks['_regex_target_abc:taskY'] - assert sel_task.file_dep == {'abc'} + assert sel_task.file_dep == ['abc'] assert sel_task.loader.basename == 'taskY' assert sel_task.loader is t2.loader @@ -159,8 +159,8 @@ def test_filter_delayed_regex_multiple_match(self): assert len(selected) == 2 assert (sorted(selected) == ['_regex_target_abc:taskY', '_regex_target_abc:taskZ']) - assert control.tasks['_regex_target_abc:taskY'].file_dep == {'abc'} - assert control.tasks['_regex_target_abc:taskZ'].file_dep == {'abc'} + assert control.tasks['_regex_target_abc:taskY'].file_dep == ['abc'] + assert control.tasks['_regex_target_abc:taskZ'].file_dep == ['abc'] assert (control.tasks['_regex_target_abc:taskY'].loader.basename == t2.name) assert (control.tasks['_regex_target_abc:taskZ'].loader.basename == @@ -177,8 +177,8 @@ def test_filter_delayed_regex_auto(self): assert len(selected) == 2 assert (sorted(selected) == ['_regex_target_abc:taskY', '_regex_target_abc:taskZ']) - assert control.tasks['_regex_target_abc:taskY'].file_dep == {'abc'} - assert control.tasks['_regex_target_abc:taskZ'].file_dep == {'abc'} + assert control.tasks['_regex_target_abc:taskY'].file_dep == ['abc'] + assert control.tasks['_regex_target_abc:taskZ'].file_dep == ['abc'] assert (control.tasks['_regex_target_abc:taskY'].loader.basename == t2.name) assert (control.tasks['_regex_target_abc:taskZ'].loader.basename == @@ -586,7 +586,7 @@ def creator(): # file_dep is removed because foo might not be task # that creates this task (support for multi regex matches) - assert n6.file_dep == {} + assert n6.file_dep == [] def test_regex_group_already_created(self): diff --git a/tests/test_dependency.py b/tests/test_dependency.py index d9414ae0..59b3dc1f 100644 --- a/tests/test_dependency.py +++ b/tests/test_dependency.py @@ -243,7 +243,7 @@ def test_save_files(self, pdep_manager): pdep_manager.save_success(t1) assert pdep_manager._get("taskId_X", filePath) is not None assert pdep_manager._get("taskId_X", filePath2) is not None - assert set(pdep_manager._get("taskId_X", 'deps:')) == t1.file_dep + assert list(pdep_manager._get("taskId_X", 'deps:')) == t1.file_dep def test_save_values(self, pdep_manager): t1 = Task('t1', None) diff --git a/tests/test_loader.py b/tests/test_loader.py index 995f7fa1..f6273cf4 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -198,7 +198,7 @@ def creator(): original.create_doit_tasks = creator task_list = load_tasks({'x': original}) assert 1 == len(task_list) - assert set(['foox']) == task_list[0].file_dep + assert ['foox'] == task_list[0].file_dep def testUse_create_doit_tasks_class_method(self): class Foo(object): @@ -209,7 +209,7 @@ def _create_doit_tasks(self): task_list = load_tasks({'Foo':Foo, 'foo':Foo()}) assert len(task_list) == 1 - assert task_list[0].file_dep == set(['fooy']) + assert task_list[0].file_dep == ['fooy'] def testUse_create_doit_tasks_basename_kwargs(self): class Foo(object): diff --git a/tests/test_runner.py b/tests/test_runner.py index 1750b686..ad4fa3be 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -337,7 +337,7 @@ def use_args(arg1): def make_args(): return {'myarg':1} def action_add_filedep(task, extra_dep): - task.file_dep.add(extra_dep) + task.file_dep.append(extra_dep) class TestRunner_run_tasks(object): @@ -542,7 +542,7 @@ def testActionModifiesFiledep(self, reporter, RunnerClass, dep_manager): assert ('start', t1) == reporter.log.pop(0), reporter.log assert ('execute', t1) == reporter.log.pop(0) assert ('success', t1) == reporter.log.pop(0) - assert t1.file_dep == set([extra_dep]) + assert t1.file_dep == [extra_dep] # SystemExit runner should not interfere with SystemExit def testSystemExitRaises(self, reporter, RunnerClass, dep_manager): diff --git a/tests/test_task.py b/tests/test_task.py index b31f64e7..525ad1f5 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -210,13 +210,13 @@ def test_invalid(self): class TestTaskExpandFileDep(object): def test_dependencyStringIsFile(self): - my_task = task.Task("Task X", ["taskcmd"], file_dep=["123","456"]) - assert set(["123","456"]) == my_task.file_dep + my_task = task.Task("Task X", ["taskcmd"], file_dep=["123", "456"]) + assert ["123", "456"] == my_task.file_dep def test_file_dep_path(self): my_task = task.Task("Task X", ["taskcmd"], file_dep=["123", Path("456"), PurePath("789")]) - assert {"123", "456", "789"} == my_task.file_dep + assert ["123", "456", "789"] == my_task.file_dep def test_file_dep_str(self): pytest.raises(task.InvalidTask, task.Task, "Task X", ["taskcmd"], @@ -248,7 +248,7 @@ def test_update_deps(self): 'uptodate': [True], 'to_be_ignored': 'asdf', }) - assert set(['fileX', 'fileY']) == my_task.file_dep + assert ['fileX', 'fileY'] == my_task.file_dep assert ['taskY'] == my_task.task_dep assert set(['calcX', 'calcY']) == my_task.calc_dep assert [(None, None, None), (True, None, None)] == my_task.uptodate From 778f448ec8a421105376201addcb936ab9c9acae Mon Sep 17 00:00:00 2001 From: Till Hoffmann Date: Fri, 1 Jul 2022 11:51:14 -0400 Subject: [PATCH 2/2] Add changing `file_dep` to list to `CHANGES`. --- CHANGES | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 6d402147..f18d63f6 100644 --- a/CHANGES +++ b/CHANGES @@ -15,8 +15,7 @@ Changes - Ref #409: make cloudpickle optional, and not installed by default on PyPy. - fix: make sure `io.capture` respects out/err option. - - +- Fix #254: use `list` for `file_dep` instead of `set` to preserve the order of dependencies. 0.36.0 (*2022-04-22*) ===================== @@ -611,4 +610,3 @@ Changes ==================== - initial release -