cycle detection!

19 views
Skip to first unread message

Evan Laforge

unread,
Feb 15, 2017, 6:45:16 PM2/15/17
to Shake build system
I just upgraded shake and discovered it now detects cycles in rules.
This is a big improvement over the old behaviour, which was to
silently quit while compiling, which looked a lot like a successful
compile, which led to a lot of confusion.

So thanks for the fix!

One quibble though, is the output is hard to read. For instance, I
get this output:

shakefile: Build system error - indirect recursion detected:
Key value 1: build/test/obj/Cmd/Im_test.hs.o build/test/obj/Cmd/Im_test.hi
Key value 2: build/test/obj/Cmd/SaveGit_test.hs.o
build/test/obj/Cmd/SaveGit_test.hi
Key value 3: build/test/obj/Cmd/Responder_test.hi
Key value 4: build/test/obj/Cmd/SaveGit.hs.o build/test/obj/Cmd/SaveGit.hi
Key value 5: build/test/obj/Cmd/Repl/Fast.hs.o
build/test/obj/Cmd/Repl/Fast.hi
Key value 6: build/test/obj/Cmd/Repl/Fast.hi
Key value 7: build/test/obj/Cmd/Undo_test.hs.o
build/test/obj/Cmd/Undo_test.hi
...
Key value 60: build/test/obj/Cmd/Integrate_test.hs.o
build/test/obj/Cmd/Integrate_test.hi
Rules may not be recursive

Meanwhile, if I load the main module into ghc I get this:

Module imports form a cycle:
module ‘Cmd.SaveGit’ (./Cmd/SaveGit.hs)
imports ‘Local.Config’ (./Local/Config.hs)
which imports ‘Cmd.SaveGit’ (./Cmd/SaveGit.hs)

Which makes it a lot easier to see what the problem is. Incidentally,
ghc also used to have a harder to read error, until I complained about
it... so maybe I'm the only one who makes cycles?

If there could be a clearer error that would be great, but mostly I'm
just happy that there is an error at all!


Also I'm happy to see that #4 has been fixed. Now I can drop my local
locking implementation. Clearly I should upgrade shake more often :)

Neil Mitchell

unread,
Feb 16, 2017, 2:23:48 AM2/16/17
to Evan Laforge, Shake build system
Hi Evan,

Yep, it must have been a while - #4 got fixed and released in April last year.

For the cycle detection, bug
https://github.com/ndmitchell/shake/issues/400 is the open issue.
Essentially there are two types of recursion that Shake detects. In
one case, it spots a recursive loop in the stack, and stops, quite
happily, with a good and precise error message. In the other case,
Shake uses multiple threads that race, and together they form a cycle
- but neither does on their own. In these cases Shake used to
terminate without completing and incorrectly declare success - I've
fixed that. Trying to get a better error message is much trickier, as
the stack isn't encoded explicitly anywhere I can access it, but the
ticket has a great suggestion of something that might work, which I'm
going to try. Hopefully in another year it will be improved too!

Thanks, Neil

Evan Laforge

unread,
Feb 16, 2017, 4:53:42 PM2/16/17
to Neil Mitchell, Shake build system
Excellent, keep up the good work. I'll keep an eye out for new
version announcements.
Reply all
Reply to author
Forward
0 new messages