build plugin in docker #65
Conversation
Co-authored-by: Paul Wilson <paul.wilson@wisc.edu>
|
@gonuke I think I answered all you open comments |
|
I'm happy to merge if @pshriwise is all set |
|
@pshriwise do you have any additional or un-adressed comments ? |
There was a problem hiding this comment.
Looks pretty good @bam241!
A few line comments and a couple of general thoughts:
-
This is a very small thing, but I think we should name the
scriptdirectoryscripts. -
I know you mentioned that you still intend to add documentation for this script, but at this point someone would have to check the PR history to figure out how to use these correctly. Can you add a short README file in this directory on how the script should be used for now?
script/linux_share_build.sh
Outdated
| mkdir ${PLUGIN_DIR} | ||
| echo "Building the Trelis plugin in ${CURRENT}\\${PLUGIN_DIR}" | ||
|
|
||
| unset LD_LIBRARY_PATH |
There was a problem hiding this comment.
I don't see the LD_LIBRARY_PATH being set anywhere else, so I think this can be removed.
There was a problem hiding this comment.
if you run this on your computer one want to be sure that the LD_LIBRARY_PATH is not set to ovoid conflict...
There was a problem hiding this comment.
@pshriwise I believe this is the only outdenting comment.
BU this was done on purpose for safety....
There was a problem hiding this comment.
Gotcha. I was understanding these as intended for use in docker only, meaning we'd have a fresh environment. Nice to be able to use them in the native kernel though -- have we tested that?
All that aside, would this command be more appropriate in the build_plugin.sh file? It would be more clear that the LD_LIBRARY_PATH is unset by these scripts I think.
script/build_plugin.sh
Outdated
| build_moab | ||
| build_dagmc | ||
|
|
||
| setup_Trelis_sdk $1 |
There was a problem hiding this comment.
I know you still have documentation to work on, but can we add a comment here for what we expect the argument, $1, to be?
script/linux_share_build.sh
Outdated
| cd ${PLUGIN_ABS_PATH} | ||
| mkdir -pv DAGMC/bld | ||
| cd DAGMC | ||
| git clone https://github.com/bam241/DAGMC -b build_exe |
There was a problem hiding this comment.
This repo and branch can be updated with the merge of your PR in DAGMC
script/linux_share_build.sh
Outdated
| -DCMAKE_POSITION_INDEPENDENT_CODE=ON \ | ||
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -DCMAKE_INSTALL_PREFIX=${PLUGIN_ABS_PATH}/DAGMC | ||
| make -j`grep -c processor /proc/cpuinfo` |
There was a problem hiding this comment.
We use this line in multiple places, let's make the number of make jobs a variable.
Co-authored-by: Patrick Shriwise <pshriwise@gmail.com>
|
@pshriwise @gonuke I added a README file and some comments, plz let me know what you think! |
Co-authored-by: Patrick Shriwise <pshriwise@gmail.com>
2 scripts to build the plugin in docker (for linux),
still need to work on documentation, to explain where to place SDK and deb file, and to detail the argument to provide.
basically
script/set_docker_env.sh "ubuntu:18.04" "$HOME/work/app/Trelis_pkg/" 17.1.0should work...