Circular dependency - strange result about marionette HOT 8 CLOSED

I've done some further testing and I'm not seeing anything else circular dependency related so this could probably be closed. I have found something else but I'll open a separate issue for it.

from marionette.

I could almost understand what is happening here to result in two, one, two:

• There are three rules.
• They should be run in order
  • So log1 runs.
  • But that requires log2 as a dependency.
  • So log2 runs : Outputs "two".
  • Now that the dependency has been executed log1 runs
    • Which outputs "one".
  • ignoring the fact that it has already been run .. log2 runs, as that's the next rule.

The surprise is almost that we don't see:

• two
• one
• two
• one
• three

I'm going to guess there's a bug in the handling of the "We've seen this rule already" code. It's either running the dependency, and not updating the seen-state, or it's running the rule without checking that the seen-state was updated as the dependency was previously-executed.

I'll take a look tomorrow, unless you want to explore in advance.. As you say it's almost hard to know what should happen. But I'd guess myself the correct result is:

• three

• two

• one

• rule one launches

  • but that requires rule two
  • rule two launches
    • but that requires rule three
    • so "THREE"
  • so "TWO"

• so "ONE"

• At that point the loop is broken because all the rules should be reported as "Already run".

I'm further guessing, we don't recurse for dependencies.

So if "one requires two" we'll handle that. But if "one requires two, which requires three", we'll only treat that as the first case.

from marionette.

Initial thoughts on a plan of attack:

• Ensure dependencies follow the chain.
  • Break loops by ignoring any dependency which is already present for a rule - including the initial one.

Then separately add a "seen" bool to the rule-struct, and mark that. Drop the use of e.seen

I guess there's going to have to be an overhaul of the notifier too:

• If rule1 notifies rule2
• And rule2 is supposed to notify rule3
• We almost certainly ignore that second trigger.

Seeing this bug-report almost makes me wonder how things are as useful as they are, and how it even works. Good find! I guess this is the sort of thing a proof of concept is supposed to identify, though I've already started to rely on it, so it's moved past that state..

from marionette.

Currently building a full dependency chain wouldn't be possible because there may be dependent rules in other files which aren't parsed until they're included.

I wonder if there's any merit in parsing all includes up front and then giving the rules within an implicit dependency on the file being included.

For illustration purposes only as the dependency would be implicit, not suggesting a change in syntax, it could work similar to

# main.txt
include {
    name => "include-file",
    source => "include.txt",
}

# include.txt
log {
    message => "In included",
    require => "include-file",
}

from marionette.

Agreed, inclusions complicate things such that we can't really have dependencies expressed across files.

I'll have to think about this some more. I think it is clear there are two distinct bugs here:

• Dependencies only stop at one level.
• We don't detect loops.

Those can be fixed, but the issue of handling dependencies properly, especially when included-files are used, is going to be hard without a significant overhaul. (It would have been easier/possible if the parser expanded things at parse-time - then all the rules would have been available at once, but I think moving those to run-time was the correct decision.)

from marionette.

Opened a pull-request which significantly improves how dependencies are handled/executed.

However I've not explicitly detected cycles, nor made changes with regard to include-files.

With that code running the example code works as "expected":

frodo ~/Repos/github.com/skx/marionette $  go build . ; ./marionette log.recipe 
2022/01/11 10:38:07 [USER] three
2022/01/11 10:38:07 [USER] two
2022/01/11 10:38:07 [USER] one
frodo ~/Repos/github.com/skx/marionette $ 

from marionette.

That certainly improves things. I'll try out some more things and see if I can break it again. 😄

from marionette.

Good luck!

I'll merge, but leave this open. I guess this, and #74, are kinda bigger than a single example. It might be that another large overhaul is required to fix things up. (Or made the decision to keep things simple and leave the limitations alone!)

from marionette.