-
-
Notifications
You must be signed in to change notification settings - Fork 207
Add top-level makefile #1085
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add top-level makefile #1085
Conversation
e16265f to
36c49e6
Compare
|
This looks awesome! |
eed8f37 to
b02b769
Compare
b02b769 to
350285d
Compare
350285d to
9134370
Compare
|
Alright, I think this is ready for final review now. First up, I'm glad you like the approach. TBH, I was a bit torn. A simple bash script is easier to grasp than makefile "magic". But overall I think it's still worth it for being more elegant and hopefully also more fun to work with. There was actually still a bug: Since both the test binary as well as the memcheck targets are supposed to produce the respective binary, they were considered successful even when running either binary would report an error. You would see this when running it for the first time (that there is some error), but subsequent runs would just have make say there is nothing to be done (because the targets' outputs are there). This is fixed now by introducing stamp files that are only created if tests/memcheck pass. The top-level target of an exercises now depends on those stamp files. This way it is only considered "done" when both stamp files are present (which is only the case when all tests pass). The discussed checks for empty concept/practice exercises lists are also implemented. Currently, Lastly, I added a Let me know if I missed anything in terms of functionality or if there's anything else you'd like to have differently! |
0e69842 to
9134370
Compare
|
For this run I just reverted the |
|
This looks great to me. @wolf99 I'm leaving this un-merged in case you want to have a look prior to merge. |
|
Great work @ahans , well done! |
Motivated by the fact that we need to add support for
exemplar.{c,h}to support concept exercises, instead of extending the existingbin/run-testsscript, here's a proposal for a top-level makefile. I have found therun-testsscript always a bit cumbersome to work with. We used something very similar over at arm64-assembly and have a top-level makefile there now as well.The advantages of this approach are that instead of simply copying over and modifying files, we tell make about each exercise and its dependencies. This way we can leverage make's builtin support for parallelism and, most importantly, let make figure out what needs to be rebuilt: you run
makeonce to build/test everything, then change some file in the tree (say add a test case to some exercise), and then the nextmakewill just rebuild/retest the changed exercise.@wolf99 @ryanplusplus Let me know what you think! Thanks!