-
Notifications
You must be signed in to change notification settings - Fork 2
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
gitlab: add flux-pmix to CI testing #10
Conversation
.gitlab-ci.yml
Outdated
- cd $CORE_BUILD_DIR | ||
- export PKG_CONFIG_PATH=$(pwd)/lib/pkgconfig:$(pkg-config --variable pc_path pkg-config) | ||
- module load openmpi | ||
- export PKG_CONFIG_PATH=/usr/tce/packages/openmpi/openmpi-4.1.2-intel-classic-2021.6.0-magic/lib/pkgconfig:$PKG_CONFIG_PATH |
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'd like to change this to be something more closely linked to the version under module load
, like:
ls $(which mpicc)/../../lib/pkgconfig
ls: cannot access '/usr/tce/packages/openmpi/openmpi-4.1.2-intel-classic-2021.6.0-magic/bin/mpicc/../../lib/pkgconfig': Not a directory
but as you can see that doesn't work. @grondo can you advise on how to do this?
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.
Since mpicc
is not a directory, you'd have to take the dirname first:
$ which mpicc
/usr/tce/packages/openmpi/openmpi-4.1.2-intel-classic-2021.6.0-magic/bin/mpicc
$ echo $(dirname $(which mpicc))
/usr/tce/packages/openmpi/openmpi-4.1.2-intel-classic-2021.6.0-magic/bin
$ ls $(dirname $(which mpicc))/../lib/pkgconfig
ompi-c.pc ompi-f77.pc ompi-fort.pc orte.pc
ompi-cxx.pc ompi-f90.pc ompi.pc pmix.pc
2f36fe1
to
7657726
Compare
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.
Looking good! Added some comments for a first pass. Apologize for my lack of knowledge of how gitlab ci works if I got anything wrong.
.gitlab/builds.gitlab-ci.yml
Outdated
- cd .. | ||
- mkdir build | ||
- cd build | ||
- ../configure --prefix=$(pwd) |
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 would not have the install prefix and build dir be the same directory, as it could get confusing. You could install into an inst
or install
subdirectory though...
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.
Apologies, I'm still a bit confused on the difference in make
and make install
. Are you proposing something like:
mkdir build
cd build
mkdir install
../configure --prefix=$(pwd)/install
make -j $(nproc) install
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.
And then (if I'm understanding this right) the binaries such as flux
will be in build/src/cmd
, while the pkgconfig files will be in build/install/lib/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.
Apologies, I'm still a bit confused on the difference in make and make install
make
just compiles a project, building all executables and libraries etc, but leaving them in place in the build directory. make install
installs a project into the system or prefix you configured it for (think of it like installing a package).
More info here: https://www.gnu.org/software/autoconf/manual/autoconf-2.63/html_node/Basic-Installation.html
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.
And then (if I'm understanding this right) the binaries such as flux will be in build/src/cmd
No, make install
will install the project to the specified --prefix
using a set of standard paths that linux distros expect, unless those paths were also overridden on the command line. So flux
will be in $prefix/usr/bin/flux
(src/cmd/flux
is the path to the built flux before installation).
Maybe an intro to autotools and the Linux filesystem hierarchy standard would be useful reading, e.g.
https://thoughtbot.com/blog/the-magic-behind-configure-make-make-install
Problem: flux-pmix and other framework projects will need to use .pc files from flux-core. Update all flux-core builds to use make install. Since projects are now running on different machines, use all available cores instead of the standard 32.
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.
Looks good! Just noted one thing that appears to be redundant.
.gitlab-ci.yml
Outdated
- git clone https://github.com/flux-framework/flux-pmix | ||
- cd flux-pmix | ||
- ./autogen.sh | ||
- PKG_CONFIG_PATH=${PKG_CONFIG_PATH} ./configure |
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.
Does this work without PKG_CONFIG_PATH=${PKG_CONFIG_PATH}
? Typically, assigning a variable to itself doesn't do anything, but if this doesn't work without it (especially after you've exported PKG_CONFIG_PATH
above), then maybe there is something to look into here.
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.
In my script, PKG_CONFIG_PATH was not exported so I needed that to get it set in the ./configure
environment.
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.
Ok, it is exported here which is why I was slightly confused, but isn't harmful so I'll leave it up to @wihobbs if he wants to do anything about 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.
Sorry - I didn't look beyond the quoted snippet :-) I agree, one or the other, not both.
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.
Dropped it.
Problem: We need flux-pmix to test OpenMPI v5.0+. Add it to our tests and check its testsuite against LC's versions of OpenMPI.
Problem: Jobs can hang on Tioga. Solution: There is now a dedicated CI queue on Tioga we can utilize.
We need a way of testing a side-installed flux-pmix along with a side-installed flux-core. This will enable us to test the LC provided versions of OpenMPI, particularly v5.0+ which will require flux-pmix.
This includes some refactoring of the way we build flux-core (
make install
to get the pkgconfig files) and a new test, corona-pmix-test.