-
Notifications
You must be signed in to change notification settings - Fork 9
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
enhanced travis build setup #33
Conversation
This is a work in progress and may take a few iterations before I can get the travis tests to complete. I will update when it is ready for review. |
This is great. Thanks. If you see some incremental reviews of some sort, just let me know. Just make sure your commits are reasonably topical. |
yup, I will be doing a lot of squashing just to get it working so as not to generate too many commits. |
fe46040
to
222a908
Compare
@dongahn It looks like everything is building and running OK. How does it look to you? The Flux script and config file made a nice template. I'm going to use it for STAT too. |
Nice work! I will leave a few minor in-line comments but things look great. |
.travis.yml
Outdated
- $HOME/openmpi-gcc | ||
- $HOME/openmpi-clang | ||
- $HOME/local | ||
- $HOME/.luarocks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably don't use this cache directory. Safely remove this?
.travis.yml
Outdated
- $HOME/openmpi-clang | ||
- $HOME/local | ||
- $HOME/.luarocks | ||
- $HOME/.local |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the use of this cache directory either from the dep script. This can go also?
@lee218llnl: this is a huge improvement! Once these changes are made, like you said, do squash the new commit to the old one; then force a push: Since this addresses the baseline capacity of Issue #25 (thought with OpenRTE), I will close that one once merged and create a new ticket with "TAP" support. |
test/src/travis-dep-builder.sh
Outdated
\n | ||
Usage: $prog [OPTIONS]\n\ | ||
Download and install to a local prefix (default=$prefix) dependencies\n\ | ||
for building flux-framework/flux-core\n\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow, my provious comment is gone... hmmm. Any case, please change "flux-framework/flux-core" to "LaunchMON"
test/src/travis-dep-builder.sh
Outdated
echo "export LD_LIBRARY_PATH=${prefix}/lib:$LD_LIBRARY_PATH" | ||
echo "export CPPFLAGS=-I${prefix}/include" | ||
echo "export LDFLAGS=-L${prefix}/lib" | ||
echo "export PKG_CONFIG_PATH=${prefix}/lib/pkgconfig:${prefix}/share/pkgconfig" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once I used pkgconfig when LaunchMON had gcrypt integrated in it directly. But this isn't the case any longer. This can be dropped.
test/src/travis-dep-builder.sh
Outdated
echo "export CPPFLAGS=-I${prefix}/include" | ||
echo "export LDFLAGS=-L${prefix}/lib" | ||
echo "export PKG_CONFIG_PATH=${prefix}/lib/pkgconfig:${prefix}/share/pkgconfig" | ||
echo "export PATH=${PATH}:${HOME}/.local/bin:${HOME}/local/usr/bin:${HOME}/local/bin" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop ${HOME}/.local/bin:
?
222a908
to
11ee645
Compare
11ee645
to
2295202
Compare
@dongahn I think I addressed all of your comments. |
LGTM. Thanks! |
No description provided.