Fix Cassius, Lucius reload mode

[alx741]

Fix #206
Fix yesodweb/yesod#1425

[psibi]

@alx741 Are you sure this is a right fix ? I think you should just remove the directives and qAddDependentFile should be present.

[alx741]

@psibi well, cassiusFile does have qAddDependentFile: https://github.com/yesodweb/shakespeare/blob/master/Text/Cassius.hs#L57-L63 , this is spected as it is the non-reloading one, but cassiusFileReload and luciusFileReload both use cssFileDebug: https://github.com/yesodweb/shakespeare/blob/master/Text/Internal/Css.hs#L172 , which is not supposed to add de files as compilation dependencies.

Is there perhaps a reason to keep qAddDependentFile that I'm not seeing?

Edit: According to my reasoning removing the directives wouldn't do much, as GHC is going to be >= 7.4 in most cases, plus I think the reason for that restriction is that addDepedentFile is only supported since GHC 7.4

[psibi]

Why do you think cassiusFileReload and luciusFileReload aren't supposed to add files as compilation dependencies ?

Only if you add files as compilation dependencies, they will get re-compiled IIRC.

[alx741]

Whenever addDepedentFile is used, the file that uses the dependent file is marked for recompilation. For instance, in the case of Yesod, cassiusFile and luciusFile require it, as any change in the corresponding template file should mark the invoking handler for recompilation; But when the reload versions are used it shouldn't make the handler recompile (that's the reason of existence of the reload versions) and, because they are read at runtime, they don't need to force recompilation of the handler. One can actually see this happening when observing the effects of the changes in the template files right before the recompiltion begins, as described here: yesodweb/yesod#1425

This way changes in template files mark the invoking Haskell file for recompilation when the normal versions are used, but doesn't when the reload versions are used. Notice how this is the behavior for Julius on the reload version: https://github.com/yesodweb/shakespeare/blob/master/Text/Julius.hs#L197 (no addDependentFile there), whereas the normal version uses shakespeareFile which, in turn, does make use of addDependentFile: https://github.com/yesodweb/shakespeare/blob/master/Text/Shakespeare.hs#L396

[snoyberg]

@psibi This was coming from my recommendation. The behavior we want is that, with yesod devel, runtime loaded files do not trigger recompilation, which is currently not the behavior (at least on @alx741's system, for some reason the behavior on my system is different).

[alx741]

@snoyberg that's the part that intrigues me though, as far as i understand it, it should be reproducible as long as that GHC_7_4 flag is set

[snoyberg]

I checked in my .hi files, I also have the lucius/cassius files listed. I don't know why on my system Stack isn't rebuilding for them. Maybe it's a bug in the devel version of Stack. In any event, the .hi files are authoritative here.

[alx741]

On the other hand... those 3 lines seem to be there for about 5 years now. Either my reasoning about them is incorrect, or there is something weird going on somewhere else.

[snoyberg]

@psibi Do you have any objections to removing this?

@alx741 Can you add a note to the changelog and a minor version bump in the cabal file to explain why the change is happening?

[psibi]

LGTM. @alx741 Does removing this line actually fixes the problem for you ?

[alx741]

@psibi oh yeah! it did. I tried with a standalone executable that loads a cassius file (with reload mode), and both in an existing Yesod project and a freshly scaffolded one. The needless recompilation is gone in all of them :)