Makefile version of build_deps.sh #417
Conversation
| cp $(llvm_builddir)/lib/libgtest_main.a $(llvm_installdir)/lib/ | ||
| cp $(llvm_builddir)/lib/libgtest.a $(llvm_installdir)/lib/ | ||
|
|
||
| llvm-build: |
There was a problem hiding this comment.
This probably needs to be .PHONY to keep llvm-build target always out-of-date.
| make -C $(llvm_builddir) -j4;\ | ||
| make -C $(llvm_builddir) -j4 install;\ | ||
| fi | ||
| alive2 : third_party/alive2 |
There was a problem hiding this comment.
Same .PHONY thing as above. There are other targets in this file as well which are not marked as .PHONY. I guess the problem will start appearing when there are files in the filesystem with exact same name as these targets. make won't build these targets because it will think that these targets are already built (even though you want them to be built unconditionally everytime).
| NINJA=ninja | ||
| # Override NINJA from the command line with something like NINJA="ninja -j1" if you are running out of memory | ||
|
|
||
| deps : alive2 llvm hiredis klee |
There was a problem hiding this comment.
nitpick: let's be consistent and not have any space before the colon.
|
Thanks for the comments, will fix and test. |
|
I'm still not sure what problem this change is intended to solve |
|
also, if we're going to do this, I'd like the full set of dependencies encoded in the makefile |
|
and then this all needs to be tested |
My reason: If llvm build is interrupted for any reason, it is annoying to nuke third_party and start from scratch. This makefile cleanly splits up cloning+cmake and the build steps, so that the second one can be resumed without redoing the first.
Will do. |
|
this seems reasonable to be clear, I'm not excited about this PR since the use case is not compelling -- but I'll merge it if there's some added value beyond picking up interrupted builds of dependencies |
|
|
||
| deps : alive2 llvm hiredis klee | ||
|
|
||
| llvm : third_party/llvm llvm-build |
There was a problem hiding this comment.
we shouldn't rely on the order of dependencies here. if you want llvm-build to be executed after third_party/llvm, then i think better to make third_party/llvm a dependency of llvm-build. Parallel make can mess things up otherwise. See: https://stackoverflow.com/questions/9159960/order-of-processing-components-in-makefile/9160175#9160175
Using the llvm version info as name of the file (eg: llvm70) and |
|
sounds good-- I want you to test this though! |
|
@LebedevRI Do you have any suggestions for improving this Makefile? |
There was a problem hiding this comment.
@LebedevRI Do you have any suggestions for improving this Makefile?
Hm, not particularly, i haven't written all that many makefiles by hand.
The one general comment was already said i think - unless this is going to replace
the existing build_deps.sh, they will get out of sync eventually..
| mkdir -p $(llvm_builddir) | ||
|
|
||
| if [ -n "`which ninja`" ]; then\ | ||
| $(NINJA) -C $(llvm_builddir);\ |
There was a problem hiding this comment.
Do you need something like set -e, so we abort on the first failure?
|
Just had a OOM while building debug build of souper (LLVM was being built). This PR would help greatly because I had to hack the current build_deps.sh script so that it doesn't start compiling LLVM from scratch. |
|
@pranavk what would happen if I was in the middle of cloning LLVM and my connection got interrupted-- would this makefile gracefully recover from that or would it simply see that the directory exists and move forward, failing in a perhaps-confusing way? |
|
i am not sure about this makefile; i don't think so. i guess manasij is working on it and will update this PR soon; putting those .PHONY attributes to make sure that we unconditionally process some targets should fix the issue. i'll have a look and test it after he updates this PR. |
|
Wouldn't unconditionally processing the target mean LLVM is cloned even if
it is already cloned?
I can imagine another hacky solution: create a dummy file iff git clone
exists successfully. Deleting that file would be a way to force a refectch.
…On Mon, Jan 28, 2019, 11:31 AM Pranav Kant ***@***.*** wrote:
i am not sure about this makefile; i don't think so. i guess manasij is
working on it and will update this PR soon; putting those .PHONY attributes
to make sure that we unconditionally process some targets should fix the
issue. i'll have a look and test it after he updates this PR.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#417 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABLavxkte26RjrfHNSWAbo1fBWy3-KANks5vHzQJgaJpZM4aHH1v>
.
|
|
how about an 'if' condition in the target that checks if .git/ directory exists. If (exists), git pull, otherwise clone? Or something along those lines. |
|
it's clear that all of these issues can be solved |
|
I'm just saying that the thing being solved here isn't something that has caused me pain over multiple years of being a souper developer, and it's very likely to not cause you guys much pain either, once you get your machine issues straightened out |
|
and also there are plenty of corner cases where you can easily make things worse, whereas "nuke it all and rebuild" is very easy to think about |
No description provided.