Isn't the needMakefileDependencies pattern very dangerous?

[Edward Z. Yang]

Specifically, the pattern where you build the C file at the same time
you generate the make rules, and then declare them as needed aftewards.

Here's a distilled example of something bad happening:

import Development.Shake
import System.IO

main :: IO ()
main = shakeArgs shakeOptions $ do
want ["foo.out"]
"baz.out" %> \out -> do
need ["baz"]
liftIO $ writeFile out =<< readFile "baz"
"foo.out" %> \out -> do
need ["link"]
-- The recipe to build "foo.out" is to read a file name
-- from "link", and then copy the contents of that
-- file to "foo.out". Usually, this would be some
-- external compiler program.
link_raw <- liftIO $ readFile "link"
let link = length link_raw `seq` head (lines link_raw)
link_contents <- liftIO $ readFile link
liftIO $ writeFile out link_contents
-- According to the needMakefileDependencies idiom,
-- it should be OK to declare this dependency AFTER
-- the heavy lifting.
need [link]

Run it like this:

[ezyang@hs01 shtest]$ rm -rf link baz baz.out foo.out .shake
[ezyang@hs01 shtest]$ echo prebaz > link
[ezyang@hs01 shtest]$ echo A > prebaz
[ezyang@hs01 shtest]$ echo C > baz.out # a stale output file
[ezyang@hs01 shtest]$ echo B > baz
[ezyang@hs01 shtest]$ ./Main
Build completed in 0:01m
[ezyang@hs01 shtest]$ cat foo.out
A
[ezyang@hs01 shtest]$ echo baz.out > link
[ezyang@hs01 shtest]$ ./Main
Build completed in 0:01m
[ezyang@hs01 shtest]$ cat foo.out
C # INCORRECT!
[ezyang@hs01 shtest]$ ./Main
Build completed in 0:01m
[ezyang@hs01 shtest]$ cat foo.out
C # STILL INCORRECT!
[ezyang@hs01 shtest]$ echo D > baz
[ezyang@hs01 shtest]$ ./Main
Build completed in 0:01m
[ezyang@hs01 shtest]$ cat foo.out
D # BETTER

It seems that it is unsound to declare a dependency after it's been
used, since Shake doesn't go and rebuild the rule if someone adds
a dependency and Shake *knows* that it was out of date. It's not
too hard to get into this state; all you need is a stale output
file, or perhaps for the compiler to not complain when the file here
is missing (because it probes or something).

Edward

[Neil Mitchell]

Hi Edward,

No time to read through the whole email at the moment alas, but a
quick pointer towards needed and neededMakefileDependencies. Generally
use need to say I will use a dependency, and needed to say I have
already used it, which will do the appropriate error checking.

In the next version of Shake, there will be more checking if this goes
wrong. I should probably also alter the manual to use needed instead
of need.

Thanks, Neil

[Edward Z. Yang]

Thanks, that's exactly the distiction I was thinking of.

Edward

Excerpts from Neil Mitchell's message of 2015-12-13 02:12:57 -0800:

[Neil Mitchell]

Reading the question properly, I think it's worth:

* Updating the manual to use needed, and at that point explain the
distinction between need and needed:
https://github.com/ndmitchell/shake/issues/344. Part of the reason for
more reference to need is that need came first, and needed (which is
just need with a check) came much later. I think it's probably worth
rephrasing things to treat them as the two first-class dependency
mechanisms.

* Adding proper docs on lint checking,
https://github.com/ndmitchell/shake/issues/342. If you have proper
trackUse annotations then it's very easy to see that a file is used
before being depended upon. In the next release these features get a
massive boost with a new type of linter that automatically calls
trackUse, making such an error easy to spot.

Thanks, Neil