Make builds actually use conda_deps stage, fix conda_deps stage

[bquan0]

Description

Closes #1515. Removes the spaces in the build args for pkg_deps in the multistage actions in docker_publish.yml so that the conda_deps stage will actually be used when the build arg is conda. Modifies the conda_deps stage in ubuntu_22.04-dev.dockerfile so that it actually works when images are built with the conda_deps stage.

Motivation and Context

Previously, the conda_deps stage wasn't being used in builds of future stages because the build arg in the multistage action was pkg_mgr = ${ matrix.pkg_mgr} instead of pkg_mgr=${ matrix.pkg_mgr}. This meant that all images were being built off the default apt_deps stage. Thus, it seemed like the conda_deps stage was working and was being used when it wasn't.

After removing the spaces, the builds that supposedly used the conda_deps stage stopped working because they were now actually using the conda_deps stage. This was because there were many dependencies missing in that stage, like make, gxx_linux-64, gcc_linux-64.

Behavior

Currently, the conda_deps builds do not work, and I suspect it has to do with the fact that inside the conda_deps stage, we do an ENV PATH /opt/conda/bin:$PATH which messes up the PATH env variable for future uses. For example, line 104:

RUN echo "export PATH=$HOME/.local/bin:\$PATH" >> ~/.bashrc

I think this because the apt_deps stage doesn't modify the PATH env variable, and because this most recent workflow run (no hdf5) shows it having some undefined references while this hdf5 run shows it not being able to find the libh5test.a file.

Other Information

The description of #1515 was a bit cryptic but I was able to figure out the problem in the end.

[ahnaf-tahmid-chowdhury]

For conda pkg build, I think we can only go with one build method (apt or conda)??? Is it necessary to support both?

[gonuke]

I think it is good to test with multiple packaging systems because our users may want/need different systems and this helps us be sure that they work

[bquan0]

Ben suggested a possible solution in this link and I tried it. Unfortunately, it didn't work, with the problem being that the workflow can't find Eigen3.

[ahnaf-tahmid-chowdhury]

It seems strange to me because, as far as I know, Eigen3 is not a dependency of MOAB.

[ahnaf-tahmid-chowdhury]

It seems Eigen is installed through conda. Perhaps MOAB is unable to detect Eigen provided by conda? I think we can directly export EIGEN3_DIR and give it a try for a temporary fix.

[ahnaf-tahmid-chowdhury]

Or, we can use the conda build of MOAB and skip the source build of MOAB, which is more convenient, I suppose.

[bquan0]

I tried using the conda build of MOAB but that caused an error with cmake not being able to find the MOAB directory in the dagmc stage. Specifically, this line && cmake .. -DMOAB_DIR=$HOME/opt/moab \ which I changed to && cmake .. -DMOAB_DIR=$HOME/opt/conda/pkgs/moab-5.5.1-nompi_notempest_py310h89735b2_100 \ since that's the moab directory in the conda pkgs folder in the docker image that I built on my computer.

[gonuke]

It seems strange to me because, as far as I know, Eigen3 is not a dependency of MOAB.

Eigen3 is definitely a dependency of MOAB

[ahnaf-tahmid-chowdhury]

I tried using the conda build of MOAB but that caused an error with cmake not being able to find the MOAB directory in the dagmc stage. Specifically, this line && cmake .. -DMOAB_DIR=$HOME/opt/moab \ which I changed to && cmake .. -DMOAB_DIR=$HOME/opt/conda/pkgs/moab-5.5.1-nompi_notempest_py310h89735b2_100 \ since that's the moab directory in the conda pkgs folder in the docker image that I built on my computer.

It should be MOAB_DIR=CONDA_DIR, in this case /opt/conda. However, for the conda build we can use the conda version of DAGMC and skip the source build. In that way we will be building only PyNE from source and other comes from conda pkg.

[ahnaf-tahmid-chowdhury]

Eigen3 is definitely a dependency of MOAB

Thanks for letting me know!

Ops! I have thought Eigen3 is Embree since I was working with that last time 🙂

[bquan0]

Just had a workflow run that got to the moab stage and didn't have the CC, CXX, CPP ENV variables in the conda_deps stage. This workflow run has the same problem as previous workflow runs that did have the ENV variables, which makes me think that the ENV variables aren't actually doing anything. For reference, the error is that the linker looks for libstdc++.so.6 in /opt/conda/bin/../lib/gcc/x86_64-conda-linux-gnu/13.2.0/../../../../x86_64-conda-linux-gnu/bin/ld: but it doesn't exist there; rather, it exists in /opt/conda/lib.

Now I'm wondering how I can change which folder the linker looks in.

[bquan0]

The "no HDF5" build is finally working after adding this line in the conda_deps stage: ENV LD_LIBRARY_PATH /opt/conda/lib:$LD_LIBRARY_PATH

The "HDF5" build isn't working because make cannot find ./.libs/libh5test.a: No such file or directory.

[ahnaf-tahmid-chowdhury]

According to the log, it seems you haven't configured the hdf5 root path to conda. It is set to /opt/hdf5

[ahnaf-tahmid-chowdhury]

Move HDF5_INSTALL_PATH near to LD_LIBRARY_PATH and configure it for both apt and conda. We may move all the source build to the apt_deps stage and the conda_deps stage may contain only conda packages. If you are not familiar with multi stage docker, please let me know.

[bquan0]

Looks like the conda + hdf5 build is now working. I will fix the apt + hdf5 build with the next commit and also clean up the dockerfile so that it will be ready to be merged.

[bquan0]

All of the builds are now working!

[gonuke]

This seems nearly ready, once we relax the numpy restriction

[gonuke]

I wonder if we should undo/revert all the conda build stuff and approach it more carefully, rather than trying to patch this.

[gonuke]

I propose that we withdraw this in favor of two separate actions with two separate Dockerfiles that build under apt and conda, respectively

[ahnaf-tahmid-chowdhury]

@bquan0, what is your plan for this? Would you like to modify/close it and go with @gonuke's proposal? Or, I could take care of it with a new PR.

[bquan0]

@ahnaf-tahmid-chowdhury I will close this PR and open a new PR that applies the change that @gonuke proposed.