Add a clean check

[ndmitchell]

A phony "clean" which removes everything would be very useful to test issues where the build system is likely reporting an old issue and needs a wipe.

[angerman]

Just linking #32 here. Ha. Almost 100 issues in-between.

[ndmitchell]

100 issues or 15 days - you guys are moving fast!

[ndmitchell]

For info, if you delete shake-files/.db then Shake considers everything dirty and tries rebuilding it, which is a reasonably good way to test the build system.

[angerman]

100 issues or 15 days - you guys are moving fast!

:100:

For info, if you delete shake-files/.db then Shake considers everything dirty and tries rebuilding it, which is a reasonably good way to test the build system.

I'm running

$ make clean && make distclean && make maintainer-clean && git clean -df && ./boot && ./configure && rm -fR shake-build/.db && rm -fR shake-build/.shake 

to be on the safe side :)

[ndmitchell]

Can we add that to the readme, or as a target "superclean" - agreed it's probably too paranoid, but useful to know.

[snowleopard]

As soon as we handle #113 this issue should be straightforward to implement.

The plan is to move all build artefacts to .build directory, as discussed in #32.

[angerman]

@ndmitchell are you up to the phony "clean" Pull Request challenge? :-)

[ndmitchell]

I'm not totally sure what the phony should do. I guess rm .build, and shake-build/.shake and the include files that were floating around?

[snowleopard]

@ndmitchell And all stuff we put into inplace.

It's going to be rather tedious to implement and hard to maintain. An ideal solution would be to eventually move everything into .build.

[ndmitchell]

Shouldn't it be about 4 or 5 directory names which don't change that often?

[snowleopard]

With includes it's not that simple: there are some source files which we'd like to keep.

However, directories inplace/bin and inplace/lib seem to contain only build artefacts.

[ndmitchell]

:( - that does seem a bit painful. But I don't think having clean is optional, sounds like it might be one for @snowleopard though.

[snowleopard]

@ndmitchell Agreed, I'll do this after #113.

[ndmitchell]

As a note, you generally only need to use removeFilesAfter if you are specifically removing the .shake.database file, which is otherwise locked open. For everything else it's better to use liftIO $ removeFiles.

[snowleopard]

@ndmitchell Thanks, I will fix this. Do we want to remove the Shake database too when doing clean?

[ndmitchell]

I see no reason not to remove the shake database - it gives you the best chance of reproducing issues from scratch, which is a major goal of doing clean.

[snowleopard]

Done, I agree.

[ndmitchell]

Thanks for this, I was waiting for a clean target before I conducted any more testing, so I'll give my Windows/Stack build a try tonight.

[snowleopard]

I hope I haven't missed anything. If you spot any left-over files while testing please let me know.

[ndmitchell]

@snowleopard - why not just test it properly on the CI machines? At the end of the build do clean, and then check something like git diff --exit-code, perhaps ignoring certain files, perhaps not applying the .gitignore. If that list is "longer than you expect", then error out. For info, I do this on all my builds, especially to check that those libraries with generators have had the generators applied and checked in (I have that pattern in derive, filepath and extra).

[snowleopard]

@ndmitchell Good idea! I'm a bit concerned that this might increase CI time; we are already close to AppVeyor time limit, where we are at the very edge even when building GHC stage1 only. I don't want to lower the bar any further (below GHC stage1). Can you spot any optimisation opportunities?

Of course, we don't have to run this check on all CI instances.

Another idea: could we ingrate the proposed check right into the clean rule? Then it will become self-checking.

[ndmitchell]

I wouldn't put the check into clean - users might (probably will) have locally modified files, or other stuff they have put somewhere else in the directory, and we don't want to report that as an error. Adding a cleancheck target that does it (just so the code can be properly integrated) which itself depends on clean, isn't a bad idea.

I suspect the clean and check will take ~10s at most - it should not be an expensive thing to do (if it is, let me know). I think the best thing is to find optimisation opportunities in the rest of the build, configure in parallel being the most obvious one I saw. Plus you can only run this on Travis, not Appveyor.

[snowleopard]

So, let me reopen this issue as it requires further work.

[snowleopard]

@ndmitchell You seem to be very familiar with the solution you are proposing, perhaps, you could take this over? Here are the tasks we have:

• [ ] Add a clean check on Travis
• [ ] Implement a cleancheck target in the build system (optional)

[ndmitchell]

Certainly, with the caveat that it will be about 1-2 weeks before I do it (assuming I don't get time tonight). The remaining part is unimportant though so can remilestone til later.

[snowleopard]

Thanks @ndmitchell! Take your time. I don't think this is on the critical path.