pydoit / doit

CLI task management & automation tool
http://pydoit.org
MIT License
1.85k stars 175 forks source link

Task can have only one config_changed() #333

Closed NathanTP closed 4 years ago

NathanTP commented 4 years ago

Bug Description

The config_changed() helper object stores the config under a static name. This means that tasks can only have one instance of config_changed() in their uptodate list. This is easy to reproduce:

def task_works():
    return {
            "actions" : [ (lambda : print("Hello World"), []) ],
            'verbosity': 2,
            "uptodate" : [config_changed("a")]
            }

def task_breaks():
    return {
            "actions" : [ (lambda : print("Hello World"), []) ],
            'verbosity': 2,
            "uptodate" : [config_changed("a"), config_changed("b")]
            }

task_works() will be built the first time, and will be up to date after that. task_breaks() will never be up to date (the db will have 'b' in '_config_changed', so 'a' won't match).

This issue is caused by line 70 in tools.py.

Proposed Fix

I have fixed this in my project by locally redefining config_changed to assign a unique ID to each config_changed object. In my solution, the unique ID is simply the order in which it was added to the uptodate list:

    def configure_task(self, task):
        # Give this object a unique ID that persists between calls (ID is the
        # order in which it was evaluated when adding)
        if not hasattr(task, '_config_changed_lastID'):
            task._config_changed_lastID = 0
        self.saverID = str(task._config_changed_lastID)
        task._config_changed_lastID += 1

        configKey = '_config_changed'+self.saverID
        task.value_savers.append(lambda: {configKey:self.config_digest})

    def __call__(self, task, values):
        """return True if config values are UNCHANGED"""

        configKey = '_config_changed'+self.saverID

        self.config_digest = self._calc_digest()
        last_success = values.get(configKey)
        if last_success is None:
            return False
        return (last_success == self.config_digest)

I'm not sure if this is robust to all the use cases (it's working for me so far).

Environment

  1. OS: Centos 7
  2. python version: 3.6
  3. doit version: 0.31.1 (although the bug remains in master at line 70 in tools.py)

Background

For the interested, I'm using doit as the underlying dependency management framework in the FireMarshal workload management tool. Tasks are generated from user-provided configurations along with some base distributions. This config_changed() thing is an issue for me because I always include one config_changed() dependency (for GCC version) and I want the task generators to be able to define their own (e.g. to check if some submodule git revision has changed).

schettino72 commented 4 years ago

config_changed() also accepts a dict as parameter. You can think of the key in dictionary as being the unique ID you are looking for.

schettino72 commented 4 years ago

@NathanTP thanks for writing about your project. If you feel like it would be welcome addition to our "success stories" in docs