search

ndmitchell / rattle

Forward build system with speculation and caching
Other
102 stars 5 forks source link

speculative bug fix #33

Closed spall closed 2 years ago

spall commented 2 years ago

I tested that the new code at line 228 actually raises the hazard by making it always throw a hazard and running it. I wasn't sure how to add this to the existing test code since it is a race condition.

CmdRattleFinished now tells us when the cmd started, stopped, and the files it read, because we only care about the read files during our hazard re-checking. Then if the cmd is requested again addhazardset is called again near like 228 with those 3 pieces of info. And, it doesn't matter if the hazard set is changed, because we discard it after this check.

ndmitchell commented 2 years ago

Thanks for the fix - very nice and localised!

spall commented 2 years ago

I forgot about the lack of printing of commands; not sure if you have an idea about that.

ndmitchell commented 2 years ago

It's cosmetic, so happy to address it later - no idea what is causing it, as the code here doesn't seem related. Are you sure this is the diff that breaks it?