-
Notifications
You must be signed in to change notification settings - Fork 65
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
Putting install scripts into dockerfile #822
Putting install scripts into dockerfile #822
Conversation
When building this new dockerfile with the defaults (moab 5.4.0) I am getting a Pymoab / PYTHONPATH error which is similar to the error I am seeing on PR #816 58%] Building CXX object test/CMakeFiles/intx_in_plane_test.dir/intx_in_plane_test.cpp.o
Complete output from command /usr/bin/python3 -c "import setuptools, tokenize;__file__='/root/build_dir/moab/bld/pymoab/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" develop --no-deps --prefix=/root/build_dir/moab/bld/pymoab:
running develop
Checking .pth file support in /root/build_dir/moab/bld/pymoab/lib/python3.6/site-packages
/usr/bin/python3 -E -c pass
TEST FAILED: /root/build_dir/moab/bld/pymoab/lib/python3.6/site-packages does NOT support .pth files
error: bad install directory or PYTHONPATH
You are attempting to install a package to a directory that is not
on PYTHONPATH and which Python does not read ".pth" files from. The
installation directory you specified (via --install-dir, --prefix, or
the distutils default setting) was:
/root/build_dir/moab/bld/pymoab/lib/python3.6/site-packages
and your PYTHONPATH environment variable currently contains:
''
Here are some of your options for correcting the problem:
* You can choose a different installation directory, i.e., one that is
on PYTHONPATH or supports .pth files
* You can add the installation directory to the PYTHONPATH environment
variable. (It must then also be on PYTHONPATH whenever you run
Python and want to use the package(s) you are installing.)
* You can set up the installation directory to support ".pth" files by
using one of the approaches described here:
https://setuptools.readthedocs.io/en/latest/easy_install.html#custom-installation-locations
Please make the appropriate changes for your system and try again.
----------------------------------------
Can't roll back pymoab; was not uninstalled
Command "/usr/bin/python3 -c "import setuptools, tokenize;__file__='/root/build_dir/moab/bld/pymoab/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" develop --no-deps --prefix=/root/build_dir/moab/bld/pymoab" failed with error code 1 in /root/build_dir/moab/bld/pymoab/
pymoab/CMakeFiles/pymoab-local-install.dir/build.make:58: recipe for target 'pymoab/CMakeFiles/pymoab-local-install' failed
make[2]: *** [pymoab/CMakeFiles/pymoab-local-install] Error 1
CMakeFiles/Makefile2:1238: recipe for target 'pymoab/CMakeFiles/pymoab-local-install.dir/all' failed
make[1]: *** [pymoab/CMakeFiles/pymoab-local-install.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs.... |
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.
Thanks @shimwell - this looks like a good step forward!
I recognize that some of these suggestions represent changes of intent that we may want to leave for a separate PR & discussion.
Co-authored-by: Paul Wilson <paul.wilson@wisc.edu>
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.
A few issues introduced with the embree directory variables
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 think we're getting closer....
CI/Dockerfile
Outdated
RUN mkdir -p ${embree_build_dir}/build && \ | ||
cd ${embree_build_dir} && \ | ||
git clone -b ${EMBREE_BRANCH} https://github.com/embree/embree && \ | ||
mkdir build && \ |
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.
No longer needed - done in #103
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.
Sorry I am not quite sure which part of this embree clone/install was done in #103
I have removed the 2nd mkdir build
if that is what was needed
CI/Dockerfile
Outdated
/root/etc/CI/docker/build_moab.sh; \ | ||
fi; | ||
# Set the hdf5 install dir as this is also needed by moab compile | ||
ENV hdf5_install_dir=${install_dir}/hdf5 |
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.
This is done in line 130 isn't it?
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.
Raising this one again...
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 thought we needed to declare ENV
in each build stage they are used as they were not carried between build stages.
A moab compile error has appeared when trying to build this docker file locally with moab 5.4.0.
|
For some reason, these latest commits didn't seem to trigger rebuild of the images on your fork??? |
Sorry about that, I've just manually trigger it |
🙏 work this time. Latest action link is here |
Got to the stage where it fails dagmc tests on my CI fork actions Anyone got any ideas on this output error, looks like it can't find some h5m files (iter_imprinted.h5m and bllite30matls.h5m) which I also can't find on the repo
|
maybe they are created by the tests. But were the |
or do the |
I think I found it! There is a whole logic in the current system about whether it is a pull request or not (then a merge) that determines whether or not to run this regression test. It should only run on merge. Simultaneously, if it is not a pull request, there are lines to wget the missing files!! |
Shall we simplify that logic so that it always downloads the model and runs the test I can see this wget command in the install.sh script wget ${MW_REG_TEST_MODELS_URL} -O mw_reg_test_files.tar.gz -o wget.out I did a quick grep for the |
${docker-env} file? |
All these bash scripts should have |
This PR is aiming to get rid of the scripts! |
Hmmmm.... it appears to have been lost in CI transitions... |
CI/Dockerfile
Outdated
-DBUILD_GEANT4=ON \ | ||
-DGEANT4_DIR=${geant4_install_dir} \ | ||
-DBUILD_CI_TESTS=ON \ | ||
-DBUILD_MW_REG_TESTS=${build_mw_reg_tests} \ |
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.
this line could be changed to avoid running these tests.
The tests download a tar.gz file from a url that we can't find anymore
-DBUILD_MW_REG_TESTS=OFF \
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.
Discussed this at the DAGMC meeting yesterday and this is the preferred route, I've opened issue #858 to track the missing tar.gz file with the models in it.
Latest attempt to get the dockerfile building is running on my forks actions over here |
Action on my fork is passing 🎉 |
If this one gets merged can I make a follow up one that removes the old scripts or is that a step too far |
Prior to #813, we had different dockerfiles for each stage, and each of those files used a different container that had already been pushed to GHCR. With a single dockerfile, we have stages that build on each other. At each step, we aim to build a particular stage, but with no caching, the CI runner has no idea that we've already built that stage. As a result, every step is rebuilding GEANT, HDF5, etc... including the step where you test the DAGMC install. |
The right way to fix this is probably introduce all the right caching, etc, but perhaps the multistage-docker-build-action is what we need to simplify this. |
Good spot yes, I can start implementing the multistage-docker-build-action on this open PR |
Closing in favour of #863 |
work in progress, not ready for review
Description
As mentioned in project https://github.com/svalinn/DAGMC/projects/6 it has been proposed to move the install scripts into the dockerfile.
Motivation and Context
This simplifies the passing of environments to cmake and other commands as they are all visible in one file and no longer creating in one script to be used in another.
Changes
What kind of change does this PR introduce? (Bug fix, feature, documentation update...)
refactored dockerfile to include more ARGs ENVs and install commands.
Behavior
What is the current behavior? What is the new behavior?
old = dockerfile copies in sh scripts and runs them, new = dockerfile runs install commands directly